Skip to content

Fixing new admin commands on main - #160

Closed
ralphbean wants to merge 44 commits into
mainfrom
agent-admin-cli-clean-room-v6
Closed

Fixing new admin commands on main#160
ralphbean wants to merge 44 commits into
mainfrom
agent-admin-cli-clean-room-v6

Conversation

@ralphbean

@ralphbean ralphbean commented Apr 3, 2026

Copy link
Copy Markdown
Member

When #142 got merged, I made a mistake. You can see (as @waynesun09 has pointed out) that the demo was based on a different branch than the branch in that PR. We merged the wrong thing. This was the right set of changes.

I want this to also incorporate the changes from #158 so we can see an e2e test run in the gh workflow before merging this.

ralphbean added 30 commits April 2, 2026 03:03
Assisted-by: OpenCode claude-opus-4-6@default
Assisted-by: OpenCode claude-opus-4-6@default
Add the ui.Printer type with lipgloss-styled methods for consistent
terminal output across the CLI: banner, headers, step indicators,
key-value pairs, summary/error boxes, and PR links. Includes full
test coverage for all 12 methods.

Assisted-by: OpenCode claude-opus-4-6@default
The forge.Client interface abstracts all git forge operations, enabling
future support for GitHub, GitLab, and Forgejo.

Assisted-by: OpenCode claude-opus-4-6@default
Implements all forge.Client methods against the GitHub REST API including
repo management, file operations, secret encryption, and workflow queries.
Includes GitHub-specific types for App configuration with role-based presets.

Assisted-by: OpenCode claude-opus-4-6@default
Handles OrgConfig types, YAML marshal/unmarshal, validation, and
helper methods for accessing enabled repos and agent slugs.

Assisted-by: OpenCode claude-opus-4-6@default
Layers represent discrete installation concerns processed in order for
install, reverse order for uninstall, and assessed individually for analyze.

Assisted-by: OpenCode claude-opus-4-6@default
Handles creation, configuration, and teardown of the org-level
.fullsend configuration repository. The layer creates the repo
(private or public based on org capability), writes config.yaml,
and provides analysis of existing installation state.

Assisted-by: OpenCode claude-opus-4-6@default
Manages reusable agent dispatch workflow, onboarding workflow, and
CODEOWNERS in the .fullsend config repo.

Assisted-by: OpenCode claude-opus-4-6@default
Stores agent app private keys as repo secrets and app IDs as repo
variables in the .fullsend config repo.

Assisted-by: OpenCode claude-opus-4-6@default
Creates enrollment PRs with shim workflow files for enabled repos
that are not yet connected to the fullsend agent pipeline.

Assisted-by: OpenCode claude-opus-4-6@default
Handles creating and installing per-role GitHub Apps using the
manifest flow, with support for reusing existing apps.

Assisted-by: OpenCode claude-opus-4-6@default
Implements fullsend admin {install,uninstall,analyze} <org> with
layer-based installation model and forge-agnostic client interface.

- Root command with Cobra, version support, and silence flags
- Admin subcommand grouping install, uninstall, and analyze
- Install: app setup, repo discovery, layer stack creation and execution
- Uninstall: confirmation prompt, layer teardown, manual cleanup hints
- Analyze: layer-by-layer status assessment with actionable reporting
- Token resolution from GH_TOKEN, GITHUB_TOKEN, or gh CLI
- Org name validation
- Dry-run mode for install preview

Assisted-by: OpenCode claude-opus-4-6@default
Critical fixes:
- Fix XSS vulnerability in app manifest HTML form (html.EscapeString)
- Add retry with backoff on rate-limited responses (429 + 403 w/ Retry-After)
- Fix exchangeManifestCode to use context-aware HTTP client with timeout
- Add GetRepo to forge.Client for O(1) repo existence checks

Important fixes:
- Add forge.IsNotFound/ErrNotFound for structured error discrimination
- Fix runUninstall to return error when uninstall operations fail
- Secrets layer Analyze now checks both secrets and variables
- Move DefaultAgentRoles to config package to decouple CLI from forge/github
- Fix FakeClient embedding by pointer in enrollment test (mutex safety)
- Add shared forge.ConfigRepoName constant used across all packages
- Fix go.mod to use go 1.25.8

