Skip to content

fix(#2163): skip PEM prompt in github setup when using mint URL#2164

Merged
rh-hemartin merged 1 commit into
mainfrom
agent/2163-github-setup-skip-pem-prompt
Jun 15, 2026
Merged

fix(#2163): skip PEM prompt in github setup when using mint URL#2164
rh-hemartin merged 1 commit into
mainfrom
agent/2163-github-setup-skip-pem-prompt

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

When github setup runs against an org where apps are already installed, it calls runAppSetup with an empty mintProject. Without a project, no GCF provisioner is created and the fallback secretExists callback checks GitHub repo secrets — which don't exist in OIDC mint mode (PEMs live in Secret Manager). The check returns false, triggering handleExistingApprecoverPEM → an interactive PEM prompt.

Add a mintURL parameter to runAppSetup. When mintURL is non-empty but mintProject is empty (the github-setup flow), skip setting secretExists and storeSecret callbacks entirely. This lets handleExistingApp take the "no secretExists — assume reuse" path, silently reusing apps without prompting.

Also skip storeSecret in the same condition since PEM storage is handled by the remote mint — the local CLI has no project to write to.

Note: go test and pre-commit could not run due to sandbox Go module cache permission errors (infrastructure issue, not code). go vet ./internal/cli/... and go vet ./internal/appsetup/... both pass cleanly.


Closes #2163

Post-script verification

  • Branch is not main/master (agent/2163-github-setup-skip-pem-prompt)
  • Secret scan passed (gitleaks — a80d472194cc922471bcd1c2fba27ce5ddf9141b..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

When `github setup` runs against an org where apps are already
installed, it calls `runAppSetup` with an empty `mintProject`.
Without a project, no GCF provisioner is created and the fallback
`secretExists` callback checks GitHub repo secrets — which don't
exist in OIDC mint mode (PEMs live in Secret Manager). The check
returns false, triggering `handleExistingApp` → `recoverPEM` →
an interactive PEM prompt.

Add a `mintURL` parameter to `runAppSetup`. When `mintURL` is
non-empty but `mintProject` is empty (the github-setup flow),
skip setting `secretExists` and `storeSecret` callbacks entirely.
This lets `handleExistingApp` take the "no secretExists — assume
reuse" path, silently reusing apps without prompting.

Also skip `storeSecret` in the same condition since PEM storage
is handled by the remote mint — the local CLI has no project
to write to.

Note: go test and pre-commit could not run due to sandbox Go
module cache permission errors (infrastructure issue, not code).
`go vet ./internal/cli/...` and `go vet ./internal/appsetup/...`
both pass cleanly.

Closes #2163
@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://0fb2ad4b-site.fullsend-ai.workers.dev

Commit: 2a452c366ada20d34cc49d58eb098ba7717a6841

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:37 AM UTC · Completed 10:47 AM UTC
Commit: 2a452c3 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [error-handling-gap / fail-open] internal/cli/admin.go:1346 — When mintURL is non-empty but mintProject is empty, neither storeSecret nor secretExists callbacks are set. If a new app is created via the manifest flow in this configuration (e.g., github setup on a fresh org with no existing apps), the PEM returned by GitHub is silently discarded (appsetup.go line 304: storeSecret nil causes storage to be skipped). The PEM is only provided once during manifest code exchange, so it is permanently lost. The recoverCreatedApp path (line 325) also short-circuits when secretExists is nil, disabling partial-failure recovery.
    Remediation: Either (a) return an error if storeSecret is nil and a new app is created with a non-empty PEM, or (b) block new app creation entirely when storeSecret is nil by guarding the manifest flow, or (c) fall back to GitHub repo secret storage.

Low

  • [architectural-coherence] internal/cli/admin.go — The PR introduces a third PEM management mode (nil callbacks when mintURL != "" && mintProject == ""), but the existing comments at lines 1346 and 1359 only describe two of three branches and are now stale. Consider updating comments to document all three PEM management modes.

  • [function-signature-complexity] internal/cli/admin.go:1314runAppSetup gains an 11th parameter (mintURL). The codebase uses config structs elsewhere (e.g., perRepoInstallConfig). Consider refactoring to a struct parameter in follow-up work.

  • [test-adequacy] internal/appsetup/appsetup_test.go:368 — The new test covers the existing-app reuse path with nil secretExists, but there is no test for the new-app-creation path when both secretExists and storeSecret are nil (the PEM loss scenario described above).

Info

  • [fail-open] internal/appsetup/appsetup.go:613 — The handleExistingApp fallback ("No secretExists function — can't check, assume reuse") is an intentional fail-open for the reuse path, acceptable for the mint-URL-only configuration where PEMs are managed externally.

  • [test-coverage] internal/appsetup/appsetup_test.go:368 — No integration test verifies the end-to-end CLI wiring: that github setup --mint-url results in secretExists being nil. The unit test does verify the key behavioral contract.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 11, 2026
@rh-hemartin rh-hemartin added this pull request to the merge queue Jun 15, 2026
Merged via the queue into main with commit 967a663 Jun 15, 2026
13 checks passed
@rh-hemartin rh-hemartin deleted the agent/2163-github-setup-skip-pem-prompt branch June 15, 2026 06:35
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 15, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 6:39 AM UTC · Completed 6:51 AM UTC
Commit: 2a452c3 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2164 — Skip PEM prompt in github setup when using mint URL

Timeline

Time (UTC) Event
Jun 11 10:16 Human files issue #2163github setup incorrectly prompts for PEM files in OIDC mint mode
Jun 11 10:23–10:25 Triage agent (run) produces accurate root cause analysis in ~2 min
Jun 11 10:26–10:34 Code agent (run) creates PR with single commit in ~8 min
Jun 11 10:35–10:47 Review agent (run) posts review: 1 medium, 3 low, 2 info findings
Jun 15 06:08 Human approves PR
Jun 15 ~06:35 PR merged, issue closed

Assessment

This workflow went well overall. Issue-to-PR in ~18 minutes, single iteration, no rework. The triage agent produced an accurate root cause analysis that guided the code agent effectively. The code agent made a reasonable engineering decision to diverge from the triage recommendation (skipping callbacks vs passing mintProject through), producing a simpler solution.

The review agent caught a legitimate medium-severity edge case: PEM loss when storeSecret is nil and a new app is created via the manifest flow. This finding was not addressed before merge — no fix agent was triggered, and the human approved 4 days later without resolving it. The code agent also noted it could not run go test due to sandbox permission errors.

Gaps identified — all already tracked

All improvement opportunities I identified are covered by existing open issues:

  • Medium finding not triggering fix agent or blocking merge#870
  • Human approving with unresolved medium+ findings#2099
  • No tracking issue auto-filed for unresolved findings#1956
  • Review agent not flagging sandbox test failures#2108

No new proposals filed — existing issues adequately cover the improvement opportunities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

github setup prompts for app PEM when apps already exist in the org

1 participant