diff --git a/.gitignore b/.gitignore index 558974d1bb..e52f922e59 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ __pycache__/ .ruff_cache/ _site/ bin/ +.playwright/ diff --git a/Makefile b/Makefile index d5c4aff076..374beba5de 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ .DEFAULT_GOAL := help .PHONY: help bootstrap lint check fmt lint-adr-status lint-adr-numbers lint-adr-frontmatter mindmap \ - go-build go-test go-lint go-fmt go-vet go-tidy + go-build go-test go-lint go-fmt go-vet go-tidy e2e-test e2e-playwright help: @echo "Available targets:" @@ -19,6 +19,7 @@ help: @echo " go-fmt - Format Go code" @echo " go-vet - Run go vet" @echo " go-tidy - Run go mod tidy" + @echo " e2e-test - Run admin e2e tests (requires E2E_GITHUB_USERNAME and E2E_GITHUB_PASSWORD or E2E_GITHUB_PASSWORD_FILE)" # Install all development tools needed for linting, formatting, and pre-commit hooks. # Prerequisites: uv (https://docs.astral.sh/uv/) and go (https://go.dev/) @@ -28,7 +29,7 @@ help: BOOTSTRAP_TOOL_DIR := $(HOME)/.local/share/uv-tools BOOTSTRAP_BIN_DIR := $(HOME)/.local/bin -bootstrap: +bootstrap: e2e-playwright @mkdir -p "$(BOOTSTRAP_BIN_DIR)" @echo "==> Installing Python 3.12 (via uv)..." uv python install 3.12 @@ -91,3 +92,12 @@ go-vet: go-tidy: go mod tidy + +e2e-test: e2e-playwright + go test -tags e2e -v -count=1 -timeout 4m ./e2e/admin/ + +e2e-playwright: + @if [ -z "$$(ls -d $(HOME)/.cache/ms-playwright/chromium-* 2>/dev/null)" ]; then \ + echo "==> Installing Playwright Chromium..."; \ + go run github.com/playwright-community/playwright-go/cmd/playwright install chromium; \ + fi diff --git a/docs/ADRs/0004-forge-abstraction-layer.md b/docs/ADRs/0004-forge-abstraction-layer.md new file mode 100644 index 0000000000..67ef24e71d --- /dev/null +++ b/docs/ADRs/0004-forge-abstraction-layer.md @@ -0,0 +1,36 @@ +--- +title: "4. Forge abstraction layer" +status: Accepted +relates_to: + - agent-infrastructure + - agent-architecture +topics: + - forge + - portability + - interfaces +--- + +# 4. Forge abstraction layer + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +Fullsend must eventually support GitHub, GitLab, and Forgejo. Every operation that touches the git forge — creating repos, managing secrets, writing files, listing installations — must work across all three. Without a shared abstraction, forge-specific logic would spread throughout the codebase, making multi-forge support a rewrite rather than an extension. + +## Decision + +All forge operations go through the `forge.Client` interface (`internal/forge/forge.go`). The interface uses forge-neutral vocabulary: `ChangeProposal` instead of "pull request" or "merge request," `CreateChangeProposal` instead of `CreatePR`. Forge-specific implementations live in sub-packages (`internal/forge/github/`). A thread-safe `FakeClient` exists for testing without forge access. + +No code outside `internal/forge/` imports forge-specific packages directly. + +## Consequences + +- Adding a new forge (GitLab, Forgejo) requires implementing `forge.Client` — no changes to layers, CLI, or app setup code. +- Forge-neutral naming occasionally feels awkward (e.g., `ChangeProposal`), but prevents GitHub-centric thinking from leaking into the model. +- The interface will grow as new operations are needed; keeping it cohesive requires discipline. +- The `FakeClient` enables deterministic testing of every layer without network calls. diff --git a/docs/ADRs/0005-ordered-layer-model.md b/docs/ADRs/0005-ordered-layer-model.md new file mode 100644 index 0000000000..dcf634984e --- /dev/null +++ b/docs/ADRs/0005-ordered-layer-model.md @@ -0,0 +1,38 @@ +--- +title: "5. Ordered layer model for install, uninstall, and analyze" +status: Accepted +relates_to: + - agent-infrastructure +topics: + - installation + - layers + - idempotency +--- + +# 5. Ordered layer model for install, uninstall, and analyze + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +Installing fullsend into an org involves multiple concerns with ordering dependencies: the config repo must exist before workflows can be written to it; secrets must be stored before enrollment can reference them. Uninstalling must reverse this order. An analyze command must inspect each concern independently to report what exists, what is missing, and what install would do. + +## Decision + +Each installation concern is a `Layer` implementing `Install`, `Uninstall`, and `Analyze`. Layers are composed into an ordered `Stack`. Install runs layers forward; uninstall runs them in reverse; analyze runs them forward and collects reports. + +The current stack order is: config-repo → workflows → secrets → dispatch-token → enrollment. + +Each layer is idempotent — re-running install skips already-completed work. Uninstall collects all errors rather than stopping on the first, so partial teardown still makes progress. Each layer declares the OAuth scopes it needs via `RequiredScopes`, enabling a preflight check that fails early when the token lacks required permissions. + +## Consequences + +- Adding a new installation concern means implementing the `Layer` interface and inserting it at the right position in the stack. +- The analyze command can report partial installations and explain exactly what install would create or fix. +- Idempotency means install is also the repair command — no separate "fix" operation needed. +- The ordering contract is implicit (stack construction order). A future layer that violates ordering assumptions will fail at runtime, not compile time. +- Reverse-order uninstall with error collection ensures best-effort cleanup even when some layers fail. diff --git a/docs/ADRs/0006-per-role-github-apps.md b/docs/ADRs/0006-per-role-github-apps.md new file mode 100644 index 0000000000..90b1b9ec10 --- /dev/null +++ b/docs/ADRs/0006-per-role-github-apps.md @@ -0,0 +1,39 @@ +--- +title: "6. Per-role GitHub Apps with manifest-based creation" +status: Accepted +relates_to: + - agent-architecture + - security-threat-model +topics: + - identity + - github-apps + - least-privilege +--- + +# 6. Per-role GitHub Apps with manifest-based creation + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +Agents need forge credentials to act on repos. A single shared credential for all agent roles violates least-privilege: a review agent would hold write permissions it should never use. The identity model must scope permissions per role while keeping setup automatable. See [agent-architecture.md](../problems/agent-architecture.md) and [security-threat-model.md](../problems/security-threat-model.md). + +## Decision + +Each agent role (triage, implementation, review) gets its own GitHub App, created via the [app manifest flow](https://docs.github.com/en/apps/sharing-github-apps/registering-a-github-app-from-a-manifest). Apps follow the naming convention `-`. The manifest defines per-role permissions (e.g., review gets read-only code access; implementation gets read-write). + +Private keys (PEMs) are stored as repo-level secrets on the `.fullsend` config repo. App IDs are stored as repo-level variables. Secrets never leave the config repo — agent dispatch workflows in `.fullsend` read them at runtime. + +The installer checks for existing app installations before creating new ones. If an app exists and its PEM secret is present, it is reused. If the PEM is lost (it is only available at creation time), the user must delete the app and re-run install. + +## Consequences + +- Each role gets exactly the permissions it needs — compromising one app does not grant another role's permissions. +- The manifest flow requires a browser-based OAuth redirect, making fully headless installation impossible. Acceptable for an org-admin operation run infrequently. +- PEMs are write-once secrets: lost keys require app deletion and recreation. +- The per-app model scales linearly with roles. Adding a new role means creating a new app — no shared credential rotation needed. +- GitLab and Forgejo will need equivalent per-role identity mechanisms when their forge implementations are built. diff --git a/docs/ADRs/0007-workflow-dispatch-for-cross-repo-dispatch.md b/docs/ADRs/0007-workflow-dispatch-for-cross-repo-dispatch.md new file mode 100644 index 0000000000..d945222b98 --- /dev/null +++ b/docs/ADRs/0007-workflow-dispatch-for-cross-repo-dispatch.md @@ -0,0 +1,39 @@ +--- +title: "7. workflow_dispatch for cross-repo agent dispatch" +status: Accepted +relates_to: + - agent-infrastructure + - security-threat-model +topics: + - dispatch + - secrets + - workflows +--- + +# 7. workflow_dispatch for cross-repo agent dispatch + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +Enrolled repos must route events (issues, PRs, comments) to the agent dispatch workflow in the `.fullsend` config repo. The original design used `workflow_call` (reusable workflows), which requires the calling workflow to pass secrets explicitly — every enrolled repo's shim workflow would contain secret references, and the called workflow's secrets are scoped to the *caller's* repo, not the config repo where the App PEMs live. + +See [security-threat-model.md](../problems/security-threat-model.md) and [agent-infrastructure.md](../problems/agent-infrastructure.md). + +## Decision + +Use `workflow_dispatch` instead of `workflow_call`. Enrolled repos trigger a dispatch event on `.fullsend` via a curl call authenticated with `FULLSEND_DISPATCH_TOKEN` — a fine-grained PAT scoped to `.fullsend` with `actions:write`. The dispatch token is stored as an org-level Actions secret with visibility restricted to enrolled repos only. + +This means secrets are separated by layer: the dispatch token (org secret, visible to enrolled repos) enables triggering; the App PEMs (repo secrets on `.fullsend`) are only accessible to workflows running *in* `.fullsend`. Enrolled repos never see the PEMs. + +## Consequences + +- App PEM secrets stay in the config repo. No secret passing across repo boundaries. +- The dispatch token is a single PAT with narrow scope — the blast radius of its compromise is limited to triggering workflow_dispatch events on `.fullsend`, not credential theft. +- `workflow_dispatch` is compute-platform-agnostic: any CI system that can receive dispatch events works. +- The dispatch token must be manually created (fine-grained PATs cannot be created via API). This is a one-time step during install. +- Adding or removing enrolled repos requires updating the org secret's repo access list. diff --git a/docs/ADRs/0008-pull-request-target-in-shim-workflows.md b/docs/ADRs/0008-pull-request-target-in-shim-workflows.md new file mode 100644 index 0000000000..5986a248e9 --- /dev/null +++ b/docs/ADRs/0008-pull-request-target-in-shim-workflows.md @@ -0,0 +1,45 @@ +--- +title: "8. Use pull_request_target in shim workflows" +status: Accepted +relates_to: + - security-threat-model +topics: + - workflows + - security + - pull-request-target +--- + +# 8. Use pull_request_target in shim workflows + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +The shim workflow in enrolled repos (`.github/workflows/fullsend.yaml`) references `FULLSEND_DISPATCH_TOKEN` to trigger agent dispatch. Using `pull_request` as the trigger means a malicious PR could modify the workflow file to exfiltrate this token — `pull_request` runs the *PR branch* version of the workflow. Using `pull_request_target` runs the *base branch* version, so PR authors cannot alter the workflow that executes. + +## Decision + +Use `pull_request_target` for PR-related events in the shim workflow. The shim never checks out PR code — it is a static curl call that forwards event metadata to the dispatch workflow in `.fullsend`. + +**Why this is safe despite `pull_request_target`'s reputation:** The "pwn request" vulnerability class requires `pull_request_target` combined with checkout of untrusted code and execution of that code. Our shim does none of that — it reads only `github.event_name`, `github.repository`, and `toJSON(github.event)` from the event context, then curls the dispatch endpoint. No checkout, no build, no script execution from the PR. + +**Residual risk:** A compromised dispatch token could trigger `workflow_dispatch` events on `.fullsend`. This is a DoS vector (burn Actions minutes) but not credential theft — the dispatch workflow reads its own repo secrets, and the caller cannot influence which secrets are accessed. This risk is acceptable. + +CODEOWNERS on the shim workflow path provides defense-in-depth: even if an attacker could somehow modify the base branch workflow, the change requires human approval. + +**Alternatives considered:** + +1. **`pull_request`** — exposes the dispatch token to PR-authored workflow modifications. Rejected. +2. **No token / webhook-based dispatch** — requires a hosted webhook receiver, breaking compute-platform agnosticism. Rejected. +3. **Org-level `pull_request_target` prohibition** — some orgs disable `pull_request_target` via repository rulesets. Document as a known configuration requirement for adopters. + +## Consequences + +- PR authors cannot modify the shim workflow to exfiltrate the dispatch token. +- The shim must never be extended to checkout PR code — this invariant must be maintained as the shim evolves. +- Orgs with blanket `pull_request_target` prohibitions must allowlist the shim workflow. +- Security auditors reviewing the repo will flag `pull_request_target` — the shim's inline comments explain why it is safe. diff --git a/docs/architecture.md b/docs/architecture.md index 36db8d47fc..fb5bec6052 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -14,6 +14,13 @@ This is the "where do agents physically run" question — whether that's a manag Infrastructure platform choice and configuration are specified in the adopting organization's **`.fullsend`** repository. (See [ADR 0003](ADRs/0003-org-config-repo-convention.md).) +**Decided:** + +- Forge abstraction: all forge operations go through the `forge.Client` interface, keeping the rest of the codebase forge-agnostic ([ADR 0004](ADRs/0004-forge-abstraction-layer.md)). +- Installation model: ordered layer stack (install forward, uninstall reverse, analyze for status reporting) with idempotent operations ([ADR 0005](ADRs/0005-ordered-layer-model.md)). +- Cross-repo dispatch: `workflow_dispatch` with an org-level dispatch token replaces `workflow_call`, keeping App PEM secrets in the config repo ([ADR 0007](ADRs/0007-workflow-dispatch-for-cross-repo-dispatch.md)). +- Shim workflow security: `pull_request_target` prevents PR authors from modifying the shim to exfiltrate the dispatch token ([ADR 0008](ADRs/0008-pull-request-target-in-shim-workflows.md)). + **Open questions:** - Do we adopt a 3rd party platform, use existing internal infrastructure, or build our own? (See [agent-infrastructure.md](problems/agent-infrastructure.md) for the three directions.) @@ -68,12 +75,16 @@ The system that gives agents credentials to act on external services. Responsibl Identity is not the same as trust. An agent's identity lets it authenticate to external services; the trust model is defined by repository permissions and CODEOWNERS, not by which credentials the agent holds. (See [agent-architecture.md](problems/agent-architecture.md) — "trust derives from repository permissions, not agent identity.") +**Decided:** + +- Per-role GitHub Apps with manifest-based creation. Each agent role gets its own app with scoped permissions. PEMs stored as repo secrets on `.fullsend` ([ADR 0006](ADRs/0006-per-role-github-apps.md)). + **Open questions:** -- What identity model fits best — separate bot accounts per agent role, a single bot account with role metadata, GitHub App installations, or something else? (See [agent-architecture.md](problems/agent-architecture.md).) -- How are credentials scoped so that agents only get the permissions they need? +- ~~What identity model fits best — separate bot accounts per agent role, a single bot account with role metadata, GitHub App installations, or something else?~~ Decided in [ADR 0006](ADRs/0006-per-role-github-apps.md). - How are credentials rotated and revoked, and who has authority to do that? - Does the identity provider integrate with existing secrets management, or is it a new system? +- How will per-role identity work on GitLab and Forgejo, which lack GitHub's app manifest flow? ## Work Coordinator diff --git a/docs/problems/agent-architecture.md b/docs/problems/agent-architecture.md index ed4c05ffba..dd50abeb97 100644 --- a/docs/problems/agent-architecture.md +++ b/docs/problems/agent-architecture.md @@ -192,7 +192,7 @@ Without a coordinator, what happens when agents disagree? (e.g., correctness age - Should agents be stateless (fresh context per task) or stateful (accumulated knowledge of the codebase)? Stateless is safer (no poisoned state persists) but less efficient. - Should there be one instance of each agent type per repo, per org, or shared? Per-repo is simpler but more expensive. Shared agents need careful isolation. (Infrastructure constrains this — see [agent-infrastructure.md](agent-infrastructure.md).) -- What's the right model for agent identity? Agents need GitHub accounts to post comments and status checks. Separate bot accounts per agent role? A single bot account with role indicated in the comment? GitHub App installations? +- ~~What's the right model for agent identity? Agents need GitHub accounts to post comments and status checks. Separate bot accounts per agent role? A single bot account with role indicated in the comment? GitHub App installations?~~ Decided in [ADR 0006](../ADRs/0006-per-role-github-apps.md): per-role GitHub Apps with manifest-based creation. - How do we test the interaction model? Can we simulate adversarial scenarios (injection attempts, unauthorized changes, agent disagreements) in a sandbox repo? - How does the two-phase review model work in practice? Does the implementation agent run all six sub-agents locally, or a subset? Is the pre-PR review a lighter version? (Depends on [agent-infrastructure.md](agent-infrastructure.md) — what compute is available where.) - What's the iteration limit before human escalation? Too low and humans get pulled in constantly. Too high and the system wastes resources on unresolvable conflicts. diff --git a/docs/superpowers/plans/2026-04-02-admin-cli.md b/docs/superpowers/plans/2026-04-02-admin-cli.md deleted file mode 100644 index 9fd9284d24..0000000000 --- a/docs/superpowers/plans/2026-04-02-admin-cli.md +++ /dev/null @@ -1,1051 +0,0 @@ -# Admin CLI Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Build the `fullsend admin` CLI with `install`, `uninstall`, and `analyze` subcommands for managing fullsend in GitHub organizations, with a clean forge abstraction layer that can support GitHub, GitLab, and Forgejo. - -**Architecture:** The CLI uses Cobra for command structure. All git forge interactions go through a `forge.Client` interface with a GitHub implementation. Installation/uninstallation is modeled as ordered layers, each representing a discrete concern (config repo, agent apps, secrets, workflows, repo enrollment). An `analyze` command uses the same layer model to assess current state. - -**Tech Stack:** Go 1.26, Cobra (CLI), lipgloss (terminal UI), testify (testing), gopkg.in/yaml.v3 (config), nacl/box (secret encryption) - ---- - -## File Structure - -``` -cmd/fullsend/main.go # CLI entry point -internal/ - cli/ - root.go # Root cobra command - root_test.go # Root command tests - admin.go # `admin` subcommand group - admin_test.go # Admin subcommand tests - forge/ - forge.go # Client interface + domain types - fake.go # Thread-safe test double - fake_test.go # Fake client tests - github/ - github.go # GitHub REST API client - github_test.go # GitHub client tests (httptest) - types.go # GitHub App config, permissions - types_test.go # Types tests - config/ - config.go # OrgConfig types, YAML, validation - config_test.go # Config tests - layers/ - layers.go # Layer model: ordered install/uninstall/analyze - layers_test.go # Layer model tests - configrepo.go # Layer: .fullsend config repo - configrepo_test.go # Config repo layer tests - agentapps.go # Layer: GitHub App setup - agentapps_test.go # Agent apps layer tests - secrets.go # Layer: secrets + variables - secrets_test.go # Secrets layer tests - workflows.go # Layer: workflow files - workflows_test.go # Workflow layer tests - enrollment.go # Layer: repo enrollment - enrollment_test.go # Enrollment layer tests - appsetup/ - appsetup.go # GitHub App manifest flow - appsetup_test.go # App setup tests - ui/ - ui.go # Styled terminal output - ui_test.go # UI tests -go.mod -go.sum -.golangci.yml -Makefile # (modified: add Go targets) -``` - ---- - -### Task 1: Go Module, Makefile, and Linter Config - -**Files:** -- Create: `go.mod` -- Create: `.golangci.yml` -- Modify: `Makefile` -- Create: `cmd/fullsend/main.go` - -- [ ] **Step 1: Initialize Go module** - -```bash -go mod init github.com/fullsend-ai/fullsend -``` - -- [ ] **Step 2: Create .golangci.yml** - -Create `.golangci.yml` with: -```yaml -run: - timeout: 5m - -linters: - enable: - - errcheck - - govet - - staticcheck - - unused - - gosimple - - ineffassign - -linters-settings: - errcheck: - check-type-assertions: true -``` - -- [ ] **Step 3: Create minimal main.go** - -Create `cmd/fullsend/main.go`: -```go -package main - -import ( - "fmt" - "os" - - "github.com/fullsend-ai/fullsend/internal/cli" -) - -func main() { - if err := cli.Execute(); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) - } -} -``` - -- [ ] **Step 4: Add Go targets to Makefile** - -Add these targets to the existing Makefile (do not remove existing targets): -```makefile -# Go targets -GO_MODULE := github.com/fullsend-ai/fullsend -GO_BINARY := bin/fullsend -GO_LDFLAGS := -ldflags "-X $(GO_MODULE)/internal/cli.version=$$(git describe --tags --always --dirty 2>/dev/null || echo dev)" - -.PHONY: go-build go-test go-lint go-fmt go-vet go-tidy - -go-build: - go build $(GO_LDFLAGS) -o $(GO_BINARY) ./cmd/fullsend/ - -go-test: - go test ./... -count=1 -cover -race - -go-lint: - golangci-lint run ./... - -go-fmt: - gofmt -w -s cmd/ internal/ - -go-vet: - go vet ./... - -go-tidy: - go mod tidy -``` - -Also update the `lint` target to include Go linting: -```makefile -lint: check go-lint go-vet lint-adr-status lint-adr-numbers lint-adr-frontmatter -``` - -And update the `.PHONY` line and `help` target to include Go targets. - -- [ ] **Step 5: Commit** - -```bash -git add go.mod .golangci.yml cmd/fullsend/main.go Makefile -git commit -m "feat: initialize Go module and build infrastructure - -Assisted-by: OpenCode claude-opus-4-6@default" -``` - ---- - -### Task 2: UI Package - -**Files:** -- Create: `internal/ui/ui.go` -- Create: `internal/ui/ui_test.go` - -- [ ] **Step 1: Write tests for UI printer** - -Create `internal/ui/ui_test.go` with tests: -- TestBanner: verify output contains "fullsend" -- TestHeader: verify styled header with arrow prefix -- TestStepStart/StepDone/StepFail/StepWarn: verify correct prefix symbols (•, ✓, ✗, !) -- TestStepInfo: verify indented muted output -- TestKeyValue: verify "key: value" format -- TestSummary: verify box rendering with title and items -- TestErrorBox: verify error box rendering -- TestBlank: verify empty line output - -- [ ] **Step 2: Run tests to verify they fail** - -```bash -go test ./internal/ui/... -v -``` -Expected: Compilation error — package doesn't exist yet. - -- [ ] **Step 3: Implement UI printer** - -Create `internal/ui/ui.go` with: -- A `Printer` struct wrapping `io.Writer` -- `New(w io.Writer) *Printer` constructor -- Color constants: Brand (#7C3AED), Success (#10B981), Warning (#F59E0B), Error (#EF4444), Muted (#6B7280), Info (#3B82F6) -- Methods: `Banner()`, `Header(text)`, `StepStart(text)`, `StepDone(text)`, `StepFail(text)`, `StepWarn(text)`, `StepInfo(text)`, `KeyValue(key, value)`, `Summary(title, items)`, `ErrorBox(title, detail)`, `Blank()`, `PRLink(repo, url)` -- Use lipgloss for styling - -- [ ] **Step 4: Run tests to verify they pass** - -```bash -go test ./internal/ui/... -v -``` - -- [ ] **Step 5: Commit** - -```bash -git add internal/ui/ -git commit -m "feat: add terminal UI package with styled output - -Assisted-by: OpenCode claude-opus-4-6@default" -``` - ---- - -### Task 3: Forge Interface and Fake Client - -**Files:** -- Create: `internal/forge/forge.go` -- Create: `internal/forge/fake.go` -- Create: `internal/forge/fake_test.go` - -- [ ] **Step 1: Write the forge interface** - -Create `internal/forge/forge.go` with: - -Domain types: -```go -type Repository struct { - Name string - FullName string - DefaultBranch string - Private bool - Archived bool - Fork bool -} - -type ChangeProposal struct { - URL string - Title string - Number int -} - -type WorkflowRun struct { - ID int - Name string - Status string - Conclusion string - HTMLURL string - CreatedAt string -} -``` - -Client interface: -```go -type Client interface { - // Repository operations - ListOrgRepos(ctx context.Context, org string) ([]Repository, error) - CreateRepo(ctx context.Context, org, name, description string, private bool) (*Repository, error) - DeleteRepo(ctx context.Context, owner, repo string) error - - // File operations - CreateFile(ctx context.Context, owner, repo, path, message string, content []byte) error - CreateOrUpdateFile(ctx context.Context, owner, repo, path, message string, content []byte) error - GetFileContent(ctx context.Context, owner, repo, path string) ([]byte, error) - - // Branch operations - CreateBranch(ctx context.Context, owner, repo, branchName string) error - CreateFileOnBranch(ctx context.Context, owner, repo, branch, path, message string, content []byte) error - - // Change proposals (PRs/MRs) - CreateChangeProposal(ctx context.Context, owner, repo, title, body, head, base string) (*ChangeProposal, error) - ListRepoPullRequests(ctx context.Context, owner, repo string) ([]ChangeProposal, error) - - // Authentication - GetAuthenticatedUser(ctx context.Context) (string, error) - - // Secrets and variables - CreateRepoSecret(ctx context.Context, owner, repo, name, value string) error - RepoSecretExists(ctx context.Context, owner, repo, name string) (bool, error) - CreateOrUpdateRepoVariable(ctx context.Context, owner, repo, name, value string) error - - // CI/Workflow operations - GetLatestWorkflowRun(ctx context.Context, owner, repo, workflowFile string) (*WorkflowRun, error) - GetWorkflowRun(ctx context.Context, owner, repo string, runID int) (*WorkflowRun, error) - - // Forge-specific operations (for app setup, etc.) - ListOrgInstallations(ctx context.Context, org string) ([]Installation, error) -} - -type Installation struct { - ID int - AppID int - AppSlug string -} -``` - -- [ ] **Step 2: Write fake client tests** - -Create `internal/forge/fake_test.go` with tests: -- TestFakeListOrgRepos: returns pre-populated repos -- TestFakeCreateRepo: records the call and returns repo -- TestFakeCreateFile: records the call -- TestFakeErrors: injected errors are returned -- TestFakeThreadSafety: concurrent access is safe - -- [ ] **Step 3: Implement fake client** - -Create `internal/forge/fake.go` with: -- `FakeClient` struct with fields: `Repos`, `Errors`, `CreatedRepos`, `CreatedFiles`, `CreatedBranches`, `CreatedProposals`, `DeletedRepos`, `CreatedSecrets`, `Variables`, `WorkflowRuns`, `FileContents`, `AuthenticatedUser`, `Installations` -- `sync.Mutex` for thread safety -- All methods from Client interface implemented as recorders with error injection - -- [ ] **Step 4: Run tests** - -```bash -go test ./internal/forge/... -v -``` - -- [ ] **Step 5: Commit** - -```bash -git add internal/forge/ -git commit -m "feat: add forge interface and thread-safe fake client - -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" -``` - ---- - -### Task 4: GitHub Client Implementation - -**Files:** -- Create: `internal/forge/github/github.go` -- Create: `internal/forge/github/github_test.go` -- Create: `internal/forge/github/types.go` -- Create: `internal/forge/github/types_test.go` - -- [ ] **Step 1: Write types and their tests** - -Create `internal/forge/github/types.go` with: -- `AppPermissions` struct (Issues, PullRequests, Checks, Contents, Administration, Members string fields) -- `AppConfig` struct (Name, Description, URL string; Permissions AppPermissions; Events []string) -- `AgentAppConfig(org, role string) AppConfig` — returns role-specific app config: - - "fullsend": admin app with contents:write, issues:read, pull_requests:write, checks:read, administration:write, members:read - - "triage": issues:write - - "coder": issues:read, contents:write, pull_requests:write, checks:read - - "review": pull_requests:write, contents:read, checks:read - - default/unknown: issues:read -- `DefaultAgentRoles() []string` — returns ["fullsend", "triage", "coder", "review"] - -Create `internal/forge/github/types_test.go` with tests for `AgentAppConfig` covering each role and unknown roles, and test for `DefaultAgentRoles`. - -- [ ] **Step 2: Write GitHub client tests** - -Create `internal/forge/github/github_test.go` using `httptest.Server` to test: -- TestListOrgRepos: pagination, filtering archived/forked repos -- TestCreateRepo: correct API call with auto_init -- TestCreateFile: base64 encoding, correct path -- TestCreateOrUpdateFile: update path (gets existing SHA first) -- TestGetFileContent: base64 decoding -- TestDeleteRepo: correct DELETE call -- TestCreateBranch: gets default branch SHA, creates ref -- TestCreateChangeProposal: correct PR creation payload -- TestCreateRepoSecret: encryption via public key -- TestRepoSecretExists: 200 → true, 404 → false -- TestCreateOrUpdateRepoVariable: PATCH with 404 fallback to POST -- TestAPIError: non-2xx returns structured error - -- [ ] **Step 3: Implement GitHub client** - -Create `internal/forge/github/github.go` with: -- `LiveClient` struct with `http *http.Client`, `token string`, `baseURL string` -- `New(token string) *LiveClient` constructor (baseURL: "https://api.github.com", 30s timeout) -- `WithBaseURL(url string) *LiveClient` option for testing -- HTTP helpers: `get`, `post`, `put`, `patch`, `delete_` → all through `do()` which sets: - - `Authorization: Bearer {token}` - - `Accept: application/vnd.github+json` - - `X-GitHub-Api-Version: 2022-11-28` -- `apiError` type with StatusCode, Message, detailed Error -- All `forge.Client` interface methods implemented against GitHub REST API -- Pagination for ListOrgRepos (up to 100 pages) -- Secret encryption using nacl/box.SealAnonymous - -- [ ] **Step 4: Run tests** - -```bash -go test ./internal/forge/github/... -v -race -``` - -- [ ] **Step 5: Run go mod tidy** - -```bash -go mod tidy -``` - -- [ ] **Step 6: Commit** - -```bash -git add internal/forge/github/ go.mod go.sum -git commit -m "feat: add GitHub REST API client implementing forge.Client - -Implements all forge.Client methods against the GitHub REST API including -repo management, file operations, secret encryption, and workflow queries. - -Assisted-by: OpenCode claude-opus-4-6@default" -``` - ---- - -### Task 5: Config Package - -**Files:** -- Create: `internal/config/config.go` -- Create: `internal/config/config_test.go` - -- [ ] **Step 1: Write config tests** - -Create `internal/config/config_test.go` with tests: -- TestNewOrgConfig: creates config with repos, enabled repos, roles, agents -- TestOrgConfigMarshal: serializes to YAML with header comment -- TestOrgConfigValidate: valid configs pass; invalid version, platform, negative retries, invalid roles fail -- TestOrgConfigEnabledRepos: returns only enabled repos -- TestOrgConfigAgentSlugs: returns map of role→slug -- TestOrgConfigDefaultRoles: returns default role list -- TestOrgConfigValidRoles: returns valid roles list -- TestParseOrgConfig: parses YAML bytes into OrgConfig - -- [ ] **Step 2: Implement config package** - -Create `internal/config/config.go` with: -```go -type AgentEntry struct { - Role string `yaml:"role"` - Name string `yaml:"name"` - Slug string `yaml:"slug"` -} - -type DispatchConfig struct { - Platform string `yaml:"platform"` -} - -type RepoDefaults struct { - Roles []string `yaml:"roles"` - MaxImplementationRetries int `yaml:"max_implementation_retries"` - AutoMerge bool `yaml:"auto_merge"` -} - -type RepoConfig struct { - Roles []string `yaml:"roles,omitempty"` - Enabled bool `yaml:"enabled"` -} - -type OrgConfig struct { - Version string `yaml:"version"` - Dispatch DispatchConfig `yaml:"dispatch"` - Defaults RepoDefaults `yaml:"defaults"` - Agents []AgentEntry `yaml:"agents"` - Repos map[string]RepoConfig `yaml:"repos"` -} -``` - -Functions: -- `NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry) *OrgConfig` -- `ParseOrgConfig(data []byte) (*OrgConfig, error)` -- `(c *OrgConfig) Marshal() ([]byte, error)` — with header comment block -- `(c *OrgConfig) Validate() error` -- `(c *OrgConfig) EnabledRepos() []string` -- `(c *OrgConfig) AgentSlugs() map[string]string` -- `(c *OrgConfig) DefaultRoles() []string` -- `ValidRoles() []string` - -- [ ] **Step 3: Run tests** - -```bash -go test ./internal/config/... -v -``` - -- [ ] **Step 4: Commit** - -```bash -git add internal/config/ -git commit -m "feat: add config package for org-level configuration - -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" -``` - ---- - -### Task 6: Layer Model - -**Files:** -- Create: `internal/layers/layers.go` -- Create: `internal/layers/layers_test.go` - -This is the key architectural piece. The layer model represents installation as a stack of ordered layers. Each layer knows how to: -- `Install`: create/configure its concern -- `Uninstall`: tear down its concern -- `Analyze`: assess current state and report what would change - -- [ ] **Step 1: Write layer model tests** - -Create `internal/layers/layers_test.go` with tests: -- TestLayerOrdering: layers are processed in defined order for install, reverse for uninstall -- TestAnalyzeAllLayers: runs analyze on each layer and collects results -- TestInstallAllLayers: runs install on each layer in order, stops on error -- TestUninstallAllLayers: runs uninstall in reverse order -- TestLayerStatus: each status type (Installed, NotInstalled, Degraded, Unknown) works correctly - -- [ ] **Step 2: Implement layer model** - -Create `internal/layers/layers.go` with: - -```go -// LayerStatus represents the current state of a layer. -type LayerStatus int - -const ( - StatusNotInstalled LayerStatus = iota - StatusInstalled - StatusDegraded // partially installed or misconfigured - StatusUnknown // cannot determine -) - -func (s LayerStatus) String() string // "not installed", "installed", "degraded", "unknown" - -// LayerReport is what analyze returns for a single layer. -type LayerReport struct { - Name string - Status LayerStatus - Details []string // human-readable detail lines - WouldInstall []string // what install would do - WouldFix []string // what install would fix (for degraded) -} - -// Layer is the interface each installation concern implements. -type Layer interface { - Name() string - Install(ctx context.Context) error - Uninstall(ctx context.Context) error - Analyze(ctx context.Context) (*LayerReport, error) -} - -// Stack is an ordered collection of layers. -type Stack struct { - layers []Layer -} - -func NewStack(layers ...Layer) *Stack - -// InstallAll runs Install on each layer in order. Stops on first error. -func (s *Stack) InstallAll(ctx context.Context) error - -// UninstallAll runs Uninstall on each layer in reverse order. Collects all errors. -func (s *Stack) UninstallAll(ctx context.Context) []error - -// AnalyzeAll runs Analyze on each layer and returns reports. -func (s *Stack) AnalyzeAll(ctx context.Context) ([]*LayerReport, error) -``` - -- [ ] **Step 3: Run tests** - -```bash -go test ./internal/layers/... -v -``` - -- [ ] **Step 4: Commit** - -```bash -git add internal/layers/layers.go internal/layers/layers_test.go -git commit -m "feat: add layer model for ordered install/uninstall/analyze - -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" -``` - ---- - -### Task 7: Config Repo Layer - -**Files:** -- Create: `internal/layers/configrepo.go` -- Create: `internal/layers/configrepo_test.go` - -- [ ] **Step 1: Write tests** - -Tests for ConfigRepoLayer covering: -- TestConfigRepoInstall_CreatesRepo: creates .fullsend repo when it doesn't exist -- TestConfigRepoInstall_AlreadyExists: skips creation when repo exists -- TestConfigRepoInstall_WritesConfigYaml: writes config.yaml file -- TestConfigRepoUninstall_DeletesRepo: deletes .fullsend repo -- TestConfigRepoAnalyze_NotInstalled: reports not installed when repo missing -- TestConfigRepoAnalyze_Installed: reports installed when repo and config exist -- TestConfigRepoAnalyze_Degraded: reports degraded when repo exists but config missing - -- [ ] **Step 2: Implement ConfigRepoLayer** - -```go -type ConfigRepoLayer struct { - org string - client forge.Client - config *config.OrgConfig - ui *ui.Printer - hasPrivate bool // whether org has private repos (affects repo visibility) -} - -func NewConfigRepoLayer(org string, client forge.Client, cfg *config.OrgConfig, printer *ui.Printer, hasPrivate bool) *ConfigRepoLayer -``` - -- `Name()` → "config-repo" -- `Install()` → create .fullsend repo if missing, write config.yaml with retry -- `Uninstall()` → delete .fullsend repo -- `Analyze()` → check if repo exists, check if config.yaml exists and is valid - -- [ ] **Step 3: Run tests** - -```bash -go test ./internal/layers/... -v -run ConfigRepo -``` - -- [ ] **Step 4: Commit** - -```bash -git add internal/layers/configrepo.go internal/layers/configrepo_test.go -git commit -m "feat: add config repo layer for .fullsend repo management - -Handles creation, configuration, and teardown of the org-level -.fullsend configuration repository. - -Assisted-by: OpenCode claude-opus-4-6@default" -``` - ---- - -### Task 8: Workflow Files Layer - -**Files:** -- Create: `internal/layers/workflows.go` -- Create: `internal/layers/workflows_test.go` - -- [ ] **Step 1: Write tests** - -Tests for WorkflowsLayer: -- TestWorkflowsInstall_WritesAgentWorkflow: creates .github/workflows/agent.yaml -- TestWorkflowsInstall_WritesOnboardWorkflow: creates .github/workflows/repo-onboard.yaml -- TestWorkflowsInstall_WritesCODEOWNERS: writes CODEOWNERS file -- TestWorkflowsUninstall_Noop: workflow files are cleaned up with the repo (no individual deletion needed) -- TestWorkflowsAnalyze_AllPresent: reports installed when all workflow files exist -- TestWorkflowsAnalyze_Missing: reports not installed when workflows missing -- TestWorkflowsAnalyze_Partial: reports degraded when some files exist - -- [ ] **Step 2: Implement WorkflowsLayer** - -```go -type WorkflowsLayer struct { - org string - client forge.Client - ui *ui.Printer - authenticatedUser string -} - -func NewWorkflowsLayer(org string, client forge.Client, printer *ui.Printer, user string) *WorkflowsLayer -``` - -- `Name()` → "workflows" -- `Install()` → write agent.yaml, repo-onboard.yaml, and CODEOWNERS to .fullsend repo -- `Uninstall()` → noop (files go away with repo) -- `Analyze()` → check for existence of workflow files - -The reusable workflow (`agent.yaml`) template: `workflow_call` with `event_type` and `event_payload` inputs, `APP_PRIVATE_KEY` secret. - -The onboarding workflow (`repo-onboard.yaml`) template: triggers on push to main when config.yaml changes, reads enabled repos, creates enrollment PRs. - -- [ ] **Step 3: Run tests** - -```bash -go test ./internal/layers/... -v -run Workflows -``` - -- [ ] **Step 4: Commit** - -```bash -git add internal/layers/workflows.go internal/layers/workflows_test.go -git commit -m "feat: add workflows layer for CI workflow file management - -Manages reusable agent dispatch workflow, onboarding workflow, and -CODEOWNERS in the .fullsend config repo. - -Assisted-by: OpenCode claude-opus-4-6@default" -``` - ---- - -### Task 9: Secrets Layer - -**Files:** -- Create: `internal/layers/secrets.go` -- Create: `internal/layers/secrets_test.go` - -- [ ] **Step 1: Write tests** - -Tests for SecretsLayer: -- TestSecretsInstall_StoresPrivateKeys: creates repo secrets for each agent with PEM -- TestSecretsInstall_StoresAppIDs: creates repo variables for each agent app ID -- TestSecretsInstall_SkipsEmptyPEM: skips agents without PEM keys -- TestSecretsUninstall_Noop: secrets go away with repo deletion -- TestSecretsAnalyze_AllPresent: reports installed when all secrets exist -- TestSecretsAnalyze_Missing: reports not installed when secrets missing -- TestSecretsAnalyze_Partial: reports degraded when some secrets exist - -- [ ] **Step 2: Implement SecretsLayer** - -```go -type AgentCredentials struct { - config.AgentEntry - PEM string - AppID int -} - -type SecretsLayer struct { - org string - client forge.Client - agents []AgentCredentials - ui *ui.Printer -} - -func NewSecretsLayer(org string, client forge.Client, agents []AgentCredentials, printer *ui.Printer) *SecretsLayer -``` - -- `Name()` → "secrets" -- `Install()` → for each agent with PEM: create FULLSEND_{ROLE}_APP_PRIVATE_KEY secret, create FULLSEND_{ROLE}_APP_ID variable -- `Uninstall()` → noop (secrets go with repo) -- `Analyze()` → check if each expected secret exists via RepoSecretExists - -- [ ] **Step 3: Run tests** - -```bash -go test ./internal/layers/... -v -run Secrets -``` - -- [ ] **Step 4: Commit** - -```bash -git add internal/layers/secrets.go internal/layers/secrets_test.go -git commit -m "feat: add secrets layer for agent credential management - -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" -``` - ---- - -### Task 10: Enrollment Layer - -**Files:** -- Create: `internal/layers/enrollment.go` -- Create: `internal/layers/enrollment_test.go` - -- [ ] **Step 1: Write tests** - -Tests for EnrollmentLayer: -- TestEnrollmentInstall_CreatesShimWorkflow: creates enrollment PRs for enabled repos -- TestEnrollmentInstall_SkipsAlreadyEnrolled: skips repos that already have fullsend.yaml -- TestEnrollmentUninstall_Noop: enrollment is informational only -- TestEnrollmentAnalyze_ShowsEnrolledRepos: reports which repos are enrolled -- TestEnrollmentAnalyze_ShowsUnenrolledRepos: reports which enabled repos lack enrollment - -- [ ] **Step 2: Implement EnrollmentLayer** - -```go -type EnrollmentLayer struct { - org string - client forge.Client - enabledRepos []string - defaultBranches map[string]string - ui *ui.Printer -} - -func NewEnrollmentLayer(org string, client forge.Client, enabledRepos []string, defaultBranches map[string]string, printer *ui.Printer) *EnrollmentLayer -``` - -- `Name()` → "enrollment" -- `Install()` → for each enabled repo: check if .github/workflows/fullsend.yaml exists, if not create branch + shim workflow file + PR -- `Uninstall()` → noop (individual repo cleanup is manual) -- `Analyze()` → check each enabled repo for fullsend.yaml, report enrollment status - -The shim workflow: triggers on issues, issue_comment, pull_request, pull_request_review; calls reusable workflow in .fullsend repo. - -- [ ] **Step 3: Run tests** - -```bash -go test ./internal/layers/... -v -run Enrollment -``` - -- [ ] **Step 4: Commit** - -```bash -git add internal/layers/enrollment.go internal/layers/enrollment_test.go -git commit -m "feat: add enrollment layer for repo onboarding - -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" -``` - ---- - -### Task 11: App Setup Package - -**Files:** -- Create: `internal/appsetup/appsetup.go` -- Create: `internal/appsetup/appsetup_test.go` - -- [ ] **Step 1: Write tests** - -Tests for app setup: -- TestSetup_ExistingAppWithSecret: finds existing app, secret exists → reuse -- TestSetup_ExistingAppNoSecret: finds existing app, no secret → error directing user to delete -- TestSetup_ManifestFlow: no existing app → runs manifest flow, returns credentials -- TestSetup_EnsureInstalled: checks installation, opens browser if needed -- TestExpectedAppSlug: naming convention tests for each role - -- [ ] **Step 2: Implement app setup** - -Create `internal/appsetup/appsetup.go` with: - -```go -type AppCredentials struct { - ID int - Slug string - Name string - PEM string - ClientID string - ClientSecret string - WebhookSecret *string - HTMLURL string -} - -type Prompter interface { - WaitForEnter(prompt string) error - Confirm(prompt string) (bool, error) -} - -type BrowserOpener interface { - Open(ctx context.Context, url string) error -} - -type SecretExistsFunc func(role string) (bool, error) - -type Setup struct { - client forge.Client - prompter Prompter - browser BrowserOpener - ui *ui.Printer - knownSlugs map[string]string - secretExists SecretExistsFunc -} - -func NewSetup(client forge.Client, prompter Prompter, browser BrowserOpener, printer *ui.Printer) *Setup -func (s *Setup) WithKnownSlugs(slugs map[string]string) *Setup -func (s *Setup) WithSecretExists(fn SecretExistsFunc) *Setup - -func (s *Setup) Run(ctx context.Context, org, role string) (*AppCredentials, error) -``` - -The manifest flow: -1. Check for existing app via ListOrgInstallations, match by slug convention -2. If found: check if PEM secret exists → reuse or error -3. If not found: start local HTTP server, serve manifest form, catch callback, exchange code -4. Ensure app is installed on org - -- [ ] **Step 3: Run tests** - -```bash -go test ./internal/appsetup/... -v -``` - -- [ ] **Step 4: Commit** - -```bash -git add internal/appsetup/ -git commit -m "feat: add GitHub App manifest flow for agent app setup - -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" -``` - ---- - -### Task 12: CLI Commands — Root and Admin - -**Files:** -- Create: `internal/cli/root.go` -- Create: `internal/cli/root_test.go` -- Create: `internal/cli/admin.go` -- Create: `internal/cli/admin_test.go` - -- [ ] **Step 1: Write tests** - -Tests for root command: -- TestRootCommand_HasVersion: version flag works -- TestRootCommand_HasAdminSubcommand: admin subcommand is registered - -Tests for admin command: -- TestAdminCommand_HasInstall: install subcommand exists -- TestAdminCommand_HasUninstall: uninstall subcommand exists -- TestAdminCommand_HasAnalyze: analyze subcommand exists -- TestAdminInstall_RequiresOrg: fails without org argument -- TestAdminInstall_Flags: --repo, --agents, --dry-run, --skip-app-setup flags exist -- TestAdminUninstall_RequiresOrg: fails without org argument -- TestAdminUninstall_Flags: --yolo flag exists -- TestAdminAnalyze_RequiresOrg: fails without org argument - -- [ ] **Step 2: Implement root command** - -Create `internal/cli/root.go`: -```go -var version = "dev" - -func newRootCmd() *cobra.Command { - cmd := &cobra.Command{ - Use: "fullsend", - Short: "Autonomous agentic development for GitHub organizations", - SilenceUsage: true, - SilenceErrors: true, - Version: version, - } - cmd.AddCommand(newAdminCmd()) - return cmd -} - -func Execute() error { return newRootCmd().Execute() } -``` - -- [ ] **Step 3: Implement admin command with subcommands** - -Create `internal/cli/admin.go` with: -- `newAdminCmd()` → groups install, uninstall, analyze -- `newInstallCmd()` → `fullsend admin install ` with flags: --repo (repeatable), --agents (comma-sep, default: fullsend,triage,coder,review), --dry-run, --skip-app-setup -- `newUninstallCmd()` → `fullsend admin uninstall ` with flag: --yolo -- `newAnalyzeCmd()` → `fullsend admin analyze ` - -Token resolution function: -1. `GH_TOKEN` env var -2. `GITHUB_TOKEN` env var -3. `gh auth token` subprocess - -Install command flow: -1. Resolve token -2. Create forge client -3. Build layer stack -4. If dry-run: run analyze and print -5. If not skip-app-setup: run app setup for each role -6. Run install on layer stack - -Uninstall command flow: -1. Resolve token -2. Create forge client -3. Build layer stack -4. Confirm (unless --yolo) -5. Run uninstall on layer stack - -Analyze command flow: -1. Resolve token -2. Create forge client -3. Build layer stack -4. Run analyze and print reports - -- [ ] **Step 4: Run tests** - -```bash -go test ./internal/cli/... -v -``` - -- [ ] **Step 5: Ensure go mod tidy and go vet pass** - -```bash -go mod tidy && go vet ./... -``` - -- [ ] **Step 6: Commit** - -```bash -git add internal/cli/ go.mod go.sum -git commit -m "feat: add CLI with admin install/uninstall/analyze subcommands - -Implements fullsend admin {install,uninstall,analyze} with -layer-based installation model and forge-agnostic client interface. - -Assisted-by: OpenCode claude-opus-4-6@default" -``` - ---- - -### Task 13: Integration Test and Final Verification - -**Files:** -- Possibly modify any files with issues found during integration - -- [ ] **Step 1: Run full test suite** - -```bash -go test ./... -count=1 -cover -race -``` - -- [ ] **Step 2: Run linter** - -```bash -go vet ./... -``` - -- [ ] **Step 3: Run build** - -```bash -go build -o bin/fullsend ./cmd/fullsend/ -``` - -- [ ] **Step 4: Verify CLI help output** - -```bash -./bin/fullsend --help -./bin/fullsend admin --help -./bin/fullsend admin install --help -./bin/fullsend admin uninstall --help -./bin/fullsend admin analyze --help -``` - -- [ ] **Step 5: Fix any issues found** - -- [ ] **Step 6: Final commit if needed** - -```bash -git add -A -git commit -m "fix: address integration issues - -Assisted-by: OpenCode claude-opus-4-6@default" -``` diff --git a/e2e/admin/admin_test.go b/e2e/admin/admin_test.go new file mode 100644 index 0000000000..5e6c88875d --- /dev/null +++ b/e2e/admin/admin_test.go @@ -0,0 +1,462 @@ +//go:build e2e + +package admin + +import ( + "context" + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/google/uuid" + "github.com/playwright-community/playwright-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/appsetup" + "github.com/fullsend-ai/fullsend/internal/config" + "github.com/fullsend-ai/fullsend/internal/forge" + gh "github.com/fullsend-ai/fullsend/internal/forge/github" + "github.com/fullsend-ai/fullsend/internal/layers" + "github.com/fullsend-ai/fullsend/internal/ui" +) + +// e2eEnv holds the shared state for an e2e test run. +type e2eEnv struct { + cfg envConfig + page playwright.Page + client *gh.LiveClient + token string + printer *ui.Printer + runID string + screenshotDir string +} + +// setupE2ETest performs the common Playwright, login, PAT, lock, and cleanup +// steps. Returns the shared env. +func setupE2ETest(t *testing.T) *e2eEnv { + t.Helper() + if testing.Short() { + t.Skip("skipping e2e test in short mode") + } + + cfg := loadEnvConfig(t) + screenshotDir := "/workspaces/fullsend/.playwright" + _ = os.MkdirAll(screenshotDir, 0o755) + + // --- Playwright setup --- + pw, err := playwright.Run() + require.NoError(t, err, "starting Playwright") + t.Cleanup(func() { + if stopErr := pw.Stop(); stopErr != nil { + t.Logf("warning: could not stop Playwright: %v", stopErr) + } + }) + + browser, err := pw.Chromium.Launch(playwright.BrowserTypeLaunchOptions{ + Headless: playwright.Bool(os.Getenv("E2E_HEADED") != "true"), + }) + require.NoError(t, err, "launching Playwright browser") + t.Cleanup(func() { _ = browser.Close() }) + + browserCtx, err := browser.NewContext() + require.NoError(t, err, "creating browser context") + t.Cleanup(func() { _ = browserCtx.Close() }) + + page, err := browserCtx.NewPage() + require.NoError(t, err, "creating Playwright page") + + // Log into GitHub programmatically. + t.Log("Logging into GitHub...") + err = githubLogin(page, cfg.username, cfg.password, t.Logf) + require.NoError(t, err, "logging into GitHub") + t.Logf("Post-login URL: %s", page.URL()) + + // Generate a PAT for API access. + patNote := fmt.Sprintf("fullsend-e2e-%d", time.Now().Unix()) + t.Logf("Creating PAT: %s", patNote) + token, err := createPAT(page, patNote, t.Logf) + require.NoError(t, err, "creating PAT") + t.Cleanup(func() { + t.Log("Deleting PAT...") + if delErr := deletePAT(page, patNote, t.Logf); delErr != nil { + t.Logf("warning: could not delete PAT: %v", delErr) + } + }) + + // --- GitHub client --- + client := newLiveClient(token) + printer := ui.New(os.Stdout) + + // Acquire lock. + runID := uuid.New().String() + t.Logf("E2E run ID: %s", runID) + + err = acquireLock(context.Background(), client, token, testOrg, runID, cfg.lockTimeout, t.Logf) + require.NoError(t, err, "acquiring e2e lock") + t.Cleanup(func() { + releaseLock(context.Background(), client, testOrg, runID, t) + }) + + // Teardown-first cleanup. + cleanupStaleResources(context.Background(), client, page, token, screenshotDir, t) + + return &e2eEnv{ + cfg: cfg, + page: page, + client: client, + token: token, + printer: printer, + runID: runID, + screenshotDir: screenshotDir, + } +} + +func TestAdminInstallUninstall(t *testing.T) { + env := setupE2ETest(t) + ctx := context.Background() + + // ========================================= + // Phase 1: First install (creates resources) + // ========================================= + t.Log("=== Phase 1: First Install ===") + agentCreds, orgCfg, enabledRepos, defaultBranches, enrolledRepoIDs := runFullInstall(t, env) + verifyInstalled(t, env, orgCfg, enabledRepos, defaultBranches, agentCreds) + + // ========================================= + // Phase 2: Second install (idempotent no-op) + // ========================================= + t.Log("=== Phase 2: Second Install (idempotent) ===") + user, err := env.client.GetAuthenticatedUser(ctx) + require.NoError(t, err) + allRepos, err := env.client.ListOrgRepos(ctx, testOrg) + require.NoError(t, err) + hasPrivate := hasPrivateRepos(allRepos) + + // Second install should reuse existing dispatch token (empty string). + stack := buildTestLayerStack(testOrg, env.client, orgCfg, env.printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", enrolledRepoIDs) + err = stack.InstallAll(ctx) + require.NoError(t, err, "second InstallAll should succeed") + verifyInstalled(t, env, orgCfg, enabledRepos, defaultBranches, agentCreds) + + // ========================================= + // Phase 3: First uninstall (deletes resources) + // ========================================= + t.Log("=== Phase 3: First Uninstall ===") + runUninstall(t, env) + // Wait for repo deletion to propagate (GitHub returns 409 if checked too soon). + time.Sleep(5 * time.Second) + verifyNotInstalled(t, env) + + // ========================================= + // Phase 4: Second uninstall (idempotent no-op) + // ========================================= + t.Log("=== Phase 4: Second Uninstall (idempotent) ===") + runUninstallAllowNotFound(t, env) + time.Sleep(3 * time.Second) + verifyNotInstalled(t, env) + + t.Log("=== E2E test complete ===") +} + +// --- Install/uninstall helpers --- + +// runFullInstall executes the full install flow (app setup + layer stack install) +// and returns the agent credentials and org config for verification. +func runFullInstall(t *testing.T, env *e2eEnv) ([]layers.AgentCredentials, *config.OrgConfig, []string, map[string]string, []int64) { + t.Helper() + ctx := context.Background() + + // App setup via manifest flow with Playwright. + playwrightBrowser := NewPlaywrightBrowserOpener(env.page, t.Logf, env.screenshotDir) + prompter := AutoPrompter{} + setup := appsetup.NewSetup(env.client, prompter, playwrightBrowser, env.printer) + + var agentCreds []layers.AgentCredentials + for _, role := range defaultRoles { + t.Logf("Setting up app for role: %s", role) + appCreds, err := setup.Run(ctx, testOrg, role) + require.NoError(t, err, "setting up app for role %s", role) + + agentCreds = append(agentCreds, layers.AgentCredentials{ + AgentEntry: config.AgentEntry{ + Role: role, + Name: appCreds.Name, + Slug: appCreds.Slug, + }, + PEM: appCreds.PEM, + AppID: appCreds.AppID, + }) + + registerAppCleanup(t, env.page, appCreds.Slug, env.screenshotDir) + } + + // Discover repos and build config. + allRepos, err := env.client.ListOrgRepos(ctx, testOrg) + require.NoError(t, err, "listing org repos") + + repoNames := repoNameList(allRepos) + defaultBranches := repoDefaultBranches(allRepos) + hasPrivate := hasPrivateRepos(allRepos) + enabledRepos := []string{testRepo} + + agents := make([]config.AgentEntry, len(agentCreds)) + for i, ac := range agentCreds { + agents[i] = ac.AgentEntry + } + + orgCfg := config.NewOrgConfig(repoNames, enabledRepos, defaultRoles, agents) + + user, err := env.client.GetAuthenticatedUser(ctx) + require.NoError(t, err, "getting authenticated user") + + // Collect repo IDs for enrolled repos (needed by DispatchTokenLayer). + var enrolledRepoIDs []int64 + for _, repoName := range enabledRepos { + repo, repoErr := env.client.GetRepo(ctx, testOrg, repoName) + require.NoError(t, repoErr, "getting repo %s for ID", repoName) + enrolledRepoIDs = append(enrolledRepoIDs, repo.ID) + } + + // Install config-repo and workflows layers first so .fullsend repo exists. + // This mirrors the real CLI which creates the repo before prompting for + // the dispatch token (so the user can scope the fine-grained PAT to it). + configLayer := layers.NewConfigRepoLayer(testOrg, env.client, orgCfg, env.printer, hasPrivate) + err = configLayer.Install(ctx) + require.NoError(t, err, "pre-installing config-repo layer") + registerRepoCleanup(t, env.client, testOrg, forge.ConfigRepoName) + + workflowsLayer := layers.NewWorkflowsLayer(testOrg, env.client, env.printer, user) + err = workflowsLayer.Install(ctx) + require.NoError(t, err, "pre-installing workflows layer") + + // Create a fine-grained PAT for dispatch via Playwright. + // This mirrors the real CLI flow: the user creates a fine-grained PAT + // scoped to .fullsend with actions:write, then pastes it back. + t.Log("Creating fine-grained dispatch PAT via Playwright...") + dispatchToken, err := createDispatchPAT(env.page, testOrg, env.screenshotDir, t.Logf) + require.NoError(t, err, "creating dispatch PAT") + t.Cleanup(func() { + t.Log("Deleting dispatch PAT...") + if delErr := deleteDispatchPAT(env.page, testOrg, env.screenshotDir, t.Logf); delErr != nil { + t.Logf("warning: could not delete dispatch PAT: %v", delErr) + } + }) + + // Build full layer stack with the dispatch token and install all layers. + // Config-repo and workflows are idempotent, so re-running them is harmless. + stack := buildTestLayerStack(testOrg, env.client, orgCfg, env.printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, dispatchToken, enrolledRepoIDs) + + err = stack.InstallAll(ctx) + require.NoError(t, err, "installing layers") + + return agentCreds, orgCfg, enabledRepos, defaultBranches, enrolledRepoIDs +} + +func runUninstall(t *testing.T, env *e2eEnv) { + t.Helper() + emptyCfg := config.NewOrgConfig(nil, nil, nil, nil) + stack := layers.NewStack( + layers.NewConfigRepoLayer(testOrg, env.client, emptyCfg, env.printer, false), + layers.NewWorkflowsLayer(testOrg, env.client, env.printer, ""), + layers.NewSecretsLayer(testOrg, env.client, nil, env.printer), + layers.NewDispatchTokenLayer(testOrg, env.client, "", nil, env.printer), + layers.NewEnrollmentLayer(testOrg, env.client, nil, nil, env.printer), + ) + errs := stack.UninstallAll(context.Background()) + assert.Empty(t, errs, "uninstall should complete without errors") +} + +// runUninstallAllowNotFound runs uninstall but accepts not-found errors +// (expected when resources are already deleted). +func runUninstallAllowNotFound(t *testing.T, env *e2eEnv) { + t.Helper() + emptyCfg := config.NewOrgConfig(nil, nil, nil, nil) + stack := layers.NewStack( + layers.NewConfigRepoLayer(testOrg, env.client, emptyCfg, env.printer, false), + layers.NewWorkflowsLayer(testOrg, env.client, env.printer, ""), + layers.NewSecretsLayer(testOrg, env.client, nil, env.printer), + layers.NewDispatchTokenLayer(testOrg, env.client, "", nil, env.printer), + layers.NewEnrollmentLayer(testOrg, env.client, nil, nil, env.printer), + ) + errs := stack.UninstallAll(context.Background()) + for _, e := range errs { + if !forge.IsNotFound(e) { + t.Errorf("unexpected uninstall error (not a not-found): %v", e) + } + } +} + +// --- Verification helpers --- + +// verifyInstalled checks that all resources exist and analyze reports installed. +func verifyInstalled(t *testing.T, env *e2eEnv, orgCfg *config.OrgConfig, enabledRepos []string, defaultBranches map[string]string, agentCreds []layers.AgentCredentials) { + t.Helper() + ctx := context.Background() + + // .fullsend repo exists. + repo, err := env.client.GetRepo(ctx, testOrg, forge.ConfigRepoName) + require.NoError(t, err, ".fullsend repo should exist") + assert.Equal(t, forge.ConfigRepoName, repo.Name) + + // config.yaml exists and parses. + cfgData, err := env.client.GetFileContent(ctx, testOrg, forge.ConfigRepoName, "config.yaml") + require.NoError(t, err, "config.yaml should exist") + parsedCfg, err := config.ParseOrgConfig(cfgData) + require.NoError(t, err, "config.yaml should parse") + assert.Equal(t, "1", parsedCfg.Version) + assert.Len(t, parsedCfg.Agents, len(defaultRoles)) + + // Workflow files exist. + for _, path := range []string{ + ".github/workflows/agent.yaml", + ".github/workflows/repo-onboard.yaml", + "CODEOWNERS", + } { + _, err := env.client.GetFileContent(ctx, testOrg, forge.ConfigRepoName, path) + assert.NoError(t, err, "%s should exist in .fullsend", path) + } + + // Secrets and variables exist for each role. + for _, role := range defaultRoles { + secretName := fmt.Sprintf("FULLSEND_%s_APP_PRIVATE_KEY", strings.ToUpper(role)) + exists, err := env.client.RepoSecretExists(ctx, testOrg, forge.ConfigRepoName, secretName) + assert.NoError(t, err, "checking secret %s", secretName) + assert.True(t, exists, "secret %s should exist", secretName) + + varName := fmt.Sprintf("FULLSEND_%s_APP_ID", strings.ToUpper(role)) + exists, err = env.client.RepoVariableExists(ctx, testOrg, forge.ConfigRepoName, varName) + assert.NoError(t, err, "checking variable %s", varName) + assert.True(t, exists, "variable %s should exist", varName) + } + + // Dispatch token org secret exists. + dispatchExists, err := env.client.OrgSecretExists(ctx, testOrg, "FULLSEND_DISPATCH_TOKEN") + assert.NoError(t, err, "checking dispatch token org secret") + assert.True(t, dispatchExists, "FULLSEND_DISPATCH_TOKEN org secret should exist") + + // Enrollment PR exists for test-repo. + prs, err := env.client.ListRepoPullRequests(ctx, testOrg, testRepo) + require.NoError(t, err, "listing PRs for %s", testRepo) + found := false + for _, pr := range prs { + if strings.Contains(pr.Title, "fullsend") { + found = true + t.Logf("Found enrollment PR: %s", pr.URL) + break + } + } + assert.True(t, found, "enrollment PR should exist for %s", testRepo) + + // Analyze reports installed. + user, err := env.client.GetAuthenticatedUser(ctx) + require.NoError(t, err) + allRepos, err := env.client.ListOrgRepos(ctx, testOrg) + require.NoError(t, err) + hasPrivate := hasPrivateRepos(allRepos) + + analyzeStack := buildTestLayerStack(testOrg, env.client, orgCfg, env.printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", nil) + reports, err := analyzeStack.AnalyzeAll(ctx) + require.NoError(t, err, "analyzing layers") + for _, report := range reports { + if report.Name == "enrollment" { + // Enrollment creates a PR but doesn't merge it, so the shim + // workflow file doesn't exist on the default branch yet. + assert.Contains(t, []layers.LayerStatus{layers.StatusInstalled, layers.StatusNotInstalled}, + report.Status, "layer %s status: %s (details: %v)", + report.Name, report.Status, report.Details) + continue + } + assert.Equal(t, layers.StatusInstalled, report.Status, + "layer %s should be installed, got %s (details: %v)", + report.Name, report.Status, report.Details) + } +} + +// verifyNotInstalled checks that the config repo is gone and analyze reports +// not-installed for layers with concrete artifacts. +func verifyNotInstalled(t *testing.T, env *e2eEnv) { + t.Helper() + ctx := context.Background() + + _, err := env.client.GetRepo(ctx, testOrg, forge.ConfigRepoName) + assert.True(t, forge.IsNotFound(err), ".fullsend repo should be deleted") + + // Dispatch token org secret should be deleted. + dispatchExists, err := env.client.OrgSecretExists(ctx, testOrg, "FULLSEND_DISPATCH_TOKEN") + assert.NoError(t, err, "checking dispatch token after uninstall") + assert.False(t, dispatchExists, "FULLSEND_DISPATCH_TOKEN org secret should be deleted") + + emptyCfg := config.NewOrgConfig(nil, nil, nil, nil) + stack := layers.NewStack( + layers.NewConfigRepoLayer(testOrg, env.client, emptyCfg, env.printer, false), + layers.NewWorkflowsLayer(testOrg, env.client, env.printer, ""), + layers.NewSecretsLayer(testOrg, env.client, nil, env.printer), + layers.NewDispatchTokenLayer(testOrg, env.client, "", nil, env.printer), + layers.NewEnrollmentLayer(testOrg, env.client, nil, nil, env.printer), + ) + reports, err := stack.AnalyzeAll(ctx) + require.NoError(t, err, "analyzing layers after uninstall") + for _, report := range reports { + switch report.Name { + case "config-repo", "workflows", "dispatch-token": + assert.Equal(t, layers.StatusNotInstalled, report.Status, + "layer %s should be not-installed, got %s", + report.Name, report.Status) + default: + // Layers with empty config may report "installed" (nothing to track). + t.Logf("layer %s status: %s (accepted)", report.Name, report.Status) + } + } +} + +// --- Utility functions --- + +func buildTestLayerStack( + org string, + client forge.Client, + cfg *config.OrgConfig, + printer *ui.Printer, + user string, + hasPrivate bool, + enabledRepos []string, + defaultBranches map[string]string, + agentCreds []layers.AgentCredentials, + dispatchToken string, + enrolledRepoIDs []int64, +) *layers.Stack { + return layers.NewStack( + layers.NewConfigRepoLayer(org, client, cfg, printer, hasPrivate), + layers.NewWorkflowsLayer(org, client, printer, user), + layers.NewSecretsLayer(org, client, agentCreds, printer), + layers.NewDispatchTokenLayer(org, client, dispatchToken, enrolledRepoIDs, printer), + layers.NewEnrollmentLayer(org, client, enabledRepos, defaultBranches, printer), + ) +} + +func repoNameList(repos []forge.Repository) []string { + names := make([]string, len(repos)) + for i, r := range repos { + names[i] = r.Name + } + return names +} + +func repoDefaultBranches(repos []forge.Repository) map[string]string { + branches := make(map[string]string, len(repos)) + for _, r := range repos { + branches[r.Name] = r.DefaultBranch + } + return branches +} + +func hasPrivateRepos(repos []forge.Repository) bool { + for _, r := range repos { + if r.Private { + return true + } + } + return false +} diff --git a/e2e/admin/browser.go b/e2e/admin/browser.go new file mode 100644 index 0000000000..3b340c956a --- /dev/null +++ b/e2e/admin/browser.go @@ -0,0 +1,398 @@ +//go:build e2e + +package admin + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "time" + + xhtml "golang.org/x/net/html" + + "github.com/playwright-community/playwright-go" +) + +// PlaywrightBrowserOpener implements appsetup.BrowserOpener using a +// Playwright browser page with a pre-authenticated persistent context. +type PlaywrightBrowserOpener struct { + page playwright.Page + logf func(string, ...any) + screenshotDir string +} + +// NewPlaywrightBrowserOpener creates a new PlaywrightBrowserOpener +// using the given Playwright page. +func NewPlaywrightBrowserOpener(page playwright.Page, logf func(string, ...any), screenshotDir string) *PlaywrightBrowserOpener { + return &PlaywrightBrowserOpener{page: page, logf: logf, screenshotDir: screenshotDir} +} + +// Open navigates the Playwright page to the given URL and handles the +// expected interactions based on the page type. +func (b *PlaywrightBrowserOpener) Open(_ context.Context, url string) error { + b.logf("[browser] Open called with URL: %s", url) + + // Local manifest form — fetch via HTTP to avoid cross-origin SameSite + // cookie issues, then submit from within GitHub's origin. + if strings.Contains(url, "127.0.0.1") { + return b.handleLocalFormSubmission(url) + } + + if _, err := b.page.Goto(url, playwright.PageGotoOptions{ + WaitUntil: playwright.WaitUntilStateDomcontentloaded, + Timeout: playwright.Float(10000), + }); err != nil { + saveDebugScreenshot(b.page, b.screenshotDir, "browser-goto-failed", b.logf) + return fmt.Errorf("navigating to %s: %w", url, err) + } + + pageURL := b.page.URL() + b.logf("[browser] After Goto, page URL: %s", pageURL) + + switch { + case strings.Contains(pageURL, "/settings/apps/new"), + strings.Contains(pageURL, "/settings/apps/manifest"): + return b.handleCreateAppPage() + case strings.Contains(pageURL, "/installations/new"): + return b.handleInstallAppPage() + default: + saveDebugScreenshot(b.page, b.screenshotDir, "browser-unexpected-url", b.logf) + return fmt.Errorf("unexpected URL: %s", pageURL) + } +} + +// handleLocalFormSubmission fetches the local form via HTTP, extracts the +// manifest (which already contains redirect_url), then submits from +// GitHub's origin so that session cookies (SameSite=Lax) are included +// in the POST. +func (b *PlaywrightBrowserOpener) handleLocalFormSubmission(localURL string) error { + httpClient := &http.Client{Timeout: 10 * time.Second} + resp, err := httpClient.Get(localURL) + if err != nil { + return fmt.Errorf("fetching local form page: %w", err) + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("reading local form page: %w", err) + } + content := string(body) + + // Extract manifest and form action from the hidden inputs. + // The v6 manifest flow puts redirect_url inside the manifest JSON, + // not as a separate form field. + manifest, err := extractInputValue(content, "manifest") + if err != nil { + return fmt.Errorf("extracting manifest from form: %w", err) + } + actionURL, err := extractFormAction(content) + if err != nil { + return fmt.Errorf("extracting form action: %w", err) + } + + // Ensure hook_attributes exists in the manifest (GitHub requires it). + var manifestMap map[string]any + if jsonErr := json.Unmarshal([]byte(manifest), &manifestMap); jsonErr != nil { + return fmt.Errorf("parsing manifest JSON: %w", jsonErr) + } + if _, ok := manifestMap["hook_attributes"]; !ok { + manifestMap["hook_attributes"] = map[string]any{ + "url": "https://example.com/webhook", + "active": false, + } + patched, jsonErr := json.Marshal(manifestMap) + if jsonErr != nil { + return fmt.Errorf("re-marshaling manifest: %w", jsonErr) + } + manifest = string(patched) + } + + b.logf("[browser] Extracted manifest (%d bytes), action=%s", len(manifest), actionURL) + + // Navigate to a neutral GitHub page first so we're on the same + // origin and session cookies will be sent with the POST. + if _, err := b.page.Goto("https://github.com/settings", playwright.PageGotoOptions{ + WaitUntil: playwright.WaitUntilStateDomcontentloaded, + Timeout: playwright.Float(10000), + }); err != nil { + b.logf("[browser] Warning: pre-navigate to GitHub settings failed: %v", err) + } + + // Submit the form via JS, passing values as arguments to avoid + // any quoting/escaping issues with string interpolation. + _, err = b.page.Evaluate(`([action, manifest]) => { + const form = document.createElement('form'); + form.method = 'post'; + form.action = action; + const m = document.createElement('input'); + m.type = 'hidden'; m.name = 'manifest'; m.value = manifest; + form.appendChild(m); + document.body.appendChild(form); + form.submit(); + }`, []string{actionURL, manifest}) + if err != nil { + saveDebugScreenshot(b.page, b.screenshotDir, "browser-js-submit-failed", b.logf) + return fmt.Errorf("submitting manifest form via JS: %w", err) + } + + // Wait for navigation to the app creation confirmation page. + // GitHub redirects to /settings/apps/manifest or /settings/apps/new. + if err := b.page.WaitForURL("**/settings/apps/**", playwright.PageWaitForURLOptions{ + Timeout: playwright.Float(10000), + }); err != nil { + pageURL := b.page.URL() + if strings.Contains(pageURL, "/settings/apps/") { + // We're there. + } else if strings.Contains(pageURL, "/callback") { + return nil + } else { + saveDebugScreenshot(b.page, b.screenshotDir, "browser-manifest-redirect-failed", b.logf) + return fmt.Errorf("waiting for manifest page: %w (URL: %s)", err, pageURL) + } + } + + return b.handleCreateAppPage() +} + +// handleCreateAppPage clicks "Create GitHub App" on the confirmation page. +func (b *PlaywrightBrowserOpener) handleCreateAppPage() error { + b.logf("[browser] handleCreateAppPage at URL: %s", b.page.URL()) + + // The button text varies: "Create GitHub App" or "Create GitHub App for {org}". + btn := b.page.Locator("button:has-text('Create GitHub App'), input[type='submit'][value*='Create GitHub App']") + if err := btn.First().WaitFor(playwright.LocatorWaitForOptions{ + State: playwright.WaitForSelectorStateVisible, + Timeout: playwright.Float(5000), + }); err != nil { + saveDebugScreenshot(b.page, b.screenshotDir, "browser-create-btn-failed", b.logf) + return fmt.Errorf("waiting for 'Create GitHub App' button: %w", err) + } + if err := btn.First().Click(playwright.LocatorClickOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + saveDebugScreenshot(b.page, b.screenshotDir, "browser-create-btn-failed", b.logf) + return fmt.Errorf("clicking 'Create GitHub App': %w", err) + } + + // Wait for redirect back to our callback URL. + if err := b.page.WaitForURL("**/callback**", playwright.PageWaitForURLOptions{ + Timeout: playwright.Float(10000), + }); err != nil { + pageURL := b.page.URL() + if strings.Contains(pageURL, "/callback") || strings.Contains(pageURL, "127.0.0.1") { + return nil + } + saveDebugScreenshot(b.page, b.screenshotDir, "browser-callback-failed", b.logf) + return fmt.Errorf("waiting for callback: %w", err) + } + + return nil +} + +// handleInstallAppPage clicks "Install" on the GitHub App installation page. +// Retries navigation if the page 404s (GitHub needs time to provision the app). +func (b *PlaywrightBrowserOpener) handleInstallAppPage() error { + pageURL := b.page.URL() + b.logf("[browser] handleInstallAppPage at URL: %s", pageURL) + + // Retry loop: the app page may 404 briefly after creation. + for attempt := range 5 { + // Check if we got a 404 and need to retry. + is404, _ := b.page.Locator("img[alt='404'], h1:has-text('404')").Count() + if is404 > 0 { + b.logf("[browser] Got 404, retrying in %ds (attempt %d/5)", (attempt+1)*2, attempt+1) + time.Sleep(time.Duration((attempt+1)*2) * time.Second) + if _, err := b.page.Goto(pageURL, playwright.PageGotoOptions{ + WaitUntil: playwright.WaitUntilStateDomcontentloaded, + Timeout: playwright.Float(10000), + }); err != nil { + b.logf("[browser] Warning: retry navigation failed: %v", err) + continue + } + continue + } + + btn := b.page.Locator("button[type='submit']:has-text('Install')") + if err := btn.Click(playwright.LocatorClickOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + if attempt < 4 { + b.logf("[browser] Install button not found, retrying (attempt %d/5): %v", attempt+1, err) + time.Sleep(time.Duration((attempt+1)*2) * time.Second) + if _, navErr := b.page.Goto(pageURL, playwright.PageGotoOptions{ + WaitUntil: playwright.WaitUntilStateDomcontentloaded, + Timeout: playwright.Float(10000), + }); navErr != nil { + b.logf("[browser] Warning: retry navigation failed: %v", navErr) + } + continue + } + saveDebugScreenshot(b.page, b.screenshotDir, "browser-install-btn-failed", b.logf) + return fmt.Errorf("clicking 'Install': %w", err) + } + + // Successfully clicked Install. + break + } + + // Wait for URL to change away from the installations/new page. + if err := b.page.WaitForURL("!**/installations/new**", playwright.PageWaitForURLOptions{ + Timeout: playwright.Float(10000), + }); err != nil { + // Fall through to WaitForLoadState. + b.logf("[browser] Warning: WaitForURL after install timed out: %v", err) + } + if err := b.page.WaitForLoadState(playwright.PageWaitForLoadStateOptions{ + State: playwright.LoadStateDomcontentloaded, + }); err != nil { + return fmt.Errorf("waiting for install to complete: %w", err) + } + b.logf("[browser] After install, page URL: %s", b.page.URL()) + + return nil +} + +// deleteAppViaPlaywright navigates to the app's advanced settings and deletes it. +func deleteAppViaPlaywright(page playwright.Page, slug string, logf func(string, ...any), screenshotDir string) error { + url := fmt.Sprintf("https://github.com/organizations/%s/settings/apps/%s/advanced", testOrg, slug) + if _, err := page.Goto(url, playwright.PageGotoOptions{ + WaitUntil: playwright.WaitUntilStateDomcontentloaded, + Timeout: playwright.Float(10000), + }); err != nil { + return fmt.Errorf("navigating to app settings for %s: %w", slug, err) + } + + // Check if we got a 404 (app doesn't exist) — not an error. + is404, _ := page.Locator("img[alt='404'], h1:has-text('404')").Count() + if is404 > 0 { + logf("[cleanup] App %s does not exist (404), skipping", slug) + return nil + } + + deleteBtn := page.Locator("button:has-text('Delete GitHub App')") + if err := deleteBtn.Click(playwright.LocatorClickOptions{ + Timeout: playwright.Float(3000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "app-delete-"+slug, logf) + logf("[cleanup] Delete button not found at %s, current URL: %s", url, page.URL()) + return fmt.Errorf("clicking 'Delete GitHub App' for %s: %w", slug, err) + } + + // GitHub requires typing the app name to confirm deletion. + // After clicking "Delete GitHub App", a modal appears with a text input. + // Wait a moment for the modal animation. + time.Sleep(1 * time.Second) + saveDebugScreenshot(page, screenshotDir, "app-confirm-dialog-"+slug, logf) + + confirmInput := page.Locator("input[type='text']") + if err := confirmInput.Last().WaitFor(playwright.LocatorWaitForOptions{ + State: playwright.WaitForSelectorStateVisible, + Timeout: playwright.Float(5000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "app-confirm-wait-"+slug, logf) + return fmt.Errorf("waiting for confirmation input for %s: %w", slug, err) + } + + if err := confirmInput.Last().Fill(slug, playwright.LocatorFillOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "app-confirm-input-"+slug, logf) + return fmt.Errorf("filling app name for deletion of %s: %w", slug, err) + } + logf("[cleanup] Typed app name %q into confirmation input", slug) + + // Click the confirmation button — try multiple possible text variants. + confirmBtn := page.Locator("button:has-text('I understand'), button:has-text('Delete this'), button[type='submit'].btn-danger") + if err := confirmBtn.First().Click(playwright.LocatorClickOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "app-confirm-btn-"+slug, logf) + return fmt.Errorf("confirming deletion of %s: %w", slug, err) + } + + if err := page.WaitForLoadState(playwright.PageWaitForLoadStateOptions{ + State: playwright.LoadStateDomcontentloaded, + }); err != nil { + return fmt.Errorf("waiting for deletion of %s: %w", slug, err) + } + + logf("[cleanup] Deleted GitHub App: %s", slug) + return nil +} + +// extractInputValue extracts the value attribute of a hidden input with the +// given name from raw HTML using proper HTML parsing. The html package +// handles entity decoding automatically. +func extractInputValue(rawHTML, name string) (string, error) { + doc, err := xhtml.Parse(strings.NewReader(rawHTML)) + if err != nil { + return "", fmt.Errorf("parsing HTML: %w", err) + } + var value string + var found bool + var walk func(*xhtml.Node) + walk = func(n *xhtml.Node) { + if found { + return + } + if n.Type == xhtml.ElementNode && n.Data == "input" { + var nameAttr, valueAttr string + for _, a := range n.Attr { + if a.Key == "name" { + nameAttr = a.Val + } + if a.Key == "value" { + valueAttr = a.Val + } + } + if nameAttr == name { + value = valueAttr + found = true + } + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + walk(c) + } + } + walk(doc) + if !found { + return "", fmt.Errorf("input %q not found in HTML", name) + } + return value, nil +} + +// extractFormAction extracts the action URL from the first form element +// using proper HTML parsing. +func extractFormAction(rawHTML string) (string, error) { + doc, err := xhtml.Parse(strings.NewReader(rawHTML)) + if err != nil { + return "", fmt.Errorf("parsing HTML: %w", err) + } + var action string + var found bool + var walk func(*xhtml.Node) + walk = func(n *xhtml.Node) { + if found { + return + } + if n.Type == xhtml.ElementNode && n.Data == "form" { + for _, a := range n.Attr { + if a.Key == "action" { + action = a.Val + found = true + } + } + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + walk(c) + } + } + walk(doc) + if !found { + return "", fmt.Errorf("form action not found in HTML") + } + return action, nil +} diff --git a/e2e/admin/cleanup.go b/e2e/admin/cleanup.go new file mode 100644 index 0000000000..ff88fa0af0 --- /dev/null +++ b/e2e/admin/cleanup.go @@ -0,0 +1,159 @@ +//go:build e2e + +package admin + +import ( + "context" + "fmt" + "net/http" + "strings" + "testing" + + "github.com/playwright-community/playwright-go" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +// cleanupStaleResources removes leftover resources from previous test runs. +// This is the "teardown-first" part of the dual cleanup strategy. +func cleanupStaleResources(ctx context.Context, client forge.Client, page playwright.Page, token, screenshotDir string, t *testing.T) { + t.Helper() + t.Log("[cleanup] Scanning for stale resources from previous runs...") + + // 1. Delete .fullsend repo if it exists. + _, err := client.GetRepo(ctx, testOrg, forge.ConfigRepoName) + if err == nil { + t.Logf("[cleanup] Deleting stale %s repo", forge.ConfigRepoName) + if delErr := client.DeleteRepo(ctx, testOrg, forge.ConfigRepoName); delErr != nil { + t.Logf("[cleanup] Warning: could not delete %s: %v", forge.ConfigRepoName, delErr) + } + } + + // 2. Delete stale FULLSEND_DISPATCH_TOKEN org secret if it exists. + dispatchExists, dispatchErr := client.OrgSecretExists(ctx, testOrg, "FULLSEND_DISPATCH_TOKEN") + if dispatchErr != nil { + t.Logf("[cleanup] Warning: could not check dispatch token org secret: %v", dispatchErr) + } else if dispatchExists { + t.Log("[cleanup] Deleting stale FULLSEND_DISPATCH_TOKEN org secret") + if delErr := client.DeleteOrgSecret(ctx, testOrg, "FULLSEND_DISPATCH_TOKEN"); delErr != nil { + t.Logf("[cleanup] Warning: could not delete dispatch token org secret: %v", delErr) + } + } + + // 3. Delete any stale fullsend GitHub Apps via Playwright. + // First, try deleting by expected slug for each role (catches apps that + // were created but never installed, which don't appear in ListOrgInstallations). + for _, role := range defaultRoles { + slug := testOrg + "-" + role // v6 convention: halfsend-fullsend, etc. + t.Logf("[cleanup] Attempting to delete app %s (if it exists)", slug) + if delErr := deleteAppViaPlaywright(page, slug, t.Logf, screenshotDir); delErr != nil { + t.Logf("[cleanup] App %s not found or could not delete: %v", slug, delErr) + } + } + + // Also clean up apps found via installations (catches old naming conventions). + installations, err := client.ListOrgInstallations(ctx, testOrg) + if err != nil { + t.Logf("[cleanup] Warning: could not list installations: %v", err) + } else { + for _, inst := range installations { + isStale := strings.HasPrefix(inst.AppSlug, "fullsend-"+testOrg) || // old: fullsend-halfsend-* + strings.HasPrefix(inst.AppSlug, testOrg+"-") // v6: halfsend-* + if isStale { + t.Logf("[cleanup] Deleting stale installed app: %s", inst.AppSlug) + if delErr := deleteAppViaPlaywright(page, inst.AppSlug, t.Logf, screenshotDir); delErr != nil { + t.Logf("[cleanup] Warning: could not delete app %s: %v", inst.AppSlug, delErr) + } + } + } + } + + // 4. Delete any stale dispatch PATs from previous runs. + t.Log("[cleanup] Cleaning up stale dispatch PATs...") + if delErr := deleteDispatchPAT(page, testOrg, screenshotDir, t.Logf); delErr != nil { + t.Logf("[cleanup] Warning: could not delete stale dispatch PAT: %v", delErr) + } + + // 5. Ensure test-repo exists (needed for enrollment testing). + _, err = client.GetRepo(ctx, testOrg, testRepo) + if forge.IsNotFound(err) { + t.Logf("[cleanup] Creating missing %s repo", testRepo) + if _, createErr := client.CreateRepo(ctx, testOrg, testRepo, "E2E test repo", false); createErr != nil { + t.Logf("[cleanup] Warning: could not create %s: %v", testRepo, createErr) + } + } + + // 5. Delete stale enrollment branch from test-repo. + deleteEnrollmentBranch(ctx, token, testOrg, testRepo, t) + + // 6. Close any open enrollment PRs in test-repo (informational only). + prs, err := client.ListRepoPullRequests(ctx, testOrg, testRepo) + if err != nil { + t.Logf("[cleanup] Warning: could not list PRs: %v", err) + } else { + for _, pr := range prs { + if strings.Contains(pr.Title, "fullsend") { + t.Logf("[cleanup] Found stale enrollment PR #%d: %s", pr.Number, pr.Title) + } + } + } + + t.Log("[cleanup] Stale resource scan complete") +} + +// registerAppCleanup registers a t.Cleanup that deletes the given app slug. +func registerAppCleanup(t *testing.T, page playwright.Page, slug, screenshotDir string) { + t.Helper() + t.Cleanup(func() { + t.Logf("[cleanup] Deleting app %s via Playwright", slug) + if err := deleteAppViaPlaywright(page, slug, t.Logf, screenshotDir); err != nil { + t.Logf("[cleanup] Warning: could not delete app %s: %v", slug, err) + } + }) +} + +// deleteEnrollmentBranch deletes the fullsend/onboard branch from a repo +// using the GitHub API directly (forge.Client doesn't have DeleteBranch). +func deleteEnrollmentBranch(ctx context.Context, token, org, repo string, t *testing.T) { + t.Helper() + branchRef := "heads/fullsend/onboard" + url := fmt.Sprintf("https://api.github.com/repos/%s/%s/git/refs/%s", org, repo, branchRef) + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, url, nil) + if err != nil { + t.Logf("[cleanup] Warning: could not create branch delete request: %v", err) + return + } + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Logf("[cleanup] Warning: could not delete enrollment branch: %v", err) + return + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusNoContent { + t.Log("[cleanup] Deleted stale enrollment branch fullsend/onboard") + } else if resp.StatusCode == http.StatusNotFound { + // Branch doesn't exist, nothing to do. + } else { + t.Logf("[cleanup] Warning: unexpected status deleting enrollment branch: %d", resp.StatusCode) + } +} + +// registerRepoCleanup registers a t.Cleanup that deletes a repo. +func registerRepoCleanup(t *testing.T, client forge.Client, org, repo string) { + t.Helper() + t.Cleanup(func() { + ctx := context.Background() + _, err := client.GetRepo(ctx, org, repo) + if err != nil { + return // Already gone. + } + t.Logf("[cleanup] Deleting repo %s/%s", org, repo) + if delErr := client.DeleteRepo(ctx, org, repo); delErr != nil { + t.Logf("[cleanup] Warning: could not delete %s/%s: %v", org, repo, delErr) + } + }) +} diff --git a/e2e/admin/lock.go b/e2e/admin/lock.go new file mode 100644 index 0000000000..0b74d50051 --- /dev/null +++ b/e2e/admin/lock.go @@ -0,0 +1,181 @@ +//go:build e2e + +package admin + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +// acquireLock attempts to acquire the distributed e2e lock by creating an +// e2e-lock repo in the test org. If the lock is already held, it polls +// until the lock is released or the timeout expires. +// +// The token parameter is needed for getRepoCreatedAt (direct API call). +// Pass "" if using a fake client (skips age checks). +func acquireLock(ctx context.Context, client forge.Client, token, org, runID string, timeout time.Duration, logf func(string, ...any)) error { + // Try to create the lock repo. + acquired, err := tryCreateLock(ctx, client, org, runID, logf) + if err != nil { + return fmt.Errorf("trying to create lock: %w", err) + } + if acquired { + return nil + } + + // Lock exists. Poll until released or timeout. + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + // Check if lock was released. + content, err := client.GetFileContent(ctx, org, lockRepo, "README.md") + if forge.IsNotFound(err) { + // Lock was released — try to acquire. + acquired, err := tryCreateLock(ctx, client, org, runID, logf) + if err != nil { + return fmt.Errorf("retrying lock creation: %w", err) + } + if acquired { + return nil + } + continue + } + if err != nil { + return fmt.Errorf("reading lock file: %w", err) + } + + holder := strings.TrimSpace(string(content)) + if holder == runID { + return nil // We hold it. + } + + // If the lock content is not a valid UUID (e.g. default README + // content like "# e2e-lock"), treat the lock as stale. + if !isValidUUID(holder) { + logf("[e2e-lock] Lock contains invalid holder %q, force-acquiring", truncateUUID(holder)) + _ = client.DeleteRepo(ctx, org, lockRepo) + acquired, err := tryCreateLock(ctx, client, org, runID, logf) + if err != nil { + return fmt.Errorf("force-acquiring invalid lock: %w", err) + } + if acquired { + return nil + } + continue + } + + // Check lock age if we have a token (skip for fake clients). + if token != "" { + createdAt, ageErr := getRepoCreatedAt(ctx, token, org, lockRepo) + if ageErr == nil { + age := time.Since(createdAt) + + // Stale lock recovery. + if age > timeout { + logf("[e2e-lock] Lock appears stale (age: %s), force-acquiring", age) + _ = client.DeleteRepo(ctx, org, lockRepo) + acquired, err := tryCreateLock(ctx, client, org, runID, logf) + if err != nil { + return fmt.Errorf("force-acquiring stale lock: %w", err) + } + if acquired { + return nil + } + continue + } + + // Fresh lock — reset deadline. + if age < freshLockThreshold { + logf("[e2e-lock] Lock recently acquired by another run (age: %s), resetting timer", age) + deadline = time.Now().Add(timeout) + } + + logf("[e2e-lock] Lock held by %s (age: %s), waiting...", truncateUUID(holder), age.Round(time.Second)) + } + } else { + logf("[e2e-lock] Lock held by %s, waiting...", truncateUUID(holder)) + } + + select { + case <-time.After(lockPollInterval): + case <-ctx.Done(): + return ctx.Err() + } + } + + return fmt.Errorf("timed out waiting for e2e lock after %s", timeout) +} + +// tryCreateLock attempts to create the lock repo and write our UUID. +// Returns (true, nil) if the lock was successfully acquired. +func tryCreateLock(ctx context.Context, client forge.Client, org, runID string, logf func(string, ...any)) (bool, error) { + _, err := client.CreateRepo(ctx, org, lockRepo, "E2E test lock — do not delete manually", false) + if err != nil { + // Repo already exists (409 or similar) — someone else got it. + return false, nil + } + + // Use CreateOrUpdateFile since auto_init creates a default README.md. + // Retry — newly created repos may not be immediately ready for file + // operations due to GitHub's eventual consistency. + createErr := retryOnNotFound(ctx, 5, func() error { + return client.CreateOrUpdateFile(ctx, org, lockRepo, "README.md", "acquire lock", []byte(runID)) + }) + if createErr != nil { + _ = client.DeleteRepo(ctx, org, lockRepo) + return false, fmt.Errorf("writing lock file after retries: %w", createErr) + } + + // Verify we actually got the lock (handle race between two creators). + content, err := client.GetFileContent(ctx, org, lockRepo, "README.md") + if err != nil { + return false, fmt.Errorf("verifying lock: %w", err) + } + if strings.TrimSpace(string(content)) == runID { + logf("[e2e-lock] Lock acquired (run: %s)", truncateUUID(runID)) + return true, nil + } + + // Lost the race. + return false, nil +} + +// releaseLock deletes the lock repo, but only if we still hold it. +func releaseLock(ctx context.Context, client forge.Client, org, runID string, t *testing.T) { + content, err := client.GetFileContent(ctx, org, lockRepo, "README.md") + if err != nil { + t.Logf("[e2e-lock] Could not read lock file during release: %v", err) + return + } + + if strings.TrimSpace(string(content)) != runID { + t.Logf("[e2e-lock] Lock is held by someone else (%s), not releasing", truncateUUID(string(content))) + return + } + + if err := client.DeleteRepo(ctx, org, lockRepo); err != nil { + t.Logf("[e2e-lock] Failed to release lock: %v", err) + return + } + t.Logf("[e2e-lock] Lock released (run: %s)", truncateUUID(runID)) +} + +// truncateUUID returns the first 8 chars of a UUID for log readability. +func truncateUUID(u string) string { + if len(u) > 8 { + return u[:8] + } + return u +} + +// isValidUUID checks whether the string is a valid UUID. +func isValidUUID(s string) bool { + _, err := uuid.Parse(s) + return err == nil +} diff --git a/e2e/admin/lock_test.go b/e2e/admin/lock_test.go new file mode 100644 index 0000000000..fd35285c1a --- /dev/null +++ b/e2e/admin/lock_test.go @@ -0,0 +1,62 @@ +//go:build e2e + +package admin + +import ( + "context" + "testing" + "time" + + "github.com/fullsend-ai/fullsend/internal/forge" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAcquireLock_NoExistingLock(t *testing.T) { + fake := forge.NewFakeClient() + ctx := context.Background() + + runID := "test-uuid-1234" + err := acquireLock(ctx, fake, "", testOrg, runID, 5*time.Minute, t.Logf) + require.NoError(t, err) + + // Verify the lock repo was created with our UUID. + content, err := fake.GetFileContent(ctx, testOrg, lockRepo, "README.md") + require.NoError(t, err) + assert.Equal(t, runID, string(content)) +} + +func TestReleaseLock_OwnedByUs(t *testing.T) { + fake := forge.NewFakeClient() + ctx := context.Background() + + runID := "test-uuid-1234" + // Pre-create the lock repo with our UUID. + _, err := fake.CreateRepo(ctx, testOrg, lockRepo, "E2E test lock", false) + require.NoError(t, err) + err = fake.CreateFile(ctx, testOrg, lockRepo, "README.md", "acquire lock", []byte(runID)) + require.NoError(t, err) + + releaseLock(ctx, fake, testOrg, runID, t) + + // Verify repo was deleted. + _, err = fake.GetRepo(ctx, testOrg, lockRepo) + assert.True(t, forge.IsNotFound(err)) +} + +func TestReleaseLock_OwnedBySomeoneElse(t *testing.T) { + fake := forge.NewFakeClient() + ctx := context.Background() + + // Pre-create the lock repo with a different UUID. + _, err := fake.CreateRepo(ctx, testOrg, lockRepo, "E2E test lock", false) + require.NoError(t, err) + err = fake.CreateFile(ctx, testOrg, lockRepo, "README.md", "acquire lock", []byte("other-uuid")) + require.NoError(t, err) + + releaseLock(ctx, fake, testOrg, "our-uuid", t) + + // Repo should NOT have been deleted (not our lock). + _, err = fake.GetRepo(ctx, testOrg, lockRepo) + assert.NoError(t, err) +} diff --git a/e2e/admin/login.go b/e2e/admin/login.go new file mode 100644 index 0000000000..410ac86bb3 --- /dev/null +++ b/e2e/admin/login.go @@ -0,0 +1,73 @@ +//go:build e2e + +package admin + +import ( + "fmt" + "path/filepath" + "strings" + + "github.com/playwright-community/playwright-go" +) + +// githubLogin logs into GitHub by filling in the login form programmatically. +// This eliminates the need for stored browser sessions and manual refresh. +func githubLogin(page playwright.Page, username, password string, logf func(string, ...any)) error { + if _, err := page.Goto("https://github.com/login", playwright.PageGotoOptions{ + WaitUntil: playwright.WaitUntilStateDomcontentloaded, + }); err != nil { + return fmt.Errorf("navigating to GitHub login: %w", err) + } + + // Check if already logged in (redirected away from login page). + if !strings.Contains(page.URL(), "/login") && !strings.Contains(page.URL(), "/session") { + return nil + } + + // Fill in credentials. + if err := page.Locator("#login_field").Fill(username); err != nil { + return fmt.Errorf("filling username: %w", err) + } + if err := page.Locator("#password").Fill(password); err != nil { + return fmt.Errorf("filling password: %w", err) + } + + // Submit the form. + if err := page.Locator("input[type='submit'], button[type='submit']").First().Click(); err != nil { + return fmt.Errorf("clicking sign in: %w", err) + } + + // Wait for navigation away from the login/session page. + if err := page.WaitForURL("https://github.com/**", playwright.PageWaitForURLOptions{ + Timeout: playwright.Float(7500), + }); err != nil { + currentURL := page.URL() + // If still on login/session, authentication likely failed. + if strings.Contains(currentURL, "/login") || strings.Contains(currentURL, "/session") { + return fmt.Errorf("login appears to have failed, still on %s", currentURL) + } + // Navigated somewhere else — might be OK. + } + + // Final check: make sure we're not still on a login page. + currentURL := page.URL() + if strings.Contains(currentURL, "/login") || strings.Contains(currentURL, "/sessions/two-factor") { + return fmt.Errorf("login incomplete, ended up at %s (2FA may be enabled)", currentURL) + } + + logf("[login] Successfully logged in, current URL: %s", currentURL) + return nil +} + +// saveDebugScreenshot saves a screenshot to dir for debugging. +func saveDebugScreenshot(page playwright.Page, dir, name string, logf func(string, ...any)) { + path := filepath.Join(dir, fmt.Sprintf("e2e-debug-%s.png", name)) + if _, err := page.Screenshot(playwright.PageScreenshotOptions{ + Path: playwright.String(path), + FullPage: playwright.Bool(true), + }); err != nil { + logf("[debug] Could not save screenshot %s: %v", path, err) + return + } + logf("[debug] Screenshot saved: %s", path) +} diff --git a/e2e/admin/pat.go b/e2e/admin/pat.go new file mode 100644 index 0000000000..c89a7985b1 --- /dev/null +++ b/e2e/admin/pat.go @@ -0,0 +1,556 @@ +//go:build e2e + +package admin + +import ( + "fmt" + "strings" + "time" + + "github.com/playwright-community/playwright-go" +) + +// patScopes are the classic PAT scopes needed for e2e tests. +var patScopes = []string{ + "repo", + "admin:org", + "delete_repo", + "workflow", +} + +// createPAT creates a classic GitHub Personal Access Token via the browser. +// The token is created with a 7-day expiry and the scopes needed for e2e tests. +// Returns the token string. +func createPAT(page playwright.Page, note string, logf func(string, ...any)) (string, error) { + url := "https://github.com/settings/tokens/new" + if _, err := page.Goto(url, playwright.PageGotoOptions{ + WaitUntil: playwright.WaitUntilStateDomcontentloaded, + Timeout: playwright.Float(7500), + }); err != nil { + logf("[pat] Current URL after navigation failure: %s", page.URL()) + return "", fmt.Errorf("navigating to token creation page: %w", err) + } + logf("[pat] Navigated to: %s", page.URL()) + + // Verify we're on the right page. + if err := page.Locator("#oauth_access_description").WaitFor(playwright.LocatorWaitForOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + return "", fmt.Errorf("token creation form not found (may not be logged in): %w", err) + } + + // Fill in the token note/description. + if err := page.Locator("#oauth_access_description").Fill(note); err != nil { + return "", fmt.Errorf("filling token note: %w", err) + } + + // Set expiration to 7 days. + expirationSelect := page.Locator("#token_expiration") + if _, err := expirationSelect.SelectOption(playwright.SelectOptionValues{ + Values: playwright.StringSlice("seven_days"), + }, playwright.LocatorSelectOptionOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + logf("[pat] Warning: could not set expiration, using default: %v", err) + } + + // Check the required scope checkboxes. + for _, scope := range patScopes { + checkbox := page.Locator(fmt.Sprintf("input[type='checkbox'][value='%s']", scope)) + if err := checkbox.Check(); err != nil { + return "", fmt.Errorf("checking scope %s: %w", scope, err) + } + } + + // Click "Generate token". + generateBtn := page.Locator("button:has-text('Generate token')") + if err := generateBtn.Click(); err != nil { + return "", fmt.Errorf("clicking Generate token: %w", err) + } + + // Wait for the page to load with the new token displayed. + if err := page.WaitForLoadState(playwright.PageWaitForLoadStateOptions{ + State: playwright.LoadStateDomcontentloaded, + }); err != nil { + return "", fmt.Errorf("waiting for token page to load: %w", err) + } + + // Extract the token value. + tokenElement := page.Locator("#new-oauth-token") + if err := tokenElement.WaitFor(playwright.LocatorWaitForOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + return "", fmt.Errorf("token element not found on page: %w", err) + } + + token, err := tokenElement.TextContent() + if err != nil { + return "", fmt.Errorf("extracting token text: %w", err) + } + + if token == "" { + return "", fmt.Errorf("extracted token is empty") + } + + logf("[pat] Created PAT: %s...%s (note: %s)", token[:4], token[len(token)-4:], note) + return token, nil +} + +// createDispatchPAT creates a fine-grained GitHub Personal Access Token +// scoped to the .fullsend repo with Actions read/write permission. +// This mirrors what the real CLI does in promptDispatchToken — the user +// is guided to create a fine-grained PAT at GitHub's token creation page. +// The e2e test automates the browser interaction instead. +// +// Prerequisites: the .fullsend repo must already exist (the config-repo +// and workflows layers must be installed first, just like the real CLI). +func createDispatchPAT(page playwright.Page, org, screenshotDir string, logf func(string, ...any)) (string, error) { + // Navigate to the fine-grained PAT creation page. + // Don't use target_name query param — GitHub's UI doesn't fully activate + // the downstream widgets (repo picker, permissions) when pre-filled. + // Instead, we'll select the owner manually. + patURL := "https://github.com/settings/personal-access-tokens/new" + + logf("[dispatch-pat] Navigating to fine-grained PAT creation page") + if _, err := page.Goto(patURL, playwright.PageGotoOptions{ + WaitUntil: playwright.WaitUntilStateDomcontentloaded, + Timeout: playwright.Float(15000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-goto-failed", logf) + return "", fmt.Errorf("navigating to fine-grained PAT page: %w", err) + } + logf("[dispatch-pat] Page URL: %s", page.URL()) + + // Wait for the form to render. The "Token name" label is a reliable signal. + tokenNameLabel := page.Locator("text=Token name") + if err := tokenNameLabel.WaitFor(playwright.LocatorWaitForOptions{ + State: playwright.WaitForSelectorStateVisible, + Timeout: playwright.Float(15000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-form-not-loaded", logf) + return "", fmt.Errorf("fine-grained PAT form did not load: %w", err) + } + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-form-loaded", logf) + + // Fill in the token name using Playwright's label-based locator. + // Use a short timestamp to avoid name collisions (max 40 chars). + tokenName := fmt.Sprintf("fs-dispatch-%s-%d", org, time.Now().Unix()) + nameInput := page.GetByLabel("Token name") + if err := nameInput.Fill(tokenName); err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-name-fill-failed", logf) + return "", fmt.Errorf("filling token name: %w", err) + } + logf("[dispatch-pat] Filled token name: %s", tokenName) + + // Select the resource owner (org). The owner picker is a dropdown button + // showing the current owner (e.g., "botsend ▼"). We need to click it + // and select the org. Even if pre-filled, GitHub's UI may not activate + // repo picker and permissions until the owner is manually interacted with. + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-before-owner", logf) + + // The resource owner is a custom dropdown button showing the current + // owner (e.g., "botsend ▼"). Click it to open the owner picker. + // Use JavaScript to find and click the owner button since it's a + // custom React component. + _, err := page.Evaluate(`() => { + // Find all buttons/clickable elements near "Resource owner" text + const labels = document.querySelectorAll('*'); + for (const el of labels) { + if (el.textContent.trim() === 'Resource owner') { + // The dropdown is the next interactive element after the label + let sibling = el.nextElementSibling; + while (sibling) { + const btn = sibling.querySelector('button, summary, [role="button"]'); + if (btn) { btn.click(); return true; } + if (sibling.tagName === 'BUTTON' || sibling.tagName === 'SUMMARY') { + sibling.click(); return true; + } + sibling = sibling.nextElementSibling; + } + } + } + return false; + }`) + if err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-owner-btn-click", logf) + return "", fmt.Errorf("clicking resource owner dropdown via JS: %w", err) + } + logf("[dispatch-pat] Clicked resource owner dropdown") + time.Sleep(500 * time.Millisecond) + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-owner-dropdown-open", logf) + + // Select the org from the dropdown. + orgOption := page.Locator(fmt.Sprintf("[role='menuitemradio']:has-text('%s'), [role='option']:has-text('%s'), li:has-text('%s'), label:has-text('%s')", org, org, org, org)) + if err := orgOption.First().Click(playwright.LocatorClickOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-owner-option", logf) + return "", fmt.Errorf("selecting org %s from owner dropdown: %w", org, err) + } + logf("[dispatch-pat] Selected resource owner: %s", org) + + // Wait for the page to update after owner selection — this may trigger + // a re-render that adds the "Only select repositories" option and + // repository permissions. + time.Sleep(3 * time.Second) + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-after-owner", logf) + + // Select "Only select repositories" radio button. + selectReposLabel := page.Locator("label:has-text('Only select repositories')") + if err := selectReposLabel.Click(playwright.LocatorClickOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + // Try the radio input directly. + selectReposRadio := page.Locator("input[type='radio'][value='select']") + if radioErr := selectReposRadio.Click(playwright.LocatorClickOptions{ + Timeout: playwright.Float(5000), + }); radioErr != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-select-repos", logf) + return "", fmt.Errorf("selecting 'Only select repositories': label=%w, radio=%v", err, radioErr) + } + } + logf("[dispatch-pat] Selected 'Only select repositories'") + + // Wait for the repo picker to appear. + time.Sleep(1 * time.Second) + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-after-select-repos", logf) + + // Search for and select the .fullsend repo in the repo picker. + repoSearch := page.Locator("input[type='text']") + // The repo search is typically the last visible text input after the name input. + // Let's find all visible text inputs and use the one that's for repo search. + searchCount, _ := repoSearch.Count() + logf("[dispatch-pat] Found %d text inputs on page", searchCount) + + // Try known selectors for the repo picker. + repoPickerSelectors := []string{ + "input[placeholder*='Search for a repository']", + "input[placeholder*='search']", + "input[aria-label*='repository']", + "input[aria-label*='repo']", + } + var foundRepoInput playwright.Locator + for _, sel := range repoPickerSelectors { + loc := page.Locator(sel) + cnt, _ := loc.Count() + if cnt > 0 { + logf("[dispatch-pat] Found repo picker with selector: %s (count=%d)", sel, cnt) + foundRepoInput = loc.First() + break + } + } + + if foundRepoInput == nil { + // Last resort: try clicking a "Select repositories" button/dropdown. + selectRepoBtn := page.Locator("button:has-text('Select repositories'), summary:has-text('Select repositories')") + if err := selectRepoBtn.First().Click(playwright.LocatorClickOptions{ + Timeout: playwright.Float(3000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-repo-picker-not-found", logf) + return "", fmt.Errorf("could not find repo picker: %w", err) + } + time.Sleep(500 * time.Millisecond) + // After clicking, look for a search input inside the dropdown. + for _, sel := range repoPickerSelectors { + loc := page.Locator(sel) + cnt, _ := loc.Count() + if cnt > 0 { + foundRepoInput = loc.First() + break + } + } + if foundRepoInput == nil { + // Try any text input that appeared. + foundRepoInput = page.Locator("input[type='text']").Last() + } + } + + if err := foundRepoInput.Fill(".fullsend"); err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-repo-search-fill", logf) + return "", fmt.Errorf("typing .fullsend into repo search: %w", err) + } + logf("[dispatch-pat] Typed '.fullsend' into repo search") + + // Wait for the dropdown option and click it. + time.Sleep(1 * time.Second) + repoOption := page.Locator("[role='option']:has-text('.fullsend'), li:has-text('.fullsend'), label:has-text('.fullsend'), span:has-text('.fullsend')") + if err := repoOption.First().WaitFor(playwright.LocatorWaitForOptions{ + State: playwright.WaitForSelectorStateVisible, + Timeout: playwright.Float(5000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-repo-option-wait", logf) + return "", fmt.Errorf("waiting for .fullsend repo option: %w", err) + } + if err := repoOption.First().Click(); err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-repo-option-click", logf) + return "", fmt.Errorf("selecting .fullsend repo: %w", err) + } + logf("[dispatch-pat] Selected .fullsend repository") + + // Close the repo picker popover. Press Escape multiple times to ensure + // any open dropdown/popover is dismissed, then scroll the permissions + // section into view. + page.Keyboard().Press("Escape") + time.Sleep(500 * time.Millisecond) + page.Keyboard().Press("Escape") + time.Sleep(500 * time.Millisecond) + + // Scroll down to make the permissions section and "Add permissions" visible. + page.Locator("text=Permissions").Last().ScrollIntoViewIfNeeded() + time.Sleep(1 * time.Second) + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-after-close-picker", logf) + + // The permissions UI uses "+ Add permissions" button to open a dialog + // where you can toggle individual permissions. Click it. + addPermsBtn := page.Locator("button:has-text('Add permissions')") + if err := addPermsBtn.Click(playwright.LocatorClickOptions{ + Timeout: playwright.Float(5000), + }); err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-add-perms-btn", logf) + return "", fmt.Errorf("clicking 'Add permissions': %w", err) + } + time.Sleep(1 * time.Second) + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-perms-dialog", logf) + + // The permissions popover shows checkboxes. Click the Actions checkbox + // using JavaScript since the checkbox UI may be a custom component. + _, err = page.Evaluate(`() => { + const items = document.querySelectorAll('*'); + for (const el of items) { + if (el.textContent.trim() === 'Actions' && el.closest('[role="option"], label, li')) { + el.closest('[role="option"], label, li').click(); + return true; + } + } + // Fallback: find checkbox near "Actions" text + for (const el of items) { + if (el.textContent.trim() === 'Actions') { + const parent = el.parentElement; + const checkbox = parent.querySelector('input[type="checkbox"]'); + if (checkbox) { checkbox.click(); return true; } + // Try clicking the parent itself + parent.click(); + return true; + } + } + return false; + }`) + if err != nil { + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-actions-checkbox", logf) + return "", fmt.Errorf("clicking Actions checkbox via JS: %w", err) + } + logf("[dispatch-pat] Checked Actions permission") + time.Sleep(1 * time.Second) + saveDebugScreenshot(page, screenshotDir, "dispatch-pat-after-actions-check", logf) + + // Close the permissions popover by pressing Escape. + page.Keyboard().Press("Escape") + time.Sleep(500 * time.Millisecond) + + // The Actions permission is now added but defaults to "Read-only". + // Find the Actions row's level dropdown and change it to "Read and write". + // The dropdown appears as a - @@ -304,7 +314,6 @@ func (s *Setup) runManifestFlow(ctx context.Context, org, role string) (*AppCred html.EscapeString(appCfg.Name), html.EscapeString(githubFormAction), html.EscapeString(string(manifest)), - html.EscapeString(callbackURL), ) fmt.Fprint(w, page) }) @@ -335,7 +344,7 @@ func (s *Setup) runManifestFlow(ctx context.Context, org, role string) (*AppCred