Minor fixes:
- Use errors.As in isNotFound instead of manual unwrap loop
- Add pagination to ListRepoPullRequests
- Delete pointless TestCompileTimeInterfaceCheck
- Add bin/ to .gitignore

Assisted-by: OpenCode claude-opus-4-6@default
Documents non-obvious GitHub API behaviors discovered during the original
implementation:
- auto_init is async; file writes after repo creation need retry
- Contents API requires existing file SHA for updates (422 otherwise)
- Sequential file writes cause transient 404s as branch refs update
- Writing to .github/workflows/ returns 404 (not 403) without workflow scope
- App PEM private keys are one-shot; only available at creation time
- Event subscriptions must have matching permissions or manifest is rejected
- App installation URL must not include target_id parameter
- Org-scoped app settings need /advanced suffix in URL
- App uninstall API requires JWT auth, not PAT (browser fallback needed)
- Users can rename apps during creation; match by stored slug first
- Token scopes (delete_repo, workflow) are often missing from default gh auth

Assisted-by: OpenCode claude-opus-4-6@default
Preflight checks: each layer declares the OAuth scopes it needs for
install, uninstall, and analyze via RequiredScopes(). Before running
any operation, the CLI queries the token's scopes (via X-OAuth-Scopes
header) and fails early with remediation instructions if scopes are
missing. This prevents confusing mid-operation failures like the 403
on repo deletion when delete_repo scope is absent.

Auto-reuse apps: when an existing GitHub App is found with its PEM
secret still stored, it is now reused automatically without prompting.
The previous confirm prompt was inconsistent with how other resources
(repos, workflows, secrets) are handled — those are silently reused.
To get fresh apps, uninstall first, then reinstall.

Assisted-by: OpenCode claude-opus-4-6@default
Instead of printing URLs for the user to copy-paste, uninstall now
opens the browser directly to each app's advanced settings page
(/organizations/{org}/settings/apps/{slug}/advanced) where the
'Delete GitHub App' button lives.

Assisted-by: OpenCode claude-opus-4-6@default
ConfigRepoLayer.Uninstall: checks if repo exists before deleting;
if already gone, logs and proceeds. Also handles the race where the
repo is deleted between the check and the delete call.

ConfigRepoLayer.Install: if CreateRepo fails, re-checks existence
to handle 'already exists' errors from races or repeated runs.

EnrollmentLayer.Install: treats CreateBranch errors as non-fatal
(branch may exist from a previous partial run). Also checks for
existing enrollment PRs before creating duplicates.

All layers were already idempotent for their other operations:
- WorkflowsLayer uses CreateOrUpdateFile (upsert)
- SecretsLayer uses CreateRepoSecret/CreateOrUpdateRepoVariable (upsert)
- All no-op Uninstall methods are trivially idempotent

Assisted-by: OpenCode claude-opus-4-6@default
When the .fullsend repo has already been deleted (e.g., partial
uninstall), the uninstall command can no longer read config.yaml
to find the actual app slugs. Previously this caused the app
cleanup step to be silently skipped, leaving orphaned apps that
block reinstallation (PEM keys are only available at creation time).

Now falls back to the default naming convention (fullsend-{org},
fullsend-{org}-triage, etc.) so the browser is still opened to
the correct deletion pages. Also exports ExpectedAppSlug for use
outside the appsetup package.

Assisted-by: OpenCode claude-opus-4-6@default
GitHub's manifest flow requires redirect_url and hook_attributes to
be inside the JSON manifest body, not as separate form fields. The
previous code sent redirect_url as its own hidden input, which GitHub
rejected with 'redirect_url wasn't supplied'.

Also adds hook_attributes with active:false to the manifest — GitHub
requires this field even when webhooks are not used.

Assisted-by: OpenCode claude-opus-4-6@default
All app names now follow the uniform pattern <org>-<role>:
  apache-fullsend, apache-triage, apache-coder, apache-review

Previously the fullsend role used 'fullsend-<org>' while others used
'fullsend-<org>-<role>', which was inconsistent and made the
orchestrator app name ambiguous for orgs with common names.

Assisted-by: OpenCode claude-opus-4-6@default
GitHub's auto_init is asynchronous — CreateRepo returns 201 before
the default branch is fully materialized. The Contents API returns
404 until the initial commit lands. Sequential file writes can also
hit 409 (conflict) as the branch ref updates between commits.

