fix: revert PR #142, merge correct admin CLI (PR #160), update e2e tests - #162
Closed
ralphbean wants to merge 53 commits into
Closed
fix: revert PR #142, merge correct admin CLI (PR #160), update e2e tests#162ralphbean wants to merge 53 commits into
ralphbean wants to merge 53 commits into
Conversation
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
When re-running install, the enrollment layer now updates the shim workflow content on existing enrollment branches instead of skipping repos with open PRs. This ensures PRs always reflect the latest shim (e.g., after switching from workflow_call to workflow_dispatch). Adds CreateOrUpdateFileOnBranch to the forge interface — combines SHA-aware upsert with branch targeting. Also adds PullRequests field to FakeClient for pre-populating open PRs in tests. Assisted-by: OpenCode claude-opus-4-6@default
Uses the gh CLI (pre-installed on GitHub-hosted runners) instead of raw curl for dispatching. Cleaner, no manual JSON escaping, and auth is handled via GH_TOKEN env var. Assisted-by: OpenCode claude-opus-4-6@default
After the user pastes the PAT, we now make a test API call to the .fullsend repo using the token. If the PAT was created with the wrong repo selected (easy to do since GitHub can't pre-fill repo selection via URL params), this catches it immediately with a clear error message instead of silently storing a broken token that fails on every future dispatch. Also improved the step-by-step instructions to be more explicit about selecting ONLY the .fullsend repository. Assisted-by: OpenCode claude-opus-4-6@default
After opening the browser for app installation, the CLI now polls ListOrgInstallations every 2 seconds until the app appears (up to 5 minutes). The user installs the app in the browser and the CLI proceeds automatically — no need to switch back to the terminal and press Enter. Assisted-by: OpenCode claude-opus-4-6@default
During uninstall, check ListOrgInstallations to confirm each app slug is real before opening the browser. Apps that don't exist are logged and skipped. Falls back to opening all if the installations API call fails. Assisted-by: OpenCode claude-opus-4-6@default
The fine-grained PAT creation UI requires the user to select which repos the token can access. If .fullsend doesn't exist yet, the user can't select it. Now the config repo layer runs first (creating the repo and writing config.yaml), then the PAT prompt opens. The full layer stack still runs afterward — the config repo layer is idempotent so the second pass is a no-op. Assisted-by: OpenCode claude-opus-4-6@default
The previous verification used GetRepo which only checks metadata:read, a permission implicitly granted to all org repos. A PAT without .fullsend explicitly selected would pass. Now uses GetLatestWorkflowRun which requires actions:read/write on the specific repo — catches misconfigured PATs before storing them. Assisted-by: OpenCode claude-opus-4-6@default
Aggressively strips \r, \n, and whitespace from the pasted token at both the CLI input layer and the encryption layer (defense in depth). Pasting from a browser can introduce invisible characters that corrupt the token when stored as a GitHub Actions secret. Also applies TrimSpace to CreateRepoSecret for consistency. Assisted-by: OpenCode claude-opus-4-6@default
Instead of checking actions:read (which passes even with wrong PAT config), the verification now attempts an actual workflow_dispatch on agent.yaml in .fullsend. This is the exact operation the shim will perform, so if verification passes, the shim will work. Also writes workflow files before the PAT prompt so agent.yaml exists when we attempt the test dispatch. Adds DispatchWorkflow to forge.Client interface with GitHub and fake implementations. Assisted-by: OpenCode claude-opus-4-6@default
GitHub's workflow dispatch API returns 204 (not 200/201) on success. The post() helper only accepted 200 and 201, causing the verification to fail with 'HTTP 204' error even when the dispatch succeeded. Now uses do() + checkStatus(204) directly instead of post(). Assisted-by: OpenCode claude-opus-4-6@default
When a semver tag (v*) is pushed, the release workflow cross-compiles the fullsend binary for linux and darwin (amd64/arm64), generates a changelog, and publishes a GitHub Release with the binaries attached. Also fixes the ldflags path in the Makefile to target the correct package variable (internal/cli.version instead of main.version). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat: add release workflow with GoReleaser
…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
Full lifecycle test: lock acquisition, teardown-first cleanup, app creation via Playwright browser automation, layer stack install/verify/analyze/uninstall/verify. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…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
Instead of requiring an E2E_DISPATCH_TOKEN env var or reusing the classic PAT, the e2e tests now automate the fine-grained PAT creation flow via Playwright — matching what the real CLI does in promptDispatchToken. The test flow mirrors the production CLI: 1. Install config-repo and workflows layers first (so .fullsend exists) 2. Create a fine-grained PAT scoped to .fullsend with actions:write 3. Install the full layer stack with the dispatch token This ensures the e2e tests exercise the same token type and scoping that production users will use, rather than cheating with an overly- permissioned classic PAT. Changes: - Add createDispatchPAT(): automates GitHub's fine-grained PAT creation page, selects the .fullsend repo, sets Actions write permission - Add deleteDispatchPAT(): cleanup counterpart for fine-grained PATs - Restructure runFullInstall() to pre-install config-repo + workflows before creating the dispatch PAT (mirrors CLI order of operations) - Remove E2E_DISPATCH_TOKEN env var requirement - Register dispatch PAT cleanup in t.Cleanup Assisted-by: OpenCode claude-opus-4-6@default
Multiple fixes from iterating against real GitHub UI: Browser automation (browser.go): - Fix manifest form submission: remove redirect_url input extraction (v6 puts redirect_url inside manifest JSON, not as separate field) - Add retry loop for app installation page (404 on freshly created apps) - Add 404 detection to skip nonexistent apps during cleanup - Fix app deletion confirm dialog: use Last() input, add sleep for modal - Reduce delete button timeout from 5s to 3s Cleanup (cleanup.go): - Delete apps by expected slug for each role (catches uninstalled apps) - Clean up stale dispatch token org secret - Handle both old (fullsend-<org>) and v6 (<org>-<role>) naming conventions Dispatch PAT creation (pat.go): - Automate fine-grained PAT creation via Playwright - Handle resource owner dropdown (custom React component, not <select>) - Use GetByLabel for token name input (no type=text attribute) - Work around GitHub's owner selector UI quirk (must re-select owner) - Navigate repo picker, close popover, interact with permissions UI - Add debug screenshots at each step for diagnosis CLI warning (admin.go): - Warn users about GitHub's resource owner selector quirk in promptDispatchToken output Test infrastructure: - Use fixed screenshot directory (/workspaces/fullsend/.playwright/) instead of t.TempDir() for debuggability - Add .playwright/ to .gitignore Assisted-by: OpenCode claude-opus-4-6@default
… extraction - Click confirmation dialog's Generate token button (use Last() to target dialog) - Extract token via JS by searching all inputs for github_pat_ prefix - Fix Actions permission checkbox click via JS - Close permissions popover before changing level dropdown - Add CLI warning about GitHub resource owner selector quirk Assisted-by: OpenCode claude-opus-4-6@default
- Use timestamp in dispatch PAT name to avoid name collisions - Add stale dispatch PAT cleanup in cleanupStaleResources - Fix deleteDispatchPAT to match tokens by prefix Assisted-by: OpenCode claude-opus-4-6@default
…pagation - Shorten token name to fit GitHub's 40-char limit (fs-dispatch-<org>-<ts>) - Add sleep between uninstall and verify to let repo deletion propagate - Add stale dispatch PAT cleanup Assisted-by: OpenCode claude-opus-4-6@default
Member
Author
|
Moving to #166. |
This was referenced Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #142 was merged by mistake — the demo in that PR was based on a different branch than the code that was actually merged. PR #160 (
agent-admin-cli-clean-room-v6) contains the correct implementation. This PR:What's different in v6 vs the accidentally merged version
The v6 admin CLI includes significant improvements over the code from #142:
E2E test changes
The e2e tests from PR #158 were written against the simpler #142 code and needed updates:
DispatchTokenLayerto all layer stack builders (install, uninstall, verify)E2E_DISPATCH_TOKENenv var for non-interactive dispatch token provisioningFULLSEND_DISPATCH_TOKENorg secretVerification
go test -race -count=1 ./...— all 7 packages passgo vet ./...— cleango vet -tags e2e ./e2e/admin/— cleango build ./cmd/fullsend/— builds successfully