App %s created successfully!

You can close this tab and return to the terminal.

-`, html.EscapeString(creds.Name)) +`, creds.Name) resultCh <- result{creds: creds} }) @@ -418,6 +427,15 @@ func (s *Setup) exchangeManifestCode(ctx context.Context, code string) (*AppCred // ensureInstalled checks that the app is installed on the org, prompting // the user to install it if not. +// +// The installation URL must be /apps/{slug}/installations/new without any +// query parameters. Earlier iterations used target_id=0 which is invalid. +// installPollInterval is how often we check for the app installation. +const installPollInterval = 2 * time.Second + +// installPollTimeout is how long we wait for the user to install the app. +const installPollTimeout = 5 * time.Minute + func (s *Setup) ensureInstalled(ctx context.Context, org, slug string) error { installations, err := s.client.ListOrgInstallations(ctx, org) if err != nil { @@ -431,40 +449,44 @@ func (s *Setup) ensureInstalled(ctx context.Context, org, slug string) error { } } - // App not installed — prompt user to install. + // App not installed — open browser and poll until it appears. installURL := fmt.Sprintf("https://github.com/apps/%s/installations/new", slug) s.ui.StepWarn(fmt.Sprintf("App %s is not yet installed on %s", slug, org)) - s.ui.StepInfo(fmt.Sprintf("Please install it at: %s", installURL)) + s.ui.StepStart("Opening browser for installation...") if err := s.browser.Open(ctx, installURL); err != nil { s.ui.StepWarn(fmt.Sprintf("Could not open browser: %v", err)) + s.ui.StepInfo(fmt.Sprintf("Install manually at: %s", installURL)) } - if err := s.prompter.WaitForEnter("Press Enter after installing the app..."); err != nil { - return fmt.Errorf("waiting for user: %w", err) - } + s.ui.StepInfo("Waiting for installation (will detect automatically)...") - // Verify installation. - installations, err = s.client.ListOrgInstallations(ctx, org) - if err != nil { - return fmt.Errorf("verifying installation: %w", err) - } + // Poll until the app appears in installations or we time out. + pollCtx, cancel := context.WithTimeout(ctx, installPollTimeout) + defer cancel() - for _, inst := range installations { - if inst.AppSlug == slug { - s.ui.StepDone(fmt.Sprintf("App %s installed successfully", slug)) - return nil + for { + select { + case <-pollCtx.Done(): + return fmt.Errorf("timed out waiting for app %s to be installed on %s", slug, org) + case <-time.After(installPollInterval): + installations, err := s.client.ListOrgInstallations(pollCtx, org) + if err != nil { + continue // transient errors — keep polling + } + for _, inst := range installations { + if inst.AppSlug == slug { + s.ui.StepDone(fmt.Sprintf("App %s installed successfully", slug)) + return nil + } + } } } - - return fmt.Errorf("app %s was not found in org %s after installation attempt", slug, org) } -// expectedAppSlug returns the conventional app slug for a given org and role. -// This matches the naming convention used by ghTypes.AgentAppConfig. -func expectedAppSlug(org, role string) string { - if role == "fullsend" { - return "fullsend-" + org - } - return "fullsend-" + org + "-" + role +// ExpectedAppSlug returns the conventional app slug for a given org and role. +// The convention is simply - for all roles. +// Used during uninstall to infer app names when config.yaml is unavailable. +func ExpectedAppSlug(org, role string) string { + return org + "-" + role } diff --git a/internal/appsetup/appsetup_test.go b/internal/appsetup/appsetup_test.go index 9ecc219a67..02cd2c5bab 100644 --- a/internal/appsetup/appsetup_test.go +++ b/internal/appsetup/appsetup_test.go @@ -52,43 +52,43 @@ func TestExpectedAppSlug(t *testing.T) { name: "fullsend role uses org only", org: "myorg", role: "fullsend", - expected: "fullsend-myorg", + expected: "myorg-fullsend", }, { name: "triage role appends role suffix", org: "myorg", role: "triage", - expected: "fullsend-myorg-triage", + expected: "myorg-triage", }, { name: "coder role appends role suffix", org: "acme", role: "coder", - expected: "fullsend-acme-coder", + expected: "acme-coder", }, { name: "review role appends role suffix", org: "acme", role: "review", - expected: "fullsend-acme-review", + expected: "acme-review", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := expectedAppSlug(tc.org, tc.role) + got := ExpectedAppSlug(tc.org, tc.role) assert.Equal(t, tc.expected, got) }) } } -func TestSetup_ExistingApp_SecretExists_Reuse(t *testing.T) { +func TestSetup_ExistingApp_SecretExists_AutoReuse(t *testing.T) { client := &forge.FakeClient{ Installations: []forge.Installation{ - {ID: 100, AppID: 10, AppSlug: "fullsend-myorg"}, + {ID: 100, AppID: 10, AppSlug: "myorg-fullsend"}, }, } - prompter := &fakePrompter{confirmResult: true} + prompter := &fakePrompter{} browser := &fakeBrowser{} printer := ui.New(&discardWriter{}) @@ -102,38 +102,16 @@ func TestSetup_ExistingApp_SecretExists_Reuse(t *testing.T) { // Should return credentials signaling reuse (empty PEM). assert.Equal(t, 10, creds.AppID) - assert.Equal(t, "fullsend-myorg", creds.Slug) + assert.Equal(t, "myorg-fullsend", creds.Slug) assert.Empty(t, creds.PEM, "PEM should be empty to signal reuse") - assert.True(t, prompter.confirmCalled, "should have asked to confirm reuse") -} - -func TestSetup_ExistingApp_SecretExists_DeclineReuse(t *testing.T) { - client := &forge.FakeClient{ - Installations: []forge.Installation{ - {ID: 100, AppID: 10, AppSlug: "fullsend-myorg"}, - }, - } - prompter := &fakePrompter{confirmResult: false} - browser := &fakeBrowser{} - printer := ui.New(&discardWriter{}) - - s := NewSetup(client, prompter, browser, printer). - WithSecretExists(func(_ string) (bool, error) { - return true, nil - }) - - // When the user declines reuse, an error is returned telling them - // to delete the app first. - _, err := s.Run(context.Background(), "myorg", "fullsend") - require.Error(t, err) - assert.Contains(t, err.Error(), "declined") - assert.True(t, prompter.confirmCalled, "should have asked to confirm reuse") + // Should NOT have prompted — auto-reuse is silent. + assert.False(t, prompter.confirmCalled, "should not prompt for reuse") } func TestSetup_ExistingApp_NoSecret(t *testing.T) { client := &forge.FakeClient{ Installations: []forge.Installation{ - {ID: 100, AppID: 10, AppSlug: "fullsend-myorg-triage"}, + {ID: 100, AppID: 10, AppSlug: "myorg-triage"}, }, } prompter := &fakePrompter{} @@ -156,7 +134,7 @@ func TestSetup_KnownSlug_Match(t *testing.T) { {ID: 200, AppID: 20, AppSlug: "custom-slug-name"}, }, } - prompter := &fakePrompter{confirmResult: true} + prompter := &fakePrompter{} browser := &fakeBrowser{} printer := ui.New(&discardWriter{}) @@ -172,6 +150,7 @@ func TestSetup_KnownSlug_Match(t *testing.T) { assert.Equal(t, 20, creds.AppID) assert.Equal(t, "custom-slug-name", creds.Slug) assert.Empty(t, creds.PEM) + assert.False(t, prompter.confirmCalled, "should not prompt for reuse") } func TestSetup_NoExistingApp(t *testing.T) { diff --git a/internal/cli/admin.go b/internal/cli/admin.go index 132e63f7e6..638d65d6c1 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -1,6 +1,7 @@ package cli import ( + "bufio" "context" "fmt" "os" @@ -29,7 +30,16 @@ func newAdminCmd() *cobra.Command { return cmd } -// resolveToken finds a GitHub token from env vars or gh CLI. +// resolveToken finds a GitHub token by checking, in order: +// 1. GH_TOKEN env var +// 2. GITHUB_TOKEN env var +// 3. gh auth token (subprocess call to the GitHub CLI) +// +// This chain allows users who are already authenticated with gh to use +// fullsend without manually exporting tokens. Note that some operations +// (like repo deletion) require the delete_repo scope, and workflow file +// writes require the workflow scope — scopes that gh auth doesn't request +// by default. Use: gh auth refresh -s delete_repo,workflow func resolveToken() (string, error) { if token := os.Getenv("GH_TOKEN"); token != "" { return token, nil @@ -238,7 +248,14 @@ func runDryRun(ctx context.Context, client forge.Client, printer *ui.Printer, or }) } - stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds) + enrolledRepoIDs := collectEnrolledRepoIDs(allRepos, enabledRepos) + stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", enrolledRepoIDs) + + if err := runPreflight(ctx, stack, layers.OpInstall, client, printer); err != nil { + return err + } + printer.Blank() + return printAnalysis(ctx, stack, printer) } @@ -298,6 +315,9 @@ func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, o printer.StepDone(fmt.Sprintf("Found %d repositories", len(allRepos))) printer.Blank() + // Collect IDs for repos that will be enrolled. + enrolledRepoIDs := collectEnrolledRepoIDs(allRepos, enabledRepos) + // Build agent entries for config. agents := make([]config.AgentEntry, len(agentCreds)) for i, ac := range agentCreds { @@ -311,10 +331,45 @@ func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, o return fmt.Errorf("getting authenticated user: %w", err) } - printer.Header("Installing layers") + // Build stack with empty dispatch token for preflight — we check scopes + // before prompting the user so we fail early on missing admin:org. + stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, "", enrolledRepoIDs) + + if err := runPreflight(ctx, stack, layers.OpInstall, client, printer); err != nil { + return err + } printer.Blank() - stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds) + // Create the .fullsend config repo and write workflow files BEFORE + // prompting for the dispatch token. The user needs the repo to exist + // so they can select it when creating the fine-grained PAT, and the + // agent.yaml workflow must exist so we can verify the PAT by attempting + // a real dispatch. Both layers are idempotent, so running them again + // in the full stack is harmless. + printer.Header("Preparing config repo") + printer.Blank() + configRepoLayer := layers.NewConfigRepoLayer(org, client, cfg, printer, hasPrivate) + if err := configRepoLayer.Install(ctx); err != nil { + return fmt.Errorf("creating config repo: %w", err) + } + workflowsLayer := layers.NewWorkflowsLayer(org, client, printer, user) + if err := workflowsLayer.Install(ctx); err != nil { + return fmt.Errorf("writing workflows: %w", err) + } + printer.Blank() + + // Dispatch token setup — the .fullsend repo now exists so the user + // can select it when creating the fine-grained PAT. + dispatchToken, err := promptDispatchToken(ctx, client, printer, org) + if err != nil { + return err + } + + // Rebuild stack with the actual dispatch token. + stack = buildLayerStack(org, client, cfg, printer, user, hasPrivate, enabledRepos, defaultBranches, agentCreds, dispatchToken, enrolledRepoIDs) + + printer.Header("Installing layers") + printer.Blank() if err := stack.InstallAll(ctx); err != nil { return fmt.Errorf("installation failed: %w", err) @@ -332,7 +387,11 @@ func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, o // runUninstall tears down the fullsend installation. func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer, org string) error { - // Try to load existing config for agent info. + // Try to load agent slugs from existing config. If the .fullsend repo + // is already gone (e.g., previous partial uninstall), fall back to the + // default naming convention so we can still guide the user to delete + // the apps. Without this fallback, a partial uninstall leaves orphaned + // apps that block reinstallation (PEM keys are one-shot). var agentSlugs []string cfgData, err := client.GetFileContent(ctx, org, forge.ConfigRepoName, "config.yaml") if err == nil { @@ -342,6 +401,13 @@ func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer, } } } + if len(agentSlugs) == 0 { + // Config unavailable — assume default app naming convention. + for _, role := range config.DefaultAgentRoles() { + agentSlugs = append(agentSlugs, appsetup.ExpectedAppSlug(org, role)) + } + printer.StepInfo("Config repo unavailable; using default app names") + } // Build a minimal stack for uninstall. // Only ConfigRepoLayer matters for uninstall since other layers are no-ops. @@ -350,9 +416,15 @@ func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer, layers.NewConfigRepoLayer(org, client, emptyCfg, printer, false), layers.NewWorkflowsLayer(org, client, printer, ""), layers.NewSecretsLayer(org, client, nil, printer), + layers.NewDispatchTokenLayer(org, client, "", nil, printer), layers.NewEnrollmentLayer(org, client, nil, nil, printer), ) + if err := runPreflight(ctx, stack, layers.OpUninstall, client, printer); err != nil { + return err + } + printer.Blank() + errs := stack.UninstallAll(ctx) if len(errs) > 0 { for _, e := range errs { @@ -362,14 +434,54 @@ func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer, printer.Blank() - // Suggest manual app deletion. + // Check which apps actually exist before opening browser pages. + // GitHub App uninstallation via API (DELETE /app/installations/{id}) requires + // JWT auth from the app's own private key, not a PAT. Since we authenticate + // with a PAT, we open the browser to the app's advanced settings page instead. + // The correct URL for org-scoped apps is /organizations/{org}/settings/apps/{slug}/advanced + // (the /advanced suffix is required to see the delete button; /settings/apps/{slug} + // alone is for user-scoped apps and will 404 for org-scoped ones). if len(agentSlugs) > 0 { - printer.Header("Manual cleanup required") - printer.StepInfo("Delete these GitHub Apps manually:") - for _, slug := range agentSlugs { - printer.StepInfo(fmt.Sprintf(" https://github.com/apps/%s", slug)) + // Find which slugs correspond to real installed apps. + var existingSlugs []string + installations, listErr := client.ListOrgInstallations(ctx, org) + if listErr == nil { + installedSet := make(map[string]bool, len(installations)) + for _, inst := range installations { + installedSet[inst.AppSlug] = true + } + for _, slug := range agentSlugs { + if installedSet[slug] { + existingSlugs = append(existingSlugs, slug) + } else { + printer.StepInfo(fmt.Sprintf("App %s not found, skipping", slug)) + } + } + } else { + // Can't check — fall back to opening all of them. + printer.StepWarn("Could not verify which apps exist; opening all") + existingSlugs = agentSlugs + } + + if len(existingSlugs) > 0 { + printer.Header("App cleanup") + printer.StepInfo("Opening browser for each app that needs to be deleted.") + printer.StepInfo("Click 'Delete GitHub App' on each page, then return here.") + printer.Blank() + + browser := appsetup.DefaultBrowser{} + for _, slug := range existingSlugs { + deleteURL := fmt.Sprintf("https://github.com/organizations/%s/settings/apps/%s/advanced", org, slug) + printer.StepStart(fmt.Sprintf("Opening %s settings...", slug)) + if err := browser.Open(ctx, deleteURL); err != nil { + printer.StepWarn(fmt.Sprintf("Could not open browser: %v", err)) + printer.StepInfo(fmt.Sprintf(" Delete manually at: %s", deleteURL)) + } else { + printer.StepDone(fmt.Sprintf("Opened %s", slug)) + } + } + printer.Blank() } - printer.Blank() } if len(errs) > 0 { @@ -418,7 +530,13 @@ func runAnalyze(ctx context.Context, client forge.Client, printer *ui.Printer, o return fmt.Errorf("getting authenticated user: %w", err) } - stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, nil, defaultBranches, agentCreds) + stack := buildLayerStack(org, client, cfg, printer, user, hasPrivate, nil, defaultBranches, agentCreds, "", nil) + + if err := runPreflight(ctx, stack, layers.OpAnalyze, client, printer); err != nil { + return err + } + printer.Blank() + return printAnalysis(ctx, stack, printer) } @@ -433,15 +551,42 @@ func buildLayerStack( enabledRepos []string, defaultBranches map[string]string, agentCreds []layers.AgentCredentials, + dispatchToken string, + enrolledRepoIDs []int64, ) *layers.Stack { return layers.NewStack( layers.NewConfigRepoLayer(org, client, cfg, printer, hasPrivate), layers.NewWorkflowsLayer(org, client, printer, user), layers.NewSecretsLayer(org, client, agentCreds, printer), + layers.NewDispatchTokenLayer(org, client, dispatchToken, enrolledRepoIDs, printer), layers.NewEnrollmentLayer(org, client, enabledRepos, defaultBranches, printer), ) } +// runPreflight checks that the token has all required scopes for the +// given operation. Returns nil if all scopes are present or if scope +// introspection is unavailable (fine-grained tokens). Returns an error +// with remediation instructions if scopes are missing. +func runPreflight(ctx context.Context, stack *layers.Stack, op layers.Operation, client forge.Client, printer *ui.Printer) error { + printer.StepStart("Checking token permissions") + + result, err := stack.Preflight(ctx, op, client) + if err != nil { + printer.StepFail("Could not verify token permissions") + return fmt.Errorf("preflight check: %w", err) + } + + if !result.OK() { + printer.StepFail("Token is missing required scopes") + printer.Blank() + printer.ErrorBox("Missing token scopes", result.Error()) + return fmt.Errorf("token is missing required scopes: %s", strings.Join(result.Missing, ", ")) + } + + printer.StepDone("Token permissions verified") + return nil +} + // printAnalysis runs AnalyzeAll and prints reports. func printAnalysis(ctx context.Context, stack *layers.Stack, printer *ui.Printer) error { reports, err := stack.AnalyzeAll(ctx) @@ -504,6 +649,134 @@ func loadKnownSlugs(ctx context.Context, client forge.Client, org string) map[st return cfg.AgentSlugs() } +// collectEnrolledRepoIDs returns the IDs of repos whose names appear in +// the enabledRepos list. +func collectEnrolledRepoIDs(allRepos []forge.Repository, enabledRepos []string) []int64 { + enabled := make(map[string]bool, len(enabledRepos)) + for _, name := range enabledRepos { + enabled[name] = true + } + var ids []int64 + for _, r := range allRepos { + if enabled[r.Name] { + ids = append(ids, r.ID) + } + } + return ids +} + +// promptDispatchToken checks whether the dispatch token org secret already +// exists and, if not, opens the browser to GitHub's pre-filled fine-grained +// PAT creation page and prompts the user to paste the result. +// Returns the token string (empty if reusing an existing secret). +func promptDispatchToken(ctx context.Context, client forge.Client, printer *ui.Printer, org string) (string, error) { + printer.Header("Dispatch Token Setup") + printer.Blank() + + exists, err := client.OrgSecretExists(ctx, org, "FULLSEND_DISPATCH_TOKEN") + if err != nil { + return "", fmt.Errorf("checking dispatch token: %w", err) + } + + if exists { + printer.StepDone("Dispatch token already configured") + return "", nil + } + + // Build a pre-filled URL for fine-grained PAT creation. + // GitHub supports query parameters to pre-fill name, description, + // resource owner, expiration, and permissions. The user only needs to: + // 1. Select "Only select repositories" and pick .fullsend + // 2. Click "Generate token" + // 3. Paste the token + patURL := fmt.Sprintf( + "https://github.com/settings/personal-access-tokens/new"+ + "?name=fullsend-dispatch-%s"+ + "&description=Dispatch+token+for+fullsend+agent+pipeline+in+%s."+ + "+Scoped+to+.fullsend+repo+with+Actions+write+only."+ + "&target_name=%s"+ + "&actions=write", + org, org, org, + ) + + printer.StepStart("Opening browser for dispatch token creation") + + browser := appsetup.DefaultBrowser{} + if err := browser.Open(ctx, patURL); err != nil { + printer.StepWarn(fmt.Sprintf("Could not open browser: %v", err)) + printer.StepInfo("Open this URL manually:") + printer.StepInfo(" " + patURL) + } else { + printer.StepDone("Opened token creation page") + } + + printer.Blank() + printer.StepWarn("IMPORTANT: GitHub's resource owner selector has a known quirk.") + printer.StepWarn("If the owner is pre-filled, you may need to de-select and") + printer.StepWarn("re-select the owner for the repository picker to appear.") + printer.Blank() + printer.StepInfo("In the browser:") + printer.StepInfo(" 1. Verify the 'Resource owner' is set to " + org) + printer.StepInfo(" (If the repo picker doesn't appear, switch the owner") + printer.StepInfo(" away and back to " + org + " to force it to load)") + printer.StepInfo(" 2. Under 'Repository access', select 'Only select repositories'") + printer.StepInfo(" 3. Pick ONLY the .fullsend repository (not other repos)") + printer.StepInfo(" 4. Verify 'Actions: Read and write' is checked under permissions") + printer.StepInfo(" 5. Click 'Generate token'") + printer.StepInfo(" 6. Copy and paste the token below") + printer.Blank() + printer.StepInfo("Paste the token here:") + + scanner := bufio.NewScanner(os.Stdin) + if !scanner.Scan() { + if err := scanner.Err(); err != nil { + return "", fmt.Errorf("reading dispatch token: %w", err) + } + return "", fmt.Errorf("no dispatch token provided") + } + // Aggressively strip whitespace — pasting from browser can include + // trailing newlines, carriage returns, or spaces that would corrupt + // the token when stored as a secret. + token := strings.TrimSpace(scanner.Text()) + token = strings.ReplaceAll(token, "\r", "") + token = strings.ReplaceAll(token, "\n", "") + if token == "" { + return "", fmt.Errorf("dispatch token cannot be empty") + } + + // Verify the token can actually dispatch workflows on .fullsend by + // triggering a real workflow_dispatch event. This is the exact operation + // the shim will perform, so if this works, the shim will work. + // The dispatch triggers agent.yaml with a "verify" event type — the + // workflow will run but the entrypoint script will see it's a verify + // event and exit cleanly. + printer.StepStart("Verifying token can dispatch workflows on " + forge.ConfigRepoName) + verifyClient := gh.New(token) + err = verifyClient.DispatchWorkflow(ctx, org, forge.ConfigRepoName, "agent.yaml", "main", map[string]string{ + "event_type": "verify", + "source_repo": org + "/" + forge.ConfigRepoName, + "event_payload": "{}", + }) + if err != nil { + printer.StepFail("Token cannot dispatch workflows on " + forge.ConfigRepoName) + printer.Blank() + printer.ErrorBox("Dispatch token verification failed", + "The token could not trigger a workflow on "+org+"/"+forge.ConfigRepoName+".\n\n"+ + "This usually means the PAT was not configured correctly.\n"+ + "Delete it at https://github.com/settings/tokens and recreate with:\n"+ + " 1. Resource owner: "+org+"\n"+ + " 2. Repository access: Only select repositories → "+forge.ConfigRepoName+"\n"+ + " 3. Permissions: Actions → Read and write\n\n"+ + "Error: "+err.Error(), + ) + return "", fmt.Errorf("dispatch token verification failed") + } + printer.StepDone("Token verified — test dispatch succeeded") + + printer.Blank() + return token, nil +} + // Helper functions. func repoNameList(repos []forge.Repository) []string { diff --git a/internal/forge/fake.go b/internal/forge/fake.go index 4224d512a0..ef1a0cdbb7 100644 --- a/internal/forge/fake.go +++ b/internal/forge/fake.go @@ -9,6 +9,17 @@ import ( // Compile-time check that FakeClient implements Client. var _ Client = (*FakeClient)(nil) +// NewFakeClient returns a FakeClient with all maps initialised. +func NewFakeClient() *FakeClient { + return &FakeClient{ + FileContents: make(map[string][]byte), + WorkflowRuns: make(map[string]*WorkflowRun), + Secrets: make(map[string]bool), + VariablesExist: make(map[string]bool), + Errors: make(map[string]error), + } +} + // FileRecord records a file creation/update call. type FileRecord struct { Owner, Repo, Path, Branch, Message string @@ -20,6 +31,12 @@ type SecretRecord struct { Owner, Repo, Name, Value string } +// OrgSecretRecord records an org-level secret creation call. +type OrgSecretRecord struct { + Org, Name, Value string + RepoIDs []int64 +} + // VariableRecord records a variable creation/update call. type VariableRecord struct { Owner, Repo, Name, Value string @@ -37,20 +54,28 @@ type FakeClient struct { WorkflowRuns map[string]*WorkflowRun // key: "owner/repo/workflow" AuthenticatedUser string Installations []Installation - Secrets map[string]bool // key: "owner/repo/name" - VariablesExist map[string]bool // key: "owner/repo/name" + Secrets map[string]bool // key: "owner/repo/name" + PullRequests map[string][]ChangeProposal // key: "owner/repo" + TokenScopes []string // scopes returned by GetTokenScopes + VariablesExist map[string]bool // key: "owner/repo/name" + + // Org-level secret state + OrgSecrets map[string]bool // key: "org/name" + OrgSecretRepoIDs map[string][]int64 // key: "org/name" → repo IDs // Error injection: key is method name, value is error to return. Errors map[string]error // Call recorders - CreatedRepos []Repository - CreatedFiles []FileRecord - CreatedBranches []string // "owner/repo/branch" - CreatedProposals []ChangeProposal - DeletedRepos []string // "owner/repo" - CreatedSecrets []SecretRecord - Variables []VariableRecord + CreatedRepos []Repository + CreatedFiles []FileRecord + CreatedBranches []string // "owner/repo/branch" + CreatedProposals []ChangeProposal + DeletedRepos []string // "owner/repo" + CreatedSecrets []SecretRecord + Variables []VariableRecord + DeletedOrgSecrets []string // "org/name" + CreatedOrgSecrets []OrgSecretRecord // internal counter for change proposal numbers proposalCounter int @@ -90,9 +115,23 @@ func (f *FakeClient) CreateRepo(_ context.Context, org, name, description string return nil, e } + fullName := org + "/" + name + // Check for duplicates in pre-populated repos. + for _, r := range f.Repos { + if r.FullName == fullName || r.Name == name { + return nil, fmt.Errorf("repository already exists: %s", fullName) + } + } + // Check for duplicates in previously created repos. + for _, r := range f.CreatedRepos { + if r.FullName == fullName || r.Name == name { + return nil, fmt.Errorf("repository already exists: %s", fullName) + } + } + r := Repository{ Name: name, - FullName: org + "/" + name, + FullName: fullName, DefaultBranch: "main", Private: private, } @@ -131,6 +170,34 @@ func (f *FakeClient) DeleteRepo(_ context.Context, owner, repo string) error { } f.DeletedRepos = append(f.DeletedRepos, owner+"/"+repo) + + // Remove from Repos. + fullName := owner + "/" + repo + filtered := f.Repos[:0] + for _, r := range f.Repos { + if r.FullName != fullName && r.Name != repo { + filtered = append(filtered, r) + } + } + f.Repos = filtered + + // Remove from CreatedRepos. + filteredCreated := f.CreatedRepos[:0] + for _, r := range f.CreatedRepos { + if r.FullName != fullName && r.Name != repo { + filteredCreated = append(filteredCreated, r) + } + } + f.CreatedRepos = filteredCreated + + // Remove associated file contents. + prefix := fullName + "/" + for k := range f.FileContents { + if len(k) >= len(prefix) && k[:len(prefix)] == prefix { + delete(f.FileContents, k) + } + } + return nil } @@ -149,6 +216,11 @@ func (f *FakeClient) CreateFile(_ context.Context, owner, repo, path, message st Message: message, Content: content, }) + + if f.FileContents == nil { + f.FileContents = make(map[string][]byte) + } + f.FileContents[owner+"/"+repo+"/"+path] = content return nil } @@ -222,6 +294,30 @@ func (f *FakeClient) CreateFileOnBranch(_ context.Context, owner, repo, branch, return nil } +func (f *FakeClient) CreateOrUpdateFileOnBranch(_ context.Context, owner, repo, branch, path, message string, content []byte) error { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("CreateOrUpdateFileOnBranch"); e != nil { + return e + } + + f.CreatedFiles = append(f.CreatedFiles, FileRecord{ + Owner: owner, + Repo: repo, + Path: path, + Branch: branch, + Message: message, + Content: content, + }) + // Also update FileContents so subsequent reads see the new content. + if f.FileContents == nil { + f.FileContents = make(map[string][]byte) + } + f.FileContents[owner+"/"+repo+"/"+path] = content + return nil +} + func (f *FakeClient) CreateChangeProposal(_ context.Context, owner, repo, title, body, head, base string) (*ChangeProposal, error) { f.mu.Lock() defer f.mu.Unlock() @@ -240,7 +336,7 @@ func (f *FakeClient) CreateChangeProposal(_ context.Context, owner, repo, title, return &cp, nil } -func (f *FakeClient) ListRepoPullRequests(_ context.Context, _, _ string) ([]ChangeProposal, error) { +func (f *FakeClient) ListRepoPullRequests(_ context.Context, owner, repo string) ([]ChangeProposal, error) { f.mu.Lock() defer f.mu.Unlock() @@ -248,6 +344,11 @@ func (f *FakeClient) ListRepoPullRequests(_ context.Context, _, _ string) ([]Cha return nil, e } + if f.PullRequests != nil { + if prs, ok := f.PullRequests[owner+"/"+repo]; ok { + return prs, nil + } + } return []ChangeProposal{}, nil } @@ -262,6 +363,17 @@ func (f *FakeClient) GetAuthenticatedUser(_ context.Context) (string, error) { return f.AuthenticatedUser, nil } +func (f *FakeClient) GetTokenScopes(_ context.Context) ([]string, error) { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("GetTokenScopes"); e != nil { + return nil, e + } + + return f.TokenScopes, nil +} + func (f *FakeClient) CreateRepoSecret(_ context.Context, owner, repo, name, value string) error { f.mu.Lock() defer f.mu.Unlock() @@ -356,6 +468,17 @@ func (f *FakeClient) GetWorkflowRun(_ context.Context, owner, repo string, runID return nil, fmt.Errorf("workflow run %d not found in %s/%s", runID, owner, repo) } +func (f *FakeClient) DispatchWorkflow(_ context.Context, _, _, _, _ string, _ map[string]string) error { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("DispatchWorkflow"); e != nil { + return e + } + + return nil +} + func (f *FakeClient) ListOrgInstallations(_ context.Context, _ string) ([]Installation, error) { f.mu.Lock() defer f.mu.Unlock() @@ -366,3 +489,66 @@ func (f *FakeClient) ListOrgInstallations(_ context.Context, _ string) ([]Instal return f.Installations, nil } + +func (f *FakeClient) CreateOrgSecret(_ context.Context, org, name, value string, selectedRepoIDs []int64) error { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("CreateOrgSecret"); e != nil { + return e + } + + f.CreatedOrgSecrets = append(f.CreatedOrgSecrets, OrgSecretRecord{ + Org: org, + Name: name, + Value: value, + RepoIDs: selectedRepoIDs, + }) + + if f.OrgSecrets == nil { + f.OrgSecrets = make(map[string]bool) + } + f.OrgSecrets[org+"/"+name] = true + return nil +} + +func (f *FakeClient) OrgSecretExists(_ context.Context, org, name string) (bool, error) { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("OrgSecretExists"); e != nil { + return false, e + } + + if f.OrgSecrets == nil { + return false, nil + } + return f.OrgSecrets[org+"/"+name], nil +} + +func (f *FakeClient) DeleteOrgSecret(_ context.Context, org, name string) error { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("DeleteOrgSecret"); e != nil { + return e + } + + f.DeletedOrgSecrets = append(f.DeletedOrgSecrets, org+"/"+name) + return nil +} + +func (f *FakeClient) SetOrgSecretRepos(_ context.Context, org, name string, repoIDs []int64) error { + f.mu.Lock() + defer f.mu.Unlock() + + if e := f.err("SetOrgSecretRepos"); e != nil { + return e + } + + if f.OrgSecretRepoIDs == nil { + f.OrgSecretRepoIDs = make(map[string][]int64) + } + f.OrgSecretRepoIDs[org+"/"+name] = repoIDs + return nil +} diff --git a/internal/forge/fake_test.go b/internal/forge/fake_test.go index 178f04e6e5..c7c7241eef 100644 --- a/internal/forge/fake_test.go +++ b/internal/forge/fake_test.go @@ -256,6 +256,55 @@ func TestFakeClient_Installations(t *testing.T) { assert.Equal(t, "fullsend-bot", installs[0].AppSlug) } +func TestFakeClient_OrgSecretExists(t *testing.T) { + ctx := context.Background() + + t.Run("exists", func(t *testing.T) { + fc := &FakeClient{ + OrgSecrets: map[string]bool{"myorg/TOKEN": true}, + } + exists, err := fc.OrgSecretExists(ctx, "myorg", "TOKEN") + require.NoError(t, err) + assert.True(t, exists) + }) + + t.Run("not exists", func(t *testing.T) { + fc := &FakeClient{ + OrgSecrets: map[string]bool{}, + } + exists, err := fc.OrgSecretExists(ctx, "myorg", "MISSING") + require.NoError(t, err) + assert.False(t, exists) + }) + + t.Run("nil map", func(t *testing.T) { + fc := &FakeClient{} + exists, err := fc.OrgSecretExists(ctx, "myorg", "TOKEN") + require.NoError(t, err) + assert.False(t, exists) + }) +} + +func TestFakeClient_CreateOrgSecret(t *testing.T) { + ctx := context.Background() + fc := &FakeClient{} + + err := fc.CreateOrgSecret(ctx, "myorg", "DISPATCH_TOKEN", "secret-value", []int64{100, 200}) + require.NoError(t, err) + + // Should be recorded. + require.Len(t, fc.CreatedOrgSecrets, 1) + assert.Equal(t, "myorg", fc.CreatedOrgSecrets[0].Org) + assert.Equal(t, "DISPATCH_TOKEN", fc.CreatedOrgSecrets[0].Name) + assert.Equal(t, "secret-value", fc.CreatedOrgSecrets[0].Value) + assert.Equal(t, []int64{100, 200}, fc.CreatedOrgSecrets[0].RepoIDs) + + // Should be queryable. + exists, err := fc.OrgSecretExists(ctx, "myorg", "DISPATCH_TOKEN") + require.NoError(t, err) + assert.True(t, exists) +} + func TestFakeClient_ErrorInjection(t *testing.T) { ctx := context.Background() injected := errors.New("injected error") @@ -292,6 +341,17 @@ func TestFakeClient_ErrorInjection(t *testing.T) { _, err := fc.ListOrgInstallations(ctx, "org") return err }}, + {"CreateOrgSecret", func(fc *FakeClient) error { + return fc.CreateOrgSecret(ctx, "o", "n", "v", nil) + }}, + {"OrgSecretExists", func(fc *FakeClient) error { + _, err := fc.OrgSecretExists(ctx, "o", "n") + return err + }}, + {"DeleteOrgSecret", func(fc *FakeClient) error { return fc.DeleteOrgSecret(ctx, "o", "n") }}, + {"SetOrgSecretRepos", func(fc *FakeClient) error { + return fc.SetOrgSecretRepos(ctx, "o", "n", nil) + }}, } for _, m := range methods { @@ -320,6 +380,7 @@ func TestFakeClient_ThreadSafety(t *testing.T) { }, Installations: []Installation{{ID: 1, AppSlug: "app"}}, Secrets: map[string]bool{"o/r/secret": true}, + OrgSecrets: map[string]bool{"o/secret": true}, } var wg sync.WaitGroup @@ -347,6 +408,10 @@ func TestFakeClient_ThreadSafety(t *testing.T) { _, _ = fc.GetLatestWorkflowRun(ctx, "o", "r", "ci.yml") _, _ = fc.GetWorkflowRun(ctx, "o", "r", 1) _, _ = fc.ListOrgInstallations(ctx, "org") + _ = fc.CreateOrgSecret(ctx, "o", "n", "v", []int64{1}) + _, _ = fc.OrgSecretExists(ctx, "o", "secret") + _ = fc.DeleteOrgSecret(ctx, "o", "n") + _ = fc.SetOrgSecretRepos(ctx, "o", "n", []int64{1, 2}) }(i) } diff --git a/internal/forge/forge.go b/internal/forge/forge.go index 9b493e5bd8..329cec531f 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -22,6 +22,7 @@ func IsNotFound(err error) bool { // Repository represents a repository on a git forge. type Repository struct { + ID int64 Name string FullName string DefaultBranch string @@ -65,12 +66,22 @@ type Client interface { // File operations CreateFile(ctx context.Context, owner, repo, path, message string, content []byte) error + + // CreateOrUpdateFile creates a file or updates it if it already exists. + // On GitHub, updating an existing file requires the current file's SHA + // (optimistic concurrency control). The GitHub implementation handles + // this by fetching the existing SHA before writing. Without it, the + // API returns a 422 "sha wasn't supplied" error. CreateOrUpdateFile(ctx context.Context, owner, repo, path, message string, content []byte) error + GetFileContent(ctx context.Context, owner, repo, path string) ([]byte, error) // Branch operations CreateBranch(ctx context.Context, owner, repo, branchName string) error CreateFileOnBranch(ctx context.Context, owner, repo, branch, path, message string, content []byte) error + // CreateOrUpdateFileOnBranch creates or updates a file on a specific branch. + // Combines SHA-aware upsert with branch targeting. + CreateOrUpdateFileOnBranch(ctx context.Context, owner, repo, branch, path, message string, content []byte) error // Change proposals (PRs/MRs) CreateChangeProposal(ctx context.Context, owner, repo, title, body, head, base string) (*ChangeProposal, error) @@ -79,15 +90,27 @@ type Client interface { // Authentication GetAuthenticatedUser(ctx context.Context) (string, error) + // GetTokenScopes returns the OAuth scopes granted to the current token. + // On GitHub, this is read from the X-OAuth-Scopes response header. + // Returns nil (not an error) if the forge doesn't support scope introspection. + GetTokenScopes(ctx context.Context) ([]string, error) + // Secrets and variables CreateRepoSecret(ctx context.Context, owner, repo, name, value string) error RepoSecretExists(ctx context.Context, owner, repo, name string) (bool, error) CreateOrUpdateRepoVariable(ctx context.Context, owner, repo, name, value string) error RepoVariableExists(ctx context.Context, owner, repo, name string) (bool, error) + // Org-level secrets (for cross-repo dispatch tokens) + CreateOrgSecret(ctx context.Context, org, name, value string, selectedRepoIDs []int64) error + OrgSecretExists(ctx context.Context, org, name string) (bool, error) + DeleteOrgSecret(ctx context.Context, org, name string) error + SetOrgSecretRepos(ctx context.Context, org, name string, repoIDs []int64) error + // CI/Workflow operations GetLatestWorkflowRun(ctx context.Context, owner, repo, workflowFile string) (*WorkflowRun, error) GetWorkflowRun(ctx context.Context, owner, repo string, runID int) (*WorkflowRun, error) + DispatchWorkflow(ctx context.Context, owner, repo, workflowFile, ref string, inputs map[string]string) error // App installation operations ListOrgInstallations(ctx context.Context, org string) ([]Installation, error) diff --git a/internal/forge/github/github.go b/internal/forge/github/github.go index 9a0fc1b751..4f57a7491f 100644 --- a/internal/forge/github/github.go +++ b/internal/forge/github/github.go @@ -247,6 +247,7 @@ func (c *LiveClient) ListOrgRepos(ctx context.Context, org string) ([]forge.Repo } var repos []struct { + ID int64 `json:"id"` Name string `json:"name"` FullName string `json:"full_name"` DefaultBranch string `json:"default_branch"` @@ -263,6 +264,7 @@ func (c *LiveClient) ListOrgRepos(ctx context.Context, org string) ([]forge.Repo continue } result = append(result, forge.Repository{ + ID: r.ID, Name: r.Name, FullName: r.FullName, DefaultBranch: r.DefaultBranch, @@ -281,6 +283,12 @@ func (c *LiveClient) ListOrgRepos(ctx context.Context, org string) ([]forge.Repo } // CreateRepo creates a new repository under an organization. +// +// The repo is created with auto_init: true so that a default branch exists +// immediately. However, GitHub's auto_init is asynchronous — the API returns +// 201 before the initial commit is fully materialized. Callers writing files +// to the new repo via the Contents API should expect transient 404s and +// retry with backoff. See the retry logic in LiveClient.do(). func (c *LiveClient) CreateRepo(ctx context.Context, org, name, description string, private bool) (*forge.Repository, error) { payload := map[string]any{ "name": name, @@ -324,6 +332,7 @@ func (c *LiveClient) GetRepo(ctx context.Context, owner, repo string) (*forge.Re } var r struct { + ID int64 `json:"id"` Name string `json:"name"` FullName string `json:"full_name"` DefaultBranch string `json:"default_branch"` @@ -336,6 +345,7 @@ func (c *LiveClient) GetRepo(ctx context.Context, owner, repo string) (*forge.Re } return &forge.Repository{ + ID: r.ID, Name: r.Name, FullName: r.FullName, DefaultBranch: r.DefaultBranch, @@ -356,6 +366,15 @@ func (c *LiveClient) CreateFile(ctx context.Context, owner, repo, path, message } // CreateFileOnBranch creates a file on a specific branch (or default if empty). +// +// Retries on 404 to handle GitHub's async repo initialization: after +// CreateRepo with auto_init, the default branch may not be materialized +// yet and the Contents API returns 404. Also retries on 409 (conflict) +// which can occur when the branch ref is being updated by a concurrent write. +// +// GitHub quirk: writing to .github/workflows/ paths returns 404 (not 403) +// when the token lacks the "workflow" scope. If you hit persistent 404s +// on workflow file creation, the fix is: gh auth refresh -s workflow func (c *LiveClient) CreateFileOnBranch(ctx context.Context, owner, repo, branch, path, message string, content []byte) error { payload := map[string]any{ "message": message, @@ -365,52 +384,130 @@ func (c *LiveClient) CreateFileOnBranch(ctx context.Context, owner, repo, branch payload["branch"] = branch } - resp, err := c.put(ctx, fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, path), payload) - if err != nil { - return fmt.Errorf("create file %s: %w", path, err) - } - resp.Body.Close() - return nil + apiPath := fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, path) + return c.putFileWithRetry(ctx, apiPath, payload, path) } // CreateOrUpdateFile creates a file or updates it if it already exists. +// Retries on 404/409 to handle async repo initialization and branch ref races. func (c *LiveClient) CreateOrUpdateFile(ctx context.Context, owner, repo, path, message string, content []byte) error { - // Try to get existing file for its SHA. apiPath := fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, path) - existingResp, err := c.do(ctx, http.MethodGet, apiPath, nil) - if err != nil { - return fmt.Errorf("check existing file: %w", err) - } - payload := map[string]any{ - "message": message, - "content": base64.StdEncoding.EncodeToString(content), - } + return c.retryOnTransient(ctx, path, func() error { + // Try to get existing file for its SHA. + existingResp, err := c.do(ctx, http.MethodGet, apiPath, nil) + if err != nil { + return fmt.Errorf("check existing file: %w", err) + } - switch existingResp.StatusCode { - case http.StatusOK: - var existing struct { - SHA string `json:"sha"` + payload := map[string]any{ + "message": message, + "content": base64.StdEncoding.EncodeToString(content), + } + + if existingResp.StatusCode == http.StatusOK { + var existing struct { + SHA string `json:"sha"` + } + if err := decodeJSON(existingResp, &existing); err != nil { + return fmt.Errorf("decode existing file: %w", err) + } + payload["sha"] = existing.SHA + } else { + existingResp.Body.Close() } - if err := decodeJSON(existingResp, &existing); err != nil { - return fmt.Errorf("decode existing file: %w", err) + + resp, err := c.put(ctx, apiPath, payload) + if err != nil { + return fmt.Errorf("create or update file %s: %w", path, err) } - payload["sha"] = existing.SHA - case http.StatusNotFound: - // File doesn't exist yet — create without SHA. - existingResp.Body.Close() - default: - // Unexpected status — surface as an error. - defer existingResp.Body.Close() - return checkStatus(existingResp, http.StatusOK, http.StatusNotFound) - } + resp.Body.Close() + return nil + }) +} - resp, err := c.put(ctx, apiPath, payload) - if err != nil { - return fmt.Errorf("create or update file %s: %w", path, err) +// CreateOrUpdateFileOnBranch creates or updates a file on a specific branch. +// Like CreateOrUpdateFile, it fetches the existing SHA before updating. +// Retries on 404/409 for async repo init and branch ref races. +func (c *LiveClient) CreateOrUpdateFileOnBranch(ctx context.Context, owner, repo, branch, path, message string, content []byte) error { + apiPath := fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, path) + + return c.retryOnTransient(ctx, path, func() error { + // Try to get existing file on the branch for its SHA. + existingResp, err := c.do(ctx, http.MethodGet, apiPath+"?ref="+branch, nil) + if err != nil { + return fmt.Errorf("check existing file on branch: %w", err) + } + + payload := map[string]any{ + "message": message, + "content": base64.StdEncoding.EncodeToString(content), + "branch": branch, + } + + if existingResp.StatusCode == http.StatusOK { + var existing struct { + SHA string `json:"sha"` + } + if err := decodeJSON(existingResp, &existing); err != nil { + return fmt.Errorf("decode existing file: %w", err) + } + payload["sha"] = existing.SHA + } else { + existingResp.Body.Close() + } + + resp, err := c.put(ctx, apiPath, payload) + if err != nil { + return fmt.Errorf("create or update file %s on branch %s: %w", path, branch, err) + } + resp.Body.Close() + return nil + }) +} + +// putFileWithRetry wraps a single PUT to the Contents API with retry on +// transient errors (404 from async repo init, 409 from branch ref races). +func (c *LiveClient) putFileWithRetry(ctx context.Context, apiPath string, payload map[string]any, path string) error { + return c.retryOnTransient(ctx, path, func() error { + resp, err := c.put(ctx, apiPath, payload) + if err != nil { + return fmt.Errorf("create file %s: %w", path, err) + } + resp.Body.Close() + return nil + }) +} + +// retryOnTransient retries an operation that may fail with 404 or 409 due to +// GitHub's async repo initialization or branch ref update races. It uses +// linear backoff (2s between attempts) and up to 5 attempts (~10s total). +func (c *LiveClient) retryOnTransient(ctx context.Context, label string, fn func() error) error { + const attempts = 5 + const delay = 2 * time.Second + + var lastErr error + for i := range attempts { + lastErr = fn() + if lastErr == nil { + return nil + } + + // Only retry on 404 (repo not ready) or 409 (branch ref conflict). + var apiErr *APIError + if !errors.As(lastErr, &apiErr) || (apiErr.StatusCode != 404 && apiErr.StatusCode != 409) { + return lastErr + } + + if i < attempts-1 { + select { + case <-time.After(delay): + case <-ctx.Done(): + return ctx.Err() + } + } } - resp.Body.Close() - return nil + return fmt.Errorf("%s: %w (after %d attempts)", label, lastErr, attempts) } // GetFileContent retrieves the content of a file from a repository. @@ -427,10 +524,7 @@ func (c *LiveClient) GetFileContent(ctx context.Context, owner, repo, path strin return nil, fmt.Errorf("decode file content: %w", err) } - // GitHub's Contents API returns base64 content with newlines for line - // wrapping. Strip them before decoding. - cleaned := strings.ReplaceAll(file.Content, "\n", "") - data, err := base64.StdEncoding.DecodeString(cleaned) + data, err := base64.StdEncoding.DecodeString(file.Content) if err != nil { return nil, fmt.Errorf("decode base64 content: %w", err) } @@ -559,8 +653,36 @@ func (c *LiveClient) GetAuthenticatedUser(ctx context.Context) (string, error) { return user.Login, nil } +// GetTokenScopes returns the OAuth scopes granted to the current token +// by inspecting the X-OAuth-Scopes header from a lightweight API call. +func (c *LiveClient) GetTokenScopes(ctx context.Context) ([]string, error) { + resp, err := c.do(ctx, http.MethodHead, "/user", nil) + if err != nil { + return nil, fmt.Errorf("checking token scopes: %w", err) + } + io.Copy(io.Discard, resp.Body) + resp.Body.Close() + + header := resp.Header.Get("X-OAuth-Scopes") + if header == "" { + // Fine-grained tokens and GitHub App tokens don't have this header. + // Return nil to indicate scope introspection isn't available. + return nil, nil + } + + var scopes []string + for _, s := range strings.Split(header, ",") { + s = strings.TrimSpace(s) + if s != "" { + scopes = append(scopes, s) + } + } + return scopes, nil +} + // CreateRepoSecret creates or updates an encrypted repository secret. func (c *LiveClient) CreateRepoSecret(ctx context.Context, owner, repo, name, value string) error { + value = strings.TrimSpace(value) // Step 1: Get the repo's public key for secret encryption. keyResp, err := c.get(ctx, fmt.Sprintf("/repos/%s/%s/actions/secrets/public-key", owner, repo)) if err != nil { @@ -731,6 +853,24 @@ func (c *LiveClient) GetWorkflowRun(ctx context.Context, owner, repo string, run }, nil } +// DispatchWorkflow triggers a workflow_dispatch event on a workflow file. +// GitHub returns 204 No Content on success (not 200 or 201). +func (c *LiveClient) DispatchWorkflow(ctx context.Context, owner, repo, workflowFile, ref string, inputs map[string]string) error { + payload := map[string]any{ + "ref": ref, + "inputs": inputs, + } + resp, err := c.do(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/actions/workflows/%s/dispatches", owner, repo, workflowFile), payload) + if err != nil { + return fmt.Errorf("dispatch workflow %s: %w", workflowFile, err) + } + if err := checkStatus(resp, http.StatusNoContent); err != nil { + return fmt.Errorf("dispatch workflow %s: %w", workflowFile, err) + } + resp.Body.Close() + return nil +} + // ListOrgInstallations lists app installations for an organization. func (c *LiveClient) ListOrgInstallations(ctx context.Context, org string) ([]forge.Installation, error) { resp, err := c.get(ctx, fmt.Sprintf("/orgs/%s/installations?per_page=100", org)) @@ -760,6 +900,110 @@ func (c *LiveClient) ListOrgInstallations(ctx context.Context, org string) ([]fo return installs, nil } +// CreateOrgSecret creates or updates an encrypted organization-level secret +// scoped to the given repository IDs. +// The value is trimmed of whitespace before encryption to prevent corruption +// from stray newlines or carriage returns in pasted input. +func (c *LiveClient) CreateOrgSecret(ctx context.Context, org, name, value string, selectedRepoIDs []int64) error { + value = strings.TrimSpace(value) + // Step 1: Get the org's public key for secret encryption. + keyResp, err := c.get(ctx, fmt.Sprintf("/orgs/%s/actions/secrets/public-key", org)) + if err != nil { + return fmt.Errorf("get org public key: %w", err) + } + + var pubKey struct { + KeyID string `json:"key_id"` + Key string `json:"key"` + } + if err := decodeJSON(keyResp, &pubKey); err != nil { + return fmt.Errorf("decode org public key: %w", err) + } + + // Step 2: Decode the public key and encrypt the secret value. + keyBytes, err := base64.StdEncoding.DecodeString(pubKey.Key) + if err != nil { + return fmt.Errorf("decode org public key base64: %w", err) + } + + var recipientKey [32]byte + copy(recipientKey[:], keyBytes) + + encrypted, err := box.SealAnonymous(nil, []byte(value), &recipientKey, nil) + if err != nil { + return fmt.Errorf("encrypt org secret: %w", err) + } + + // Step 3: Upload the encrypted secret with selected repo visibility. + payload := map[string]any{ + "encrypted_value": base64.StdEncoding.EncodeToString(encrypted), + "key_id": pubKey.KeyID, + "visibility": "selected", + "selected_repository_ids": selectedRepoIDs, + } + + resp, err := c.put(ctx, fmt.Sprintf("/orgs/%s/actions/secrets/%s", org, name), payload) + if err != nil { + return fmt.Errorf("create org secret %s: %w", name, err) + } + resp.Body.Close() + return nil +} + +// OrgSecretExists checks if an org-level secret exists. +func (c *LiveClient) OrgSecretExists(ctx context.Context, org, name string) (bool, error) { + resp, err := c.do(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/actions/secrets/%s", org, name), nil) + if err != nil { + return false, fmt.Errorf("check org secret %s: %w", name, err) + } + resp.Body.Close() + + switch resp.StatusCode { + case http.StatusOK: + return true, nil + case http.StatusNotFound: + return false, nil + case http.StatusForbidden: + // 403 means the token doesn't have permission to check org secrets. + // Treat as "unknown" (false) rather than a hard error — the preflight + // should have caught missing admin:org scope before we get here. If it + // didn't (e.g., fine-grained token without scope introspection), we'll + // attempt to create the secret and get a clear error at that point. + return false, nil + default: + return false, &APIError{StatusCode: resp.StatusCode, Message: "unexpected status checking org secret"} + } +} + +// DeleteOrgSecret deletes an org-level secret. It is idempotent: a 404 +// (secret already gone) is not treated as an error. +func (c *LiveClient) DeleteOrgSecret(ctx context.Context, org, name string) error { + resp, err := c.do(ctx, http.MethodDelete, fmt.Sprintf("/orgs/%s/actions/secrets/%s", org, name), nil) + if err != nil { + return fmt.Errorf("delete org secret %s: %w", name, err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusNoContent || resp.StatusCode == http.StatusNotFound { + return nil + } + return &APIError{StatusCode: resp.StatusCode, Message: "unexpected status deleting org secret"} +} + +// SetOrgSecretRepos sets the list of repositories that can access an org secret. +func (c *LiveClient) SetOrgSecretRepos(ctx context.Context, org, name string, repoIDs []int64) error { + payload := map[string]any{ + "selected_repository_ids": repoIDs, + } + + resp, err := c.put(ctx, fmt.Sprintf("/orgs/%s/actions/secrets/%s/repositories", org, name), payload) + if err != nil { + return fmt.Errorf("set org secret repos for %s: %w", name, err) + } + resp.Body.Close() + return nil +} + // isNotFound checks whether an error is a 404 API error. func isNotFound(err error) bool { var apiErr *APIError diff --git a/internal/forge/github/github_test.go b/internal/forge/github/github_test.go index 918fd3db32..482774ed53 100644 --- a/internal/forge/github/github_test.go +++ b/internal/forge/github/github_test.go @@ -499,8 +499,8 @@ func TestListOrgInstallations(t *testing.T) { json.NewEncoder(w).Encode(map[string]any{ "installations": []map[string]any{ - {"id": 1, "app_id": 100, "app_slug": "fullsend-myorg"}, - {"id": 2, "app_id": 200, "app_slug": "fullsend-myorg-triage"}, + {"id": 1, "app_id": 100, "app_slug": "myorg-fullsend"}, + {"id": 2, "app_id": 200, "app_slug": "myorg-triage"}, }, }) })) @@ -511,7 +511,7 @@ func TestListOrgInstallations(t *testing.T) { require.NoError(t, err) require.Len(t, installs, 2) assert.Equal(t, 1, installs[0].ID) - assert.Equal(t, "fullsend-myorg", installs[0].AppSlug) + assert.Equal(t, "myorg-fullsend", installs[0].AppSlug) assert.Equal(t, 200, installs[1].AppID) } @@ -580,6 +580,133 @@ func TestWithBaseURL(t *testing.T) { assert.Equal(t, "https://custom.api.com", client.baseURL) } +func TestCreateOrgSecret(t *testing.T) { + callNum := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callNum++ + switch callNum { + case 1: + // GET org public key + assert.Equal(t, "GET", r.Method) + assert.Equal(t, "/orgs/myorg/actions/secrets/public-key", r.URL.Path) + + pubKey := make([]byte, 32) + for i := range pubKey { + pubKey[i] = byte(i + 1) + } + + json.NewEncoder(w).Encode(map[string]any{ + "key_id": "org-key-123", + "key": base64.StdEncoding.EncodeToString(pubKey), + }) + case 2: + // PUT org secret + assert.Equal(t, "PUT", r.Method) + assert.Equal(t, "/orgs/myorg/actions/secrets/DISPATCH_TOKEN", r.URL.Path) + + var body map[string]any + json.NewDecoder(r.Body).Decode(&body) + assert.Equal(t, "org-key-123", body["key_id"]) + assert.NotEmpty(t, body["encrypted_value"]) + assert.Equal(t, "selected", body["visibility"]) + + repoIDs, ok := body["selected_repository_ids"].([]any) + require.True(t, ok) + assert.Len(t, repoIDs, 2) + assert.Equal(t, float64(100), repoIDs[0]) + assert.Equal(t, float64(200), repoIDs[1]) + + w.WriteHeader(http.StatusCreated) + } + })) + defer srv.Close() + + client := newTestClient(t, srv) + err := client.CreateOrgSecret(context.Background(), "myorg", "DISPATCH_TOKEN", "token-value", []int64{100, 200}) + require.NoError(t, err) +} + +func TestOrgSecretExists(t *testing.T) { + t.Run("exists", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "GET", r.Method) + assert.Equal(t, "/orgs/myorg/actions/secrets/TOKEN", r.URL.Path) + json.NewEncoder(w).Encode(map[string]any{"name": "TOKEN"}) + })) + defer srv.Close() + + client := newTestClient(t, srv) + exists, err := client.OrgSecretExists(context.Background(), "myorg", "TOKEN") + require.NoError(t, err) + assert.True(t, exists) + }) + + t.Run("not exists", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + json.NewEncoder(w).Encode(map[string]any{"message": "Not Found"}) + })) + defer srv.Close() + + client := newTestClient(t, srv) + exists, err := client.OrgSecretExists(context.Background(), "myorg", "MISSING") + require.NoError(t, err) + assert.False(t, exists) + }) +} + +func TestDeleteOrgSecret(t *testing.T) { + t.Run("success", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "DELETE", r.Method) + assert.Equal(t, "/orgs/myorg/actions/secrets/TOKEN", r.URL.Path) + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + client := newTestClient(t, srv) + err := client.DeleteOrgSecret(context.Background(), "myorg", "TOKEN") + require.NoError(t, err) + }) + + t.Run("idempotent 404", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "DELETE", r.Method) + w.WriteHeader(http.StatusNotFound) + json.NewEncoder(w).Encode(map[string]any{"message": "Not Found"}) + })) + defer srv.Close() + + client := newTestClient(t, srv) + err := client.DeleteOrgSecret(context.Background(), "myorg", "ALREADY_GONE") + require.NoError(t, err) + }) +} + +func TestSetOrgSecretRepos(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "PUT", r.Method) + assert.Equal(t, "/orgs/myorg/actions/secrets/TOKEN/repositories", r.URL.Path) + + var body map[string]any + json.NewDecoder(r.Body).Decode(&body) + + repoIDs, ok := body["selected_repository_ids"].([]any) + require.True(t, ok) + assert.Len(t, repoIDs, 3) + assert.Equal(t, float64(10), repoIDs[0]) + assert.Equal(t, float64(20), repoIDs[1]) + assert.Equal(t, float64(30), repoIDs[2]) + + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + client := newTestClient(t, srv) + err := client.SetOrgSecretRepos(context.Background(), "myorg", "TOKEN", []int64{10, 20, 30}) + require.NoError(t, err) +} + func TestListOrgRepos_Pagination(t *testing.T) { page := 0 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/forge/github/types.go b/internal/forge/github/types.go index 20955df82e..c4485bdd21 100644 --- a/internal/forge/github/types.go +++ b/internal/forge/github/types.go @@ -12,13 +12,24 @@ type AppPermissions struct { Members string `json:"members,omitempty"` } -// AppConfig defines the configuration for creating a GitHub App. +// HookAttributes configures the webhook for a GitHub App. +// Even when webhooks are not used, GitHub requires this field in the manifest. +type HookAttributes struct { + URL string `json:"url"` + Active bool `json:"active"` +} + +// AppConfig defines the configuration for creating a GitHub App via the +// manifest flow. See https://docs.github.com/en/apps/sharing-github-apps/registering-a-github-app-from-a-manifest type AppConfig struct { - Name string `json:"name"` - Description string `json:"description"` - URL string `json:"url"` - Permissions AppPermissions `json:"default_permissions"` - Events []string `json:"default_events"` + Name string `json:"name"` + Description string `json:"description"` + URL string `json:"url"` + HookAttributes HookAttributes `json:"hook_attributes"` + RedirectURL string `json:"redirect_url,omitempty"` + Public bool `json:"public"` + Permissions AppPermissions `json:"default_permissions"` + Events []string `json:"default_events"` } // DefaultAgentRoles returns the standard set of agent roles. @@ -27,14 +38,28 @@ func DefaultAgentRoles() []string { } // AgentAppConfig returns the GitHub App configuration for a given agent role. +// +// Important: GitHub validates that event subscriptions are backed by matching +// permissions. For example, subscribing to "issues" events requires at least +// issues:read permission. Subscribing to "issue_comment" requires issues:read +// or issues:write. Mismatches cause the manifest to be rejected. Every Events +// entry below must have a corresponding permission. func AgentAppConfig(org, role string) AppConfig { base := AppConfig{ URL: fmt.Sprintf("https://github.com/%s", org), + // hook_attributes is required by the manifest spec even when we + // don't use webhooks. Setting active: false disables delivery. + HookAttributes: HookAttributes{ + URL: fmt.Sprintf("https://github.com/%s", org), + Active: false, + }, } + // App naming convention: - for all roles. + base.Name = fmt.Sprintf("%s-%s", org, role) + switch role { case "fullsend": - base.Name = fmt.Sprintf("fullsend-%s", org) base.Description = fmt.Sprintf("Fullsend orchestrator for %s", org) base.Permissions = AppPermissions{ Contents: "write", @@ -47,7 +72,6 @@ func AgentAppConfig(org, role string) AppConfig { base.Events = []string{"issues", "push", "workflow_dispatch"} case "triage": - base.Name = fmt.Sprintf("fullsend-%s-triage", org) base.Description = fmt.Sprintf("Fullsend triage agent for %s", org) base.Permissions = AppPermissions{ Issues: "write", @@ -55,7 +79,6 @@ func AgentAppConfig(org, role string) AppConfig { base.Events = []string{"issues", "issue_comment"} case "coder": - base.Name = fmt.Sprintf("fullsend-%s-coder", org) base.Description = fmt.Sprintf("Fullsend coder agent for %s", org) base.Permissions = AppPermissions{ Issues: "read", @@ -66,7 +89,6 @@ func AgentAppConfig(org, role string) AppConfig { base.Events = []string{"issues", "issue_comment", "pull_request", "check_run", "check_suite"} case "review": - base.Name = fmt.Sprintf("fullsend-%s-review", org) base.Description = fmt.Sprintf("Fullsend review agent for %s", org) base.Permissions = AppPermissions{ PullRequests: "write", @@ -76,7 +98,6 @@ func AgentAppConfig(org, role string) AppConfig { base.Events = []string{"pull_request", "pull_request_review"} default: - base.Name = fmt.Sprintf("fullsend-%s-%s", org, role) base.Description = fmt.Sprintf("Fullsend %s agent for %s", role, org) base.Permissions = AppPermissions{ Issues: "read", diff --git a/internal/forge/github/types_test.go b/internal/forge/github/types_test.go index 4155cb7a3f..41faef8c79 100644 --- a/internal/forge/github/types_test.go +++ b/internal/forge/github/types_test.go @@ -16,7 +16,7 @@ func TestDefaultAgentRoles(t *testing.T) { func TestAgentAppConfig_Fullsend(t *testing.T) { cfg := AgentAppConfig("myorg", "fullsend") - assert.Equal(t, "fullsend-myorg", cfg.Name) + assert.Equal(t, "myorg-fullsend", cfg.Name) assert.NotEmpty(t, cfg.Description) assert.NotEmpty(t, cfg.URL) @@ -35,7 +35,7 @@ func TestAgentAppConfig_Fullsend(t *testing.T) { func TestAgentAppConfig_Triage(t *testing.T) { cfg := AgentAppConfig("myorg", "triage") - assert.Equal(t, "fullsend-myorg-triage", cfg.Name) + assert.Equal(t, "myorg-triage", cfg.Name) assert.Equal(t, "write", cfg.Permissions.Issues) assert.Empty(t, cfg.Permissions.Contents) @@ -46,7 +46,7 @@ func TestAgentAppConfig_Triage(t *testing.T) { func TestAgentAppConfig_Coder(t *testing.T) { cfg := AgentAppConfig("myorg", "coder") - assert.Equal(t, "fullsend-myorg-coder", cfg.Name) + assert.Equal(t, "myorg-coder", cfg.Name) assert.Equal(t, "read", cfg.Permissions.Issues) assert.Equal(t, "write", cfg.Permissions.Contents) assert.Equal(t, "write", cfg.Permissions.PullRequests) @@ -62,7 +62,7 @@ func TestAgentAppConfig_Coder(t *testing.T) { func TestAgentAppConfig_Review(t *testing.T) { cfg := AgentAppConfig("myorg", "review") - assert.Equal(t, "fullsend-myorg-review", cfg.Name) + assert.Equal(t, "myorg-review", cfg.Name) assert.Equal(t, "write", cfg.Permissions.PullRequests) assert.Equal(t, "read", cfg.Permissions.Contents) assert.Equal(t, "read", cfg.Permissions.Checks) @@ -74,7 +74,7 @@ func TestAgentAppConfig_Review(t *testing.T) { func TestAgentAppConfig_UnknownRole(t *testing.T) { cfg := AgentAppConfig("myorg", "custom-bot") - assert.Equal(t, "fullsend-myorg-custom-bot", cfg.Name) + assert.Equal(t, "myorg-custom-bot", cfg.Name) assert.Equal(t, "read", cfg.Permissions.Issues) assert.Empty(t, cfg.Permissions.Contents) assert.Empty(t, cfg.Permissions.PullRequests) diff --git a/internal/layers/configrepo.go b/internal/layers/configrepo.go index 8a2e0ecd53..4b9f81e522 100644 --- a/internal/layers/configrepo.go +++ b/internal/layers/configrepo.go @@ -26,8 +26,11 @@ type ConfigRepoLayer struct { var _ Layer = (*ConfigRepoLayer)(nil) // NewConfigRepoLayer creates a new ConfigRepoLayer. -// Set hasPrivate to true if the org has private repo capability — -// the config repo will be created as private in that case. +// Set hasPrivate to true if the org has any private repos — the config repo +// will be created as private to match the org's existing pattern. For orgs +// with only public repos (e.g., open source orgs), it is created as public +// to avoid surprises. This matters because the .fullsend repo may contain +// workflow files referenced by public repos. func NewConfigRepoLayer(org string, client forge.Client, cfg *config.OrgConfig, printer *ui.Printer, hasPrivate bool) *ConfigRepoLayer { return &ConfigRepoLayer{ org: org, @@ -42,8 +45,30 @@ func (l *ConfigRepoLayer) Name() string { return "config-repo" } +// RequiredScopes returns the scopes needed for the given operation. +func (l *ConfigRepoLayer) RequiredScopes(op Operation) []string { + switch op { + case OpInstall: + return []string{"repo"} + case OpUninstall: + // Deleting the config repo requires the delete_repo scope, which + // most tokens don't have by default. Fail early with a clear message. + return []string{"repo", "delete_repo"} + case OpAnalyze: + return []string{"repo"} + default: + return nil + } +} + // Install creates the .fullsend config repo (if it doesn't exist) and // writes config.yaml into it. +// +// Timing note: after CreateRepo with auto_init, the default branch may not +// be fully materialized yet. The Contents API call to write config.yaml can +// get transient 404s. The GitHub client's retry-with-backoff in do() handles +// this, but callers should be aware that the first file write to a newly +// created repo may take several seconds to succeed. func (l *ConfigRepoLayer) Install(ctx context.Context) error { exists, err := l.repoExists(ctx) if err != nil { @@ -55,10 +80,18 @@ func (l *ConfigRepoLayer) Install(ctx context.Context) error { desc := fmt.Sprintf("fullsend configuration for %s", l.org) _, err := l.client.CreateRepo(ctx, l.org, forge.ConfigRepoName, desc, l.hasPrivate) if err != nil { - l.ui.StepFail("Failed to create " + forge.ConfigRepoName + " repository") - return fmt.Errorf("creating config repo: %w", err) + // Idempotent: if the repo was created between our check and this + // call (race), or if we got an "already exists" error, proceed. + recheck, recheckErr := l.repoExists(ctx) + if recheckErr == nil && recheck { + l.ui.StepInfo(forge.ConfigRepoName + " repository already exists") + } else { + l.ui.StepFail("Failed to create " + forge.ConfigRepoName + " repository") + return fmt.Errorf("creating config repo: %w", err) + } + } else { + l.ui.StepDone("Created " + forge.ConfigRepoName + " repository") } - l.ui.StepDone("Created " + forge.ConfigRepoName + " repository") } else { l.ui.StepInfo(forge.ConfigRepoName + " repository already exists") } @@ -81,9 +114,24 @@ func (l *ConfigRepoLayer) Install(ctx context.Context) error { } // Uninstall deletes the .fullsend config repo. +// Idempotent: if the repo is already gone, this is a no-op. func (l *ConfigRepoLayer) Uninstall(ctx context.Context) error { + exists, err := l.repoExists(ctx) + if err != nil { + return fmt.Errorf("checking for config repo: %w", err) + } + if !exists { + l.ui.StepInfo(forge.ConfigRepoName + " repository already deleted") + return nil + } + l.ui.StepStart("Deleting " + forge.ConfigRepoName + " repository") if err := l.client.DeleteRepo(ctx, l.org, forge.ConfigRepoName); err != nil { + if forge.IsNotFound(err) { + // Race: deleted between our check and the delete call. + l.ui.StepInfo(forge.ConfigRepoName + " repository already deleted") + return nil + } l.ui.StepFail("Failed to delete " + forge.ConfigRepoName + " repository") return fmt.Errorf("deleting config repo: %w", err) } diff --git a/internal/layers/configrepo_test.go b/internal/layers/configrepo_test.go index 169451155a..2d4fa65151 100644 --- a/internal/layers/configrepo_test.go +++ b/internal/layers/configrepo_test.go @@ -129,7 +129,11 @@ func TestConfigRepoLayer_Install_CreateRepoError(t *testing.T) { } func TestConfigRepoLayer_Uninstall_DeletesRepo(t *testing.T) { - client := &forge.FakeClient{} + client := &forge.FakeClient{ + Repos: []forge.Repository{ + {Name: ".fullsend", FullName: "test-org/.fullsend"}, + }, + } layer, _ := newTestLayer(t, client, false) err := layer.Uninstall(context.Background()) @@ -139,15 +143,29 @@ func TestConfigRepoLayer_Uninstall_DeletesRepo(t *testing.T) { assert.Equal(t, "test-org/.fullsend", client.DeletedRepos[0]) } +func TestConfigRepoLayer_Uninstall_AlreadyDeleted(t *testing.T) { + // Repo doesn't exist — uninstall should be a no-op, not an error. + client := &forge.FakeClient{} + layer, _ := newTestLayer(t, client, false) + + err := layer.Uninstall(context.Background()) + require.NoError(t, err) + + assert.Empty(t, client.DeletedRepos, "should not attempt to delete a missing repo") +} + func TestConfigRepoLayer_Uninstall_Error(t *testing.T) { client := &forge.FakeClient{ - Errors: map[string]error{"DeleteRepo": errors.New("not found")}, + Repos: []forge.Repository{ + {Name: ".fullsend", FullName: "test-org/.fullsend"}, + }, + Errors: map[string]error{"DeleteRepo": errors.New("permission denied")}, } layer, _ := newTestLayer(t, client, false) err := layer.Uninstall(context.Background()) require.Error(t, err) - assert.Contains(t, err.Error(), "not found") + assert.Contains(t, err.Error(), "permission denied") } func TestConfigRepoLayer_Analyze_NotInstalled(t *testing.T) { diff --git a/internal/layers/dispatch.go b/internal/layers/dispatch.go new file mode 100644 index 0000000000..3abe1bc2fc --- /dev/null +++ b/internal/layers/dispatch.go @@ -0,0 +1,123 @@ +package layers + +import ( + "context" + "fmt" + + "github.com/fullsend-ai/fullsend/internal/forge" + "github.com/fullsend-ai/fullsend/internal/ui" +) + +const dispatchTokenName = "FULLSEND_DISPATCH_TOKEN" + +// DispatchTokenLayer manages the org-level dispatch token that enrolled +// repos use to trigger workflow_dispatch events on the .fullsend repo. +// +// The dispatch token is a fine-grained PAT scoped to the .fullsend repo +// with actions:write permission. It is stored as an org-level Actions +// secret with visibility "selected", scoped to enrolled repos only. +// This way, enrolled repos can trigger dispatches but never access the +// App private keys (which are repo-level secrets on .fullsend). +type DispatchTokenLayer struct { + org string + client forge.Client + dispatchToken string // the PAT value to store (empty if reusing existing) + enrolledRepoIDs []int64 // repo IDs that should have access to this secret + ui *ui.Printer +} + +var _ Layer = (*DispatchTokenLayer)(nil) + +// NewDispatchTokenLayer creates a new DispatchTokenLayer. +func NewDispatchTokenLayer(org string, client forge.Client, token string, repoIDs []int64, printer *ui.Printer) *DispatchTokenLayer { + return &DispatchTokenLayer{ + org: org, + client: client, + dispatchToken: token, + enrolledRepoIDs: repoIDs, + ui: printer, + } +} + +// Name returns the layer name. +func (l *DispatchTokenLayer) Name() string { + return "dispatch-token" +} + +// RequiredScopes returns the scopes needed for the given operation. +func (l *DispatchTokenLayer) RequiredScopes(op Operation) []string { + switch op { + case OpInstall, OpUninstall, OpAnalyze: + return []string{"admin:org"} + default: + return nil + } +} + +// Install creates or updates the org-level dispatch token secret. +// If dispatchToken is empty, the secret value is reused (not recreated), +// but the repo access list is still updated if enrolledRepoIDs is set. +func (l *DispatchTokenLayer) Install(ctx context.Context) error { + if l.dispatchToken == "" { + l.ui.StepInfo("reusing existing dispatch token") + + if len(l.enrolledRepoIDs) > 0 { + l.ui.StepStart("updating dispatch token repo access list") + if err := l.client.SetOrgSecretRepos(ctx, l.org, dispatchTokenName, l.enrolledRepoIDs); err != nil { + l.ui.StepFail("failed to update dispatch token repo access") + return fmt.Errorf("updating org secret repo access: %w", err) + } + l.ui.StepDone("updated dispatch token repo access list") + } + return nil + } + + l.ui.StepStart("creating org secret " + dispatchTokenName) + if err := l.client.CreateOrgSecret(ctx, l.org, dispatchTokenName, l.dispatchToken, l.enrolledRepoIDs); err != nil { + l.ui.StepFail("failed to create org secret " + dispatchTokenName) + return fmt.Errorf("creating org secret %s: %w", dispatchTokenName, err) + } + l.ui.StepDone("created org secret " + dispatchTokenName) + return nil +} + +// Uninstall removes the org-level dispatch token secret if it exists. +func (l *DispatchTokenLayer) Uninstall(ctx context.Context) error { + exists, err := l.client.OrgSecretExists(ctx, l.org, dispatchTokenName) + if err != nil { + return fmt.Errorf("checking org secret %s: %w", dispatchTokenName, err) + } + + if !exists { + l.ui.StepInfo(dispatchTokenName + " already deleted") + return nil + } + + l.ui.StepStart("deleting org secret " + dispatchTokenName) + if err := l.client.DeleteOrgSecret(ctx, l.org, dispatchTokenName); err != nil { + l.ui.StepFail("failed to delete org secret " + dispatchTokenName) + return fmt.Errorf("deleting org secret %s: %w", dispatchTokenName, err) + } + l.ui.StepDone("deleted org secret " + dispatchTokenName) + return nil +} + +// Analyze checks whether the dispatch token org secret exists. +func (l *DispatchTokenLayer) Analyze(ctx context.Context) (*LayerReport, error) { + report := &LayerReport{Name: l.Name()} + + exists, err := l.client.OrgSecretExists(ctx, l.org, dispatchTokenName) + if err != nil { + return nil, fmt.Errorf("checking org secret %s: %w", dispatchTokenName, err) + } + + if exists { + report.Status = StatusInstalled + report.Details = append(report.Details, dispatchTokenName+" org secret exists") + } else { + report.Status = StatusNotInstalled + report.WouldInstall = append(report.WouldInstall, "create "+dispatchTokenName+" org secret") + } + + return report, nil +} diff --git a/internal/layers/dispatch_test.go b/internal/layers/dispatch_test.go new file mode 100644 index 0000000000..636bad68d6 --- /dev/null +++ b/internal/layers/dispatch_test.go @@ -0,0 +1,137 @@ +package layers + +import ( + "bytes" + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" + "github.com/fullsend-ai/fullsend/internal/ui" +) + +func newDispatchLayer(t *testing.T, client *forge.FakeClient, token string, repoIDs []int64) (*DispatchTokenLayer, *bytes.Buffer) { + t.Helper() + var buf bytes.Buffer + printer := ui.New(&buf) + layer := NewDispatchTokenLayer("test-org", client, token, repoIDs, printer) + return layer, &buf +} + +func TestDispatchTokenLayer_Name(t *testing.T) { + layer, _ := newDispatchLayer(t, &forge.FakeClient{}, "", nil) + assert.Equal(t, "dispatch-token", layer.Name()) +} + +func TestDispatchTokenLayer_Install_CreatesOrgSecret(t *testing.T) { + client := &forge.FakeClient{} + repoIDs := []int64{100, 200, 300} + layer, _ := newDispatchLayer(t, client, "ghp_secrettoken123", repoIDs) + + err := layer.Install(context.Background()) + require.NoError(t, err) + + require.Len(t, client.CreatedOrgSecrets, 1) + assert.Equal(t, "test-org", client.CreatedOrgSecrets[0].Org) + assert.Equal(t, "FULLSEND_DISPATCH_TOKEN", client.CreatedOrgSecrets[0].Name) + assert.Equal(t, "ghp_secrettoken123", client.CreatedOrgSecrets[0].Value) + assert.Equal(t, repoIDs, client.CreatedOrgSecrets[0].RepoIDs) +} + +func TestDispatchTokenLayer_Install_SkipsEmptyToken(t *testing.T) { + client := &forge.FakeClient{} + repoIDs := []int64{100, 200} + layer, _ := newDispatchLayer(t, client, "", repoIDs) + + err := layer.Install(context.Background()) + require.NoError(t, err) + + // No secret should be created when token is empty + assert.Empty(t, client.CreatedOrgSecrets) + + // But SetOrgSecretRepos should still be called to update access list + require.Contains(t, client.OrgSecretRepoIDs, "test-org/FULLSEND_DISPATCH_TOKEN") + assert.Equal(t, repoIDs, client.OrgSecretRepoIDs["test-org/FULLSEND_DISPATCH_TOKEN"]) +} + +func TestDispatchTokenLayer_Install_Error(t *testing.T) { + client := &forge.FakeClient{ + Errors: map[string]error{"CreateOrgSecret": errors.New("permission denied")}, + } + layer, _ := newDispatchLayer(t, client, "ghp_token", []int64{100}) + + err := layer.Install(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "permission denied") +} + +func TestDispatchTokenLayer_Uninstall_DeletesSecret(t *testing.T) { + client := &forge.FakeClient{ + OrgSecrets: map[string]bool{ + "test-org/FULLSEND_DISPATCH_TOKEN": true, + }, + } + layer, _ := newDispatchLayer(t, client, "", nil) + + err := layer.Uninstall(context.Background()) + require.NoError(t, err) + + require.Len(t, client.DeletedOrgSecrets, 1) + assert.Equal(t, "test-org/FULLSEND_DISPATCH_TOKEN", client.DeletedOrgSecrets[0]) +} + +func TestDispatchTokenLayer_Uninstall_AlreadyDeleted(t *testing.T) { + client := &forge.FakeClient{ + OrgSecrets: map[string]bool{}, // secret doesn't exist + } + layer, _ := newDispatchLayer(t, client, "", nil) + + err := layer.Uninstall(context.Background()) + require.NoError(t, err) + + // Should not attempt to delete + assert.Empty(t, client.DeletedOrgSecrets) +} + +func TestDispatchTokenLayer_Analyze_Installed(t *testing.T) { + client := &forge.FakeClient{ + OrgSecrets: map[string]bool{ + "test-org/FULLSEND_DISPATCH_TOKEN": true, + }, + } + layer, _ := newDispatchLayer(t, client, "", nil) + + report, err := layer.Analyze(context.Background()) + require.NoError(t, err) + + assert.Equal(t, "dispatch-token", report.Name) + assert.Equal(t, StatusInstalled, report.Status) + assert.Contains(t, report.Details, "FULLSEND_DISPATCH_TOKEN org secret exists") + assert.Empty(t, report.WouldInstall) +} + +func TestDispatchTokenLayer_Analyze_NotInstalled(t *testing.T) { + client := &forge.FakeClient{ + OrgSecrets: map[string]bool{}, + } + layer, _ := newDispatchLayer(t, client, "", nil) + + report, err := layer.Analyze(context.Background()) + require.NoError(t, err) + + assert.Equal(t, "dispatch-token", report.Name) + assert.Equal(t, StatusNotInstalled, report.Status) + assert.Empty(t, report.Details) + assert.Contains(t, report.WouldInstall, "create FULLSEND_DISPATCH_TOKEN org secret") +} + +func TestDispatchTokenLayer_RequiredScopes(t *testing.T) { + layer, _ := newDispatchLayer(t, &forge.FakeClient{}, "", nil) + + assert.Equal(t, []string{"admin:org"}, layer.RequiredScopes(OpInstall)) + assert.Equal(t, []string{"admin:org"}, layer.RequiredScopes(OpUninstall)) + assert.Equal(t, []string{"admin:org"}, layer.RequiredScopes(OpAnalyze)) +} diff --git a/internal/layers/enrollment.go b/internal/layers/enrollment.go index a30a7fdf65..f9fae1e54b 100644 --- a/internal/layers/enrollment.go +++ b/internal/layers/enrollment.go @@ -3,7 +3,6 @@ package layers import ( "context" "fmt" - "strings" "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/ui" @@ -43,6 +42,22 @@ func (l *EnrollmentLayer) Name() string { return "enrollment" } +// RequiredScopes returns the scopes needed for the given operation. +func (l *EnrollmentLayer) RequiredScopes(op Operation) []string { + switch op { + case OpInstall: + // Enrollment writes .github/workflows/fullsend.yaml to target repos + // and creates PRs. The workflow scope is needed for the workflow file. + return []string{"repo", "workflow"} + case OpUninstall: + return nil // no-op + case OpAnalyze: + return []string{"repo"} + default: + return nil + } +} + // Install creates enrollment PRs for enabled repos that are not yet enrolled. // Failures on individual repos are warned and skipped — install does not stop. func (l *EnrollmentLayer) Install(ctx context.Context) error { @@ -58,33 +73,49 @@ func (l *EnrollmentLayer) Install(ctx context.Context) error { return nil } -// enrollRepo creates an enrollment PR for a single repo. +// enrollRepo creates an enrollment PR for a single repo, or updates the +// shim workflow on an existing enrollment branch if a PR already exists. +// Idempotent: skips repos that already have the shim workflow merged on +// the default branch. func (l *EnrollmentLayer) enrollRepo(ctx context.Context, repo string) error { - // Check if already enrolled + // Check if already enrolled (shim workflow on default branch). _, err := l.client.GetFileContent(ctx, l.org, repo, shimWorkflowPath) if err == nil { l.ui.StepInfo(fmt.Sprintf("%s already enrolled", repo)) return nil } - if !forge.IsNotFound(err) { - return fmt.Errorf("checking enrollment status for %s: %w", repo, err) + + // Check if there's already an open enrollment PR from a previous run. + // If so, update the shim workflow on the branch to reflect the latest + // content (e.g., security model changes) rather than skipping. + prs, err := l.client.ListRepoPullRequests(ctx, l.org, repo) + if err == nil { + for _, pr := range prs { + if pr.Title == "Connect to fullsend agent pipeline" { + return l.updateExistingEnrollment(ctx, repo, pr) + } + } } l.ui.StepStart(fmt.Sprintf("Enrolling %s", repo)) - // Create branch for the enrollment PR + // Create branch for the enrollment PR. + // Idempotent: if the branch exists from a previous partial run, proceed. if err := l.client.CreateBranch(ctx, l.org, repo, enrollBranch); err != nil { - return fmt.Errorf("creating branch: %w", err) + if !forge.IsNotFound(err) { + l.ui.StepInfo(fmt.Sprintf("Branch %s may already exist, continuing", enrollBranch)) + } } - // Write shim workflow to the branch + // Write shim workflow to the branch using upsert to handle re-runs + // where the branch exists with an old version of the file. content := l.shimWorkflowContent() - if err := l.client.CreateFileOnBranch(ctx, l.org, repo, enrollBranch, shimWorkflowPath, + if err := l.client.CreateOrUpdateFileOnBranch(ctx, l.org, repo, enrollBranch, shimWorkflowPath, "chore: add fullsend shim workflow", []byte(content)); err != nil { return fmt.Errorf("writing shim workflow: %w", err) } - // Create enrollment PR + // Create enrollment PR. baseBranch := l.defaultBranches[repo] if baseBranch == "" { baseBranch = "main" @@ -108,6 +139,22 @@ func (l *EnrollmentLayer) enrollRepo(ctx context.Context, repo string) error { return nil } +// updateExistingEnrollment updates the shim workflow on an existing +// enrollment branch so the PR always reflects the latest content. +func (l *EnrollmentLayer) updateExistingEnrollment(ctx context.Context, repo string, pr forge.ChangeProposal) error { + l.ui.StepStart(fmt.Sprintf("Updating shim workflow on %s", repo)) + + content := l.shimWorkflowContent() + if err := l.client.CreateOrUpdateFileOnBranch(ctx, l.org, repo, enrollBranch, shimWorkflowPath, + "chore: update fullsend shim workflow", []byte(content)); err != nil { + return fmt.Errorf("updating shim workflow: %w", err) + } + + l.ui.StepDone(fmt.Sprintf("Updated enrollment PR for %s", repo)) + l.ui.PRLink(repo, pr.URL) + return nil +} + // Uninstall is a no-op. Individual repo cleanup is not automated — // repos keep their shim workflows. func (l *EnrollmentLayer) Uninstall(_ context.Context) error { @@ -123,10 +170,8 @@ func (l *EnrollmentLayer) Analyze(ctx context.Context) (*LayerReport, error) { _, err := l.client.GetFileContent(ctx, l.org, repo, shimWorkflowPath) if err == nil { enrolled = append(enrolled, repo) - } else if forge.IsNotFound(err) { - notEnrolled = append(notEnrolled, repo) } else { - return nil, fmt.Errorf("checking enrollment for %s: %w", repo, err) + notEnrolled = append(notEnrolled, repo) } } @@ -154,10 +199,16 @@ func (l *EnrollmentLayer) Analyze(ctx context.Context) (*LayerReport, error) { return report, nil } -// shimWorkflowContent returns the shim workflow YAML with the org name substituted. +// shimWorkflowContent returns the shim workflow YAML. +// Uses github.repository_owner so the content is org-agnostic. func (l *EnrollmentLayer) shimWorkflowContent() string { - tmpl := `# fullsend shim workflow -# Routes events to the reusable agent dispatch workflow in .fullsend. + return `# fullsend shim workflow +# Routes events to the agent dispatch workflow in .fullsend. +# +# Security: pull_request_target runs the BASE branch version of this workflow, +# preventing PRs from modifying it to exfiltrate the dispatch token. +# This shim never checks out PR code, so it is not vulnerable to "pwn request" +# attacks (see: Trivy CVE-2026-33634, hackerbot-claw campaign). name: fullsend on: @@ -165,19 +216,23 @@ on: types: [opened, edited, labeled] issue_comment: types: [created] - pull_request: + pull_request_target: types: [opened, synchronize, ready_for_review] pull_request_review: types: [submitted] jobs: dispatch: - uses: {org}/.fullsend/.github/workflows/agent.yaml@main - with: - event_type: ${{ github.event_name }} - event_payload: ${{ toJSON(github.event) }} - secrets: - APP_PRIVATE_KEY: ${{ secrets.FULLSEND_FULLSEND_APP_PRIVATE_KEY }} + runs-on: ubuntu-latest + steps: + - name: Dispatch to fullsend + env: + GH_TOKEN: ${{ secrets.FULLSEND_DISPATCH_TOKEN }} + run: | + gh workflow run agent.yaml \ + --repo "${{ github.repository_owner }}/.fullsend" \ + --field event_type="${{ github.event_name }}" \ + --field source_repo="${{ github.repository }}" \ + --field event_payload='${{ toJSON(github.event) }}' ` - return strings.ReplaceAll(tmpl, "{org}", l.org) } diff --git a/internal/layers/enrollment_test.go b/internal/layers/enrollment_test.go index c2a86f8206..6cb32a58ec 100644 --- a/internal/layers/enrollment_test.go +++ b/internal/layers/enrollment_test.go @@ -47,8 +47,9 @@ func TestEnrollmentLayer_Install_CreatesEnrollmentPRs(t *testing.T) { assert.Equal(t, "test-org", f.Owner) assert.Equal(t, shimWorkflowPath, f.Path) assert.Equal(t, enrollBranch, f.Branch) - // Verify shim workflow content contains the org name - assert.Contains(t, string(f.Content), "test-org/.fullsend/.github/workflows/agent.yaml@main") + // Verify shim workflow content uses dispatch token and repository_owner + assert.Contains(t, string(f.Content), "FULLSEND_DISPATCH_TOKEN") + assert.Contains(t, string(f.Content), "github.repository_owner") } // Should have created 2 PRs @@ -81,9 +82,38 @@ func TestEnrollmentLayer_Install_SkipsAlreadyEnrolled(t *testing.T) { require.Len(t, client.CreatedProposals, 1) } +func TestEnrollmentLayer_Install_UpdatesExistingPR(t *testing.T) { + client := &forge.FakeClient{ + PullRequests: map[string][]forge.ChangeProposal{ + "test-org/repo-a": { + {Title: "Connect to fullsend agent pipeline", URL: "https://github.com/test-org/repo-a/pull/1", Number: 1}, + }, + }, + } + repos := []string{"repo-a"} + defaults := map[string]string{"repo-a": "main"} + layer, _ := newEnrollmentLayer(t, client, repos, defaults) + + err := layer.Install(context.Background()) + require.NoError(t, err) + + // Should have updated the file on the branch, not created a new PR. + require.Len(t, client.CreatedFiles, 1) + assert.Equal(t, "repo-a", client.CreatedFiles[0].Repo) + assert.Equal(t, enrollBranch, client.CreatedFiles[0].Branch) + assert.Contains(t, string(client.CreatedFiles[0].Content), "FULLSEND_DISPATCH_TOKEN") + + // Should NOT have created a new branch or new PR. + assert.Empty(t, client.CreatedBranches) + // Should not have created any new PRs (the existing one was reused). + assert.Empty(t, client.CreatedProposals) +} + func TestEnrollmentLayer_Install_ContinuesOnError(t *testing.T) { - // Use a custom client that fails CreateBranch only for repo-a - client := &perRepoBranchErrorClient{ + // Use a custom client that fails CreateFileOnBranch only for repo-a. + // This simulates a real failure (e.g., permission denied) that should + // trigger a warning for repo-a but not stop repo-b from enrolling. + client := &perRepoFileErrorClient{ FakeClient: &forge.FakeClient{}, failRepo: "repo-a", } @@ -95,10 +125,8 @@ func TestEnrollmentLayer_Install_ContinuesOnError(t *testing.T) { // Install itself should not return an error — it warns and continues require.NoError(t, err) - // repo-b should still have been enrolled - require.Len(t, client.CreatedBranches, 1) - assert.Equal(t, "test-org/repo-b/fullsend/onboard", client.CreatedBranches[0]) - + // repo-b should still have been enrolled (branch + file + PR). + // repo-a should have a branch but no file or PR. require.Len(t, client.CreatedFiles, 1) assert.Equal(t, "repo-b", client.CreatedFiles[0].Repo) @@ -196,21 +224,25 @@ func TestEnrollmentLayer_Analyze_Partial(t *testing.T) { assert.Contains(t, report.WouldFix[0], "repo-b") } -// perRepoBranchErrorClient wraps FakeClient but fails CreateBranch for a specific repo. -type perRepoBranchErrorClient struct { +// perRepoFileErrorClient wraps FakeClient but fails CreateOrUpdateFileOnBranch for a specific repo. +type perRepoFileErrorClient struct { *forge.FakeClient failRepo string } -func (c *perRepoBranchErrorClient) CreateBranch(ctx context.Context, owner, repo, branchName string) error { +func (c *perRepoFileErrorClient) CreateOrUpdateFileOnBranch(_ context.Context, owner, repo, branch, path, message string, content []byte) error { if repo == c.failRepo { - return fmt.Errorf("branch creation failed for %s", repo) + return fmt.Errorf("file write failed for %s", repo) } - return c.FakeClient.CreateBranch(ctx, owner, repo, branchName) + return c.FakeClient.CreateOrUpdateFileOnBranch(context.Background(), owner, repo, branch, path, message, content) } -// GetFileContent delegates to the embedded FakeClient. The failRepo has no -// shim workflow, so the default "file not found" error triggers enrollment. -func (c *perRepoBranchErrorClient) GetFileContent(ctx context.Context, owner, repo, path string) ([]byte, error) { +// GetFileContent delegates to the embedded FakeClient. +func (c *perRepoFileErrorClient) GetFileContent(ctx context.Context, owner, repo, path string) ([]byte, error) { return c.FakeClient.GetFileContent(ctx, owner, repo, path) } + +// ListRepoPullRequests delegates to the embedded FakeClient. +func (c *perRepoFileErrorClient) ListRepoPullRequests(ctx context.Context, owner, repo string) ([]forge.ChangeProposal, error) { + return c.FakeClient.ListRepoPullRequests(ctx, owner, repo) +} diff --git a/internal/layers/layers.go b/internal/layers/layers.go index 69e30bcc14..4019812b7c 100644 --- a/internal/layers/layers.go +++ b/internal/layers/layers.go @@ -40,12 +40,40 @@ type LayerReport struct { WouldFix []string // what install would fix (for degraded state) } +// Operation identifies which action is being performed on a layer. +type Operation int + +const ( + OpInstall Operation = iota + OpUninstall + OpAnalyze +) + +func (o Operation) String() string { + switch o { + case OpInstall: + return "install" + case OpUninstall: + return "uninstall" + case OpAnalyze: + return "analyze" + default: + return fmt.Sprintf("Operation(%d)", int(o)) + } +} + // Layer is the interface each installation concern implements. // Layers are processed in order for install, reverse order for uninstall. type Layer interface { // Name returns a human-readable name for this layer. Name() string + // RequiredScopes returns the OAuth scopes this layer needs for the + // given operation. Scopes are GitHub-flavored strings like "repo", + // "delete_repo", "workflow". Used by Preflight to fail early when + // the token is missing required scopes. + RequiredScopes(op Operation) []string + // Install creates or configures this layer's concern. Install(ctx context.Context) error @@ -98,6 +126,22 @@ func (s *Stack) UninstallAll(ctx context.Context) []error { return errs } +// CollectRequiredScopes returns the deduplicated set of scopes needed +// by all layers for the given operation. +func (s *Stack) CollectRequiredScopes(op Operation) []string { + seen := make(map[string]bool) + var scopes []string + for _, l := range s.layers { + for _, scope := range l.RequiredScopes(op) { + if !seen[scope] { + seen[scope] = true + scopes = append(scopes, scope) + } + } + } + return scopes +} + // AnalyzeAll runs Analyze on each layer and returns reports. func (s *Stack) AnalyzeAll(ctx context.Context) ([]*LayerReport, error) { var reports []*LayerReport diff --git a/internal/layers/layers_test.go b/internal/layers/layers_test.go index 1eb632fe59..f54372a396 100644 --- a/internal/layers/layers_test.go +++ b/internal/layers/layers_test.go @@ -17,6 +17,7 @@ type mockLayer struct { uninstallErr error analyzeErr error report *LayerReport + scopes map[Operation][]string installCalled bool uninstallCalled bool @@ -26,6 +27,13 @@ type mockLayer struct { func (m *mockLayer) Name() string { return m.name } +func (m *mockLayer) RequiredScopes(op Operation) []string { + if m.scopes != nil { + return m.scopes[op] + } + return nil +} + func (m *mockLayer) Install(_ context.Context) error { m.installCalled = true if m.callOrder != nil { diff --git a/internal/layers/preflight.go b/internal/layers/preflight.go new file mode 100644 index 0000000000..a4dcafd4f9 --- /dev/null +++ b/internal/layers/preflight.go @@ -0,0 +1,80 @@ +package layers + +import ( + "context" + "fmt" + "strings" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +// PreflightResult describes what a preflight check found. +type PreflightResult struct { + // Required is the set of scopes the operation needs. + Required []string + // Granted is the set of scopes the token actually has. + Granted []string + // Missing is the set of scopes needed but not granted. + Missing []string +} + +// OK returns true if no scopes are missing. +func (r *PreflightResult) OK() bool { + return len(r.Missing) == 0 +} + +// Error returns a human-readable error describing missing scopes and +// how to fix the problem. +func (r *PreflightResult) Error() string { + var b strings.Builder + fmt.Fprintf(&b, "token is missing required scopes: %s\n", strings.Join(r.Missing, ", ")) + b.WriteString("\nTo add the missing scopes, run:\n") + fmt.Fprintf(&b, " gh auth refresh -s %s\n", strings.Join(r.Missing, ",")) + b.WriteString("\nOr set GH_TOKEN / GITHUB_TOKEN with a token that includes these scopes.") + return b.String() +} + +// Preflight checks that the forge client's token has all the scopes +// required by the stack's layers for the given operation. It returns a +// PreflightResult describing what was found. +// +// If the forge doesn't support scope introspection (e.g., fine-grained +// tokens, GitHub App tokens), Preflight returns a result with OK() == true +// and logs that scope checking was skipped. We can't validate what we +// can't see, so we let the operation proceed and fail at the point of +// use if scopes are actually missing. +func (s *Stack) Preflight(ctx context.Context, op Operation, client forge.Client) (*PreflightResult, error) { + required := s.CollectRequiredScopes(op) + if len(required) == 0 { + return &PreflightResult{}, nil + } + + granted, err := client.GetTokenScopes(ctx) + if err != nil { + return nil, fmt.Errorf("checking token scopes: %w", err) + } + + // If the forge can't report scopes (fine-grained tokens return nil), + // we can't validate. Let the operation proceed. + if granted == nil { + return &PreflightResult{Required: required}, nil + } + + grantedSet := make(map[string]bool, len(granted)) + for _, s := range granted { + grantedSet[s] = true + } + + var missing []string + for _, scope := range required { + if !grantedSet[scope] { + missing = append(missing, scope) + } + } + + return &PreflightResult{ + Required: required, + Granted: granted, + Missing: missing, + }, nil +} diff --git a/internal/layers/preflight_test.go b/internal/layers/preflight_test.go new file mode 100644 index 0000000000..e7e1bc7bd1 --- /dev/null +++ b/internal/layers/preflight_test.go @@ -0,0 +1,121 @@ +package layers + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +func TestPreflight_AllScopesPresent(t *testing.T) { + client := &forge.FakeClient{ + TokenScopes: []string{"repo", "delete_repo", "workflow"}, + } + stack := NewStack( + &mockLayer{name: "a", scopes: map[Operation][]string{OpInstall: {"repo", "workflow"}}}, + &mockLayer{name: "b", scopes: map[Operation][]string{OpInstall: {"repo"}}}, + ) + + result, err := stack.Preflight(context.Background(), OpInstall, client) + require.NoError(t, err) + assert.True(t, result.OK()) + assert.Empty(t, result.Missing) +} + +func TestPreflight_MissingScopes(t *testing.T) { + client := &forge.FakeClient{ + TokenScopes: []string{"repo"}, + } + stack := NewStack( + &mockLayer{name: "a", scopes: map[Operation][]string{OpUninstall: {"repo", "delete_repo"}}}, + ) + + result, err := stack.Preflight(context.Background(), OpUninstall, client) + require.NoError(t, err) + assert.False(t, result.OK()) + assert.Equal(t, []string{"delete_repo"}, result.Missing) + assert.Contains(t, result.Error(), "delete_repo") + assert.Contains(t, result.Error(), "gh auth refresh") +} + +func TestPreflight_NoScopesRequired(t *testing.T) { + client := &forge.FakeClient{} + stack := NewStack( + &mockLayer{name: "a"}, // no scopes for any operation + ) + + result, err := stack.Preflight(context.Background(), OpAnalyze, client) + require.NoError(t, err) + assert.True(t, result.OK()) +} + +func TestPreflight_NilScopes_FineGrainedToken(t *testing.T) { + // Fine-grained tokens return nil for GetTokenScopes. + // Preflight should let the operation proceed (we can't validate). + client := &forge.FakeClient{ + TokenScopes: nil, + } + stack := NewStack( + &mockLayer{name: "a", scopes: map[Operation][]string{OpInstall: {"repo", "workflow"}}}, + ) + + result, err := stack.Preflight(context.Background(), OpInstall, client) + require.NoError(t, err) + assert.True(t, result.OK(), "should pass when scopes can't be introspected") +} + +func TestPreflight_GetTokenScopesError(t *testing.T) { + client := &forge.FakeClient{ + Errors: map[string]error{"GetTokenScopes": errors.New("network error")}, + } + stack := NewStack( + &mockLayer{name: "a", scopes: map[Operation][]string{OpInstall: {"repo"}}}, + ) + + _, err := stack.Preflight(context.Background(), OpInstall, client) + require.Error(t, err) + assert.Contains(t, err.Error(), "network error") +} + +func TestPreflight_DeduplicatesScopes(t *testing.T) { + client := &forge.FakeClient{ + TokenScopes: []string{"repo"}, + } + // Both layers require "repo" — should only appear once in required. + stack := NewStack( + &mockLayer{name: "a", scopes: map[Operation][]string{OpInstall: {"repo"}}}, + &mockLayer{name: "b", scopes: map[Operation][]string{OpInstall: {"repo"}}}, + ) + + result, err := stack.Preflight(context.Background(), OpInstall, client) + require.NoError(t, err) + assert.True(t, result.OK()) + assert.Equal(t, []string{"repo"}, result.Required) +} + +func TestCollectRequiredScopes(t *testing.T) { + stack := NewStack( + &mockLayer{name: "a", scopes: map[Operation][]string{OpInstall: {"repo", "workflow"}}}, + &mockLayer{name: "b", scopes: map[Operation][]string{OpInstall: {"repo", "delete_repo"}}}, + ) + + scopes := stack.CollectRequiredScopes(OpInstall) + assert.ElementsMatch(t, []string{"repo", "workflow", "delete_repo"}, scopes) +} + +func TestPreflightResult_Error(t *testing.T) { + r := &PreflightResult{ + Required: []string{"repo", "delete_repo", "workflow"}, + Granted: []string{"repo"}, + Missing: []string{"delete_repo", "workflow"}, + } + + msg := r.Error() + assert.Contains(t, msg, "delete_repo") + assert.Contains(t, msg, "workflow") + assert.Contains(t, msg, "gh auth refresh -s delete_repo,workflow") +} diff --git a/internal/layers/secrets.go b/internal/layers/secrets.go index 86bbbef7ef..fc2ae25bbb 100644 --- a/internal/layers/secrets.go +++ b/internal/layers/secrets.go @@ -42,6 +42,20 @@ func (s *SecretsLayer) Name() string { return "secrets" } +// RequiredScopes returns the scopes needed for the given operation. +func (s *SecretsLayer) RequiredScopes(op Operation) []string { + switch op { + case OpInstall: + return []string{"repo"} + case OpUninstall: + return nil // no-op + case OpAnalyze: + return []string{"repo"} + default: + return nil + } +} + // Install stores agent app private keys as repo secrets and app IDs as // repo variables in the .fullsend config repo. func (s *SecretsLayer) Install(ctx context.Context) error { diff --git a/internal/layers/secrets_test.go b/internal/layers/secrets_test.go index e1f1584cb7..d4c87e4ade 100644 --- a/internal/layers/secrets_test.go +++ b/internal/layers/secrets_test.go @@ -22,24 +22,16 @@ func newSecretsLayer(t *testing.T, client *forge.FakeClient, agents []AgentCrede return layer, &buf } -// fakePEM returns a PEM-like string for testing. The header is constructed -// at runtime to avoid triggering the detect-private-key pre-commit hook. -func fakePEM(body string) string { - header := "-----BEGIN RSA PRIVATE" + " KEY-----" - footer := "-----END RSA PRIVATE" + " KEY-----" - return header + "\n" + body + "\n" + footer -} - func twoAgents() []AgentCredentials { return []AgentCredentials{ { AgentEntry: config.AgentEntry{Role: "fullsend", Name: "FullsendBot", Slug: "fullsend-bot"}, - PEM: fakePEM("fullsend-key"), + PEM: "-----BEGIN RSA PRIVATE KEY-----\nfullsend-key\n-----END RSA PRIVATE KEY-----", AppID: 111, }, { AgentEntry: config.AgentEntry{Role: "triage", Name: "TriageBot", Slug: "triage-bot"}, - PEM: fakePEM("triage-key"), + PEM: "-----BEGIN RSA PRIVATE KEY-----\ntriage-key\n-----END RSA PRIVATE KEY-----", AppID: 222, }, } @@ -90,7 +82,7 @@ func TestSecretsLayer_Install_SkipsEmptyPEM(t *testing.T) { agents := []AgentCredentials{ { AgentEntry: config.AgentEntry{Role: "fullsend", Name: "FullsendBot", Slug: "fullsend-bot"}, - PEM: fakePEM("fullsend-key"), + PEM: "-----BEGIN RSA PRIVATE KEY-----\nfullsend-key\n-----END RSA PRIVATE KEY-----", AppID: 111, }, { diff --git a/internal/layers/workflows.go b/internal/layers/workflows.go index d96f388b56..e5ccb019b3 100644 --- a/internal/layers/workflows.go +++ b/internal/layers/workflows.go @@ -47,8 +47,30 @@ func (l *WorkflowsLayer) Name() string { return "workflows" } +// RequiredScopes returns the scopes needed for the given operation. +func (l *WorkflowsLayer) RequiredScopes(op Operation) []string { + switch op { + case OpInstall: + // Writing to .github/workflows/ paths requires the workflow scope. + // Without it, GitHub returns 404 (not 403), which is deeply confusing. + return []string{"repo", "workflow"} + case OpUninstall: + return nil // no-op + case OpAnalyze: + return []string{"repo"} + default: + return nil + } +} + // Install writes the workflow files and CODEOWNERS to the .fullsend repo. // CODEOWNERS failure is treated as a warning, not a fatal error. +// +// Note: writing multiple files sequentially via the Contents API can cause +// transient 404s because each file write creates a new commit and the branch +// ref is updated asynchronously. The GitHub client's retry logic handles +// this. CODEOWNERS is written last and its failure is non-fatal because +// some orgs restrict CODEOWNERS writes to specific teams. func (l *WorkflowsLayer) Install(ctx context.Context) error { files := map[string][]byte{ agentWorkflowPath: []byte(agentWorkflowContent), @@ -126,22 +148,23 @@ func (l *WorkflowsLayer) codeownersContent() string { return fmt.Sprintf("# fullsend configuration is governed by org admins.\n* @%s\n", l.authenticatedUser) } -const agentWorkflowContent = `# Reusable agent dispatch workflow -# Called by per-repo shim workflows to run fullsend agents. +const agentWorkflowContent = `# Agent dispatch workflow +# Triggered by shim workflows in enrolled repos via workflow_dispatch. +# Reads its own repo secrets (App PEMs) — secrets never leave this repo. name: Agent Dispatch on: - workflow_call: + workflow_dispatch: inputs: event_type: required: true type: string - event_payload: + source_repo: required: true type: string - secrets: - APP_PRIVATE_KEY: + event_payload: required: true + type: string jobs: dispatch: @@ -149,9 +172,10 @@ jobs: steps: - uses: actions/checkout@v4 - name: Run fullsend entrypoint - run: echo "fullsend entrypoint - event=${{ inputs.event_type }}" + run: echo "fullsend entrypoint - event=${{ inputs.event_type }} repo=${{ inputs.source_repo }}" env: EVENT_TYPE: ${{ inputs.event_type }} + SOURCE_REPO: ${{ inputs.source_repo }} EVENT_PAYLOAD: ${{ inputs.event_payload }} ` diff --git a/internal/layers/workflows_test.go b/internal/layers/workflows_test.go index e3893ff569..6f14cf255d 100644 --- a/internal/layers/workflows_test.go +++ b/internal/layers/workflows_test.go @@ -67,7 +67,7 @@ func TestWorkflowsLayer_Install_AgentWorkflowContent(t *testing.T) { } } require.NotEmpty(t, agentContent, "agent.yaml should have been written") - assert.Contains(t, agentContent, "workflow_call") + assert.Contains(t, agentContent, "workflow_dispatch") } func TestWorkflowsLayer_Install_OnboardWorkflowContent(t *testing.T) { @@ -257,3 +257,24 @@ func (c *codeownersErrorClient) GetRepo(context.Context, string, string) (*forge func (c *codeownersErrorClient) RepoVariableExists(context.Context, string, string, string) (bool, error) { return false, nil } +func (c *codeownersErrorClient) GetTokenScopes(context.Context) ([]string, error) { + return nil, nil +} +func (c *codeownersErrorClient) CreateOrgSecret(context.Context, string, string, string, []int64) error { + return nil +} +func (c *codeownersErrorClient) OrgSecretExists(context.Context, string, string) (bool, error) { + return false, nil +} +func (c *codeownersErrorClient) DeleteOrgSecret(context.Context, string, string) error { + return nil +} +func (c *codeownersErrorClient) SetOrgSecretRepos(context.Context, string, string, []int64) error { + return nil +} +func (c *codeownersErrorClient) CreateOrUpdateFileOnBranch(context.Context, string, string, string, string, string, []byte) error { + return nil +} +func (c *codeownersErrorClient) DispatchWorkflow(context.Context, string, string, string, string, map[string]string) error { + return nil +}