CreateOrUpdateFile and CreateFileOnBranch now retry up to 5 times
with 2s linear backoff on 404 and 409 errors. Non-transient errors
(permission denied, validation errors) fail immediately.

Assisted-by: OpenCode claude-opus-4-6@default
When a previous install partially completed, the enrollment branch
and shim workflow file may already exist. CreateFileOnBranch returns
422 'sha wasn't supplied' in this case. Now treats that as 'file
already present' and proceeds to PR creation.

Assisted-by: OpenCode claude-opus-4-6@default
Adds CreateOrgSecret, OrgSecretExists, DeleteOrgSecret, and
SetOrgSecretRepos to forge.Client for managing org-level Actions
secrets. Also adds ID field to forge.Repository for scoping
org secrets to selected repositories.

Assisted-by: OpenCode claude-opus-4-6@default
The DispatchTokenLayer manages FULLSEND_DISPATCH_TOKEN, an org-level
Actions secret that enrolled repos use to trigger workflow_dispatch
events on the .fullsend config repo. This replaces the previous
model where App private keys were passed via workflow_call secrets.

Assisted-by: OpenCode claude-opus-4-6@default
The agent dispatch workflow now uses workflow_dispatch instead of
workflow_call. Shim workflows in enrolled repos trigger dispatch via
curl using FULLSEND_DISPATCH_TOKEN (an org-level secret), rather
than passing App private keys via workflow_call secrets. This ensures
private keys never leave the .fullsend repo.

The shim uses pull_request_target instead of pull_request to prevent
malicious PRs from modifying the workflow to exfiltrate the dispatch
token.

Assisted-by: OpenCode claude-opus-4-6@default
The install flow now prompts for a fine-grained PAT (or reuses an
existing one) and stores it as the FULLSEND_DISPATCH_TOKEN org secret.
Uninstall deletes it. Analyze checks for its existence.

Assisted-by: OpenCode claude-opus-4-6@default
…h security

Records architectural decisions made on the admin-cli-clean-room branch:
- ADR 0004: Forge abstraction layer
- ADR 0005: Ordered layer model
- ADR 0006: Per-role GitHub Apps
- ADR 0007: workflow_dispatch for cross-repo dispatch
- ADR 0008: pull_request_target in shim workflows

Updates architecture.md and agent-architecture.md to reflect decisions.

Assisted-by: OpenCode claude-opus-4-6@default
The preflight scope check now runs before promptDispatchToken so that
a missing admin:org scope is caught with clear remediation instructions
before the user is asked to paste a PAT.

Also makes OrgSecretExists treat 403 as 'unknown' (returns false)
instead of a hard error. This handles the case where preflight can't
introspect scopes (fine-grained tokens) — the operation proceeds and
fails at the actual CreateOrgSecret call with a clear error.

Assisted-by: OpenCode claude-opus-4-6@default
Instead of asking the user to manually fill out the fine-grained PAT
form, we now open the browser to GitHub's token creation page with
name, description, resource owner, and actions:write permission
pre-filled via URL query parameters. The user only needs to:
  1. Select 'Only select repositories' and pick .fullsend
  2. Click 'Generate token'
  3. Paste the result

Assisted-by: OpenCode claude-opus-4-6@default
feat: add release workflow with GoReleaser
@ralphbean ralphbean changed the title Agent admin cli clean room v6 Fixing main Apr 3, 2026
@ralphbean ralphbean changed the title Fixing main Fixing new admin commands on main Apr 3, 2026
ralphbean added a commit that referenced this pull request Apr 3, 2026
…n CLI implementation

Merges the correct admin CLI implementation from PR #160 after reverting
the incorrectly merged PR #142. The v6 branch includes:
- Dispatch token layer for org-level cross-repo dispatch
- Preflight scope checks and auto-reuse of existing apps
- Idempotent layer operations
- ADRs for forge abstraction, layer model, app model, dispatch security
- GoReleaser release workflow
- Numerous bug fixes and improvements

Assisted-by: OpenCode claude-opus-4-6@default
ralphbean added a commit that referenced this pull request Apr 3, 2026
…yer)

The e2e tests were written against the simpler admin CLI from PR #142 but
need to work with the v6 admin CLI from PR #160 which adds:

- DispatchTokenLayer for org-level cross-repo dispatch secrets
- Preflight scope checks
- Additional forge.Client interface methods (org secrets, dispatch)

Changes:
- Add DispatchTokenLayer to buildTestLayerStack, runUninstall,
  runUninstallAllowNotFound, and verifyNotInstalled
- Add dispatch token verification (OrgSecretExists) to verifyInstalled
  and verifyNotInstalled
- Add E2E_DISPATCH_TOKEN env var for non-interactive dispatch token
  provisioning
- Collect enrolled repo IDs during install for dispatch token scoping
- Add cleanup of stale FULLSEND_DISPATCH_TOKEN org secret
- Second install uses empty dispatch token to exercise reuse path

Assisted-by: OpenCode claude-opus-4-6@default
@ralphbean
ralphbean marked this pull request as draft April 3, 2026 16:58
ralphbean added a commit that referenced this pull request Apr 3, 2026
…en automation

Implements end-to-end tests for the admin install/uninstall/analyze flow,
adapted to work with the v6 admin CLI from PR #160 which adds the
DispatchTokenLayer.

Test flow (mirrors production CLI):
1. Create 4 GitHub Apps via manifest flow (fullsend, triage, coder, review)
2. Install apps on the org via Playwright browser automation
3. Pre-install config-repo + workflows layers (so .fullsend repo exists)
4. Create fine-grained PAT scoped to .fullsend with Actions permission
   via Playwright (automating GitHub's PAT creation UI)
5. Install full layer stack including DispatchTokenLayer
6. Verify all resources exist (secrets, variables, org secret, enrollment PR)
7. Re-install idempotently (verify no-op behavior)
8. Uninstall all layers and verify cleanup
9. Re-uninstall idempotently (verify not-found handling)

Key implementation details:
- Fine-grained PAT creation handles GitHub's resource owner selector quirk
  (must manually select org even when pre-filled via query param)
- Added CLI warning about this quirk for human users
- Browser automation handles 404 retries on freshly created apps
- Cleanup handles both old (fullsend-<org>) and v6 (<org>-<role>) app naming
- Cleanup deletes apps by expected slug (catches uninstalled apps not in
  ListOrgInstallations)
- Screenshots saved to .playwright/ for debugging
- Dispatch PAT names include timestamp to avoid collisions
- Delays after repo deletion to handle GitHub's async propagation

Assisted-by: OpenCode claude-opus-4-6@default
@ralphbean

Copy link
Copy Markdown
Member Author

Moving to #160

@ralphbean ralphbean closed this Apr 3, 2026
ralphbean added a commit that referenced this pull request Apr 3, 2026
…en automation

Implements end-to-end tests for the admin install/uninstall/analyze flow,
adapted to work with the v6 admin CLI from PR #160 which adds the
DispatchTokenLayer.

Test flow (mirrors production CLI):
1. Create 4 GitHub Apps via manifest flow (fullsend, triage, coder, review)
2. Install apps on the org via Playwright browser automation
3. Pre-install config-repo + workflows layers (so .fullsend repo exists)
4. Create fine-grained PAT scoped to .fullsend with Actions permission
   via Playwright (automating GitHub's PAT creation UI)
5. Install full layer stack including DispatchTokenLayer
6. Verify all resources exist (secrets, variables, org secret, enrollment PR)
7. Re-install idempotently (verify no-op behavior)
8. Uninstall all layers and verify cleanup
9. Re-uninstall idempotently (verify not-found handling)

Key implementation details:
- Fine-grained PAT creation handles GitHub's resource owner selector quirk
  (must manually select org even when pre-filled via query param)
- Added CLI warning about this quirk for human users
- Browser automation handles 404 retries on freshly created apps
- Cleanup handles both old (fullsend-<org>) and v6 (<org>-<role>) app naming
- Cleanup deletes apps by expected slug (catches uninstalled apps not in
  ListOrgInstallations)
- Screenshots saved to .playwright/ for debugging
- Dispatch PAT names include timestamp to avoid collisions
- Delays after repo deletion to handle GitHub's async propagation

Assisted-by: OpenCode claude-opus-4-6@default
github-merge-queue Bot pushed a commit that referenced this pull request Apr 6, 2026
…2e-clean

fix: revert PR #142, merge correct admin CLI (PR #160), add e2e tests
@ben-alkov
ben-alkov deleted the agent-admin-cli-clean-room-v6 branch April 23, 2026 16:58
This was referenced Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant