diff --git a/.github/workflows/reusable-code.yml b/.github/workflows/reusable-code.yml index fe494854b..b24d2923e 100644 --- a/.github/workflows/reusable-code.yml +++ b/.github/workflows/reusable-code.yml @@ -178,4 +178,4 @@ jobs: run-url: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} status-repo: ${{ inputs.source_repo }} status-number: ${{ fromJSON(inputs.event_payload).issue.number }} - status-token: ${{ steps.app-token.outputs.token }} + mint-url: ${{ inputs.mint_url }} diff --git a/.github/workflows/reusable-fix.yml b/.github/workflows/reusable-fix.yml index 5968c784e..21e171b3d 100644 --- a/.github/workflows/reusable-fix.yml +++ b/.github/workflows/reusable-fix.yml @@ -380,4 +380,4 @@ jobs: run-url: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} status-repo: ${{ inputs.source_repo }} status-number: ${{ steps.context.outputs.pr_number }} - status-token: ${{ steps.app-token.outputs.token }} + mint-url: ${{ inputs.mint_url }} diff --git a/.github/workflows/reusable-retro.yml b/.github/workflows/reusable-retro.yml index 8ddeb3589..fdccfa520 100644 --- a/.github/workflows/reusable-retro.yml +++ b/.github/workflows/reusable-retro.yml @@ -153,4 +153,4 @@ jobs: run-url: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} status-repo: ${{ inputs.source_repo }} status-number: ${{ fromJSON(inputs.event_payload).pull_request.number || fromJSON(inputs.event_payload).issue.number }} - status-token: ${{ steps.app-token.outputs.token }} + mint-url: ${{ inputs.mint_url }} diff --git a/.github/workflows/reusable-review.yml b/.github/workflows/reusable-review.yml index 863681129..e3c77f09f 100644 --- a/.github/workflows/reusable-review.yml +++ b/.github/workflows/reusable-review.yml @@ -169,4 +169,4 @@ jobs: run-url: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} status-repo: ${{ inputs.source_repo }} status-number: ${{ fromJSON(inputs.event_payload).pull_request.number || fromJSON(inputs.event_payload).issue.number }} - status-token: ${{ steps.app-token.outputs.token }} + mint-url: ${{ inputs.mint_url }} diff --git a/.github/workflows/reusable-triage.yml b/.github/workflows/reusable-triage.yml index ac9dd6aa0..a13d0a85a 100644 --- a/.github/workflows/reusable-triage.yml +++ b/.github/workflows/reusable-triage.yml @@ -149,4 +149,4 @@ jobs: run-url: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} status-repo: ${{ inputs.source_repo }} status-number: ${{ fromJSON(inputs.event_payload).issue.number }} - status-token: ${{ steps.app-token.outputs.token }} + mint-url: ${{ inputs.mint_url }} diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 32b39573f..000000000 --- a/CLAUDE.md +++ /dev/null @@ -1,3 +0,0 @@ -# CLAUDE.md - -Project rules and instructions live in [AGENTS.md](AGENTS.md). Read that file now — it is the single source of truth for all agent-facing guidance in this repo. diff --git a/action.yml b/action.yml index a57044a0f..1fea40b04 100644 --- a/action.yml +++ b/action.yml @@ -36,8 +36,16 @@ inputs: status-number: description: Issue/PR number for status comments (optional). default: "" + mint-url: + description: >- + Mint service URL for on-demand status comment tokens. When set, the + binary mints a fresh short-lived token before each status API call + instead of using a static status-token. + default: "" status-token: - description: Token for status comments (defaults to GH_TOKEN env var). + description: >- + DEPRECATED — use mint-url instead. Static GitHub token for status + comments. Ignored when mint-url is set. default: "" runs: @@ -363,9 +371,13 @@ runs: STATUS_RUN_URL: ${{ inputs.run-url }} STATUS_REPO: ${{ inputs.status-repo }} STATUS_NUMBER: ${{ inputs.status-number }} + MINT_URL: ${{ inputs.mint-url }} STATUS_TOKEN: ${{ inputs.status-token }} run: | set -euo pipefail + if [[ -n "${STATUS_TOKEN}" ]]; then + echo "::add-mask::${STATUS_TOKEN}" + fi FULLSEND_DIR="${FULLSEND_DIR:-${GITHUB_WORKSPACE}}" TARGET_REPO="${TARGET_REPO:-${GITHUB_WORKSPACE}/target-repo}" mkdir -p "${GITHUB_WORKSPACE}/output" @@ -373,16 +385,17 @@ runs: # Post-scripts enforce secret scanning, protected-path blocks, # and review-downgrade controls. Skipping them in CI bypasses # all post-push security gates. - if [[ -n "${STATUS_TOKEN}" ]]; then - echo "::add-mask::${STATUS_TOKEN}" - fi STATUS_FLAGS=() if [[ -n "${STATUS_REPO}" && -n "${STATUS_NUMBER}" ]]; then STATUS_FLAGS+=(--status-repo "${STATUS_REPO}" --status-number "${STATUS_NUMBER}") if [[ -n "${STATUS_RUN_URL}" ]]; then STATUS_FLAGS+=(--run-url "${STATUS_RUN_URL}") fi + if [[ -n "${MINT_URL}" ]]; then + STATUS_FLAGS+=(--mint-url "${MINT_URL}") + fi if [[ -n "${STATUS_TOKEN}" ]]; then + echo "::warning::status-token is deprecated; use mint-url instead" STATUS_FLAGS+=(--status-token "${STATUS_TOKEN}") fi fi @@ -393,10 +406,12 @@ runs: "${STATUS_FLAGS[@]+"${STATUS_FLAGS[@]}"}" - name: Finalize orphaned status comment - if: always() && inputs.agent != '__install_only__' && inputs.status-repo != '' && inputs.status-number != '' + if: always() && inputs.agent != '__install_only__' && inputs.status-repo != '' && inputs.status-number != '' && (inputs.mint-url != '' || inputs.status-token != '') shell: bash env: + MINT_URL: ${{ inputs.mint-url }} STATUS_TOKEN: ${{ inputs.status-token }} + AGENT: ${{ inputs.agent }} STATUS_REPO: ${{ inputs.status-repo }} STATUS_NUMBER: ${{ inputs.status-number }} RUN_ID: ${{ github.run_id }} @@ -405,17 +420,19 @@ runs: JOB_STATUS: ${{ job.status }} run: | set -euo pipefail + if [[ -n "${STATUS_TOKEN}" ]]; then + echo "::add-mask::${STATUS_TOKEN}" + fi # When the fullsend process is hard-killed (SIGKILL, OOM, segfault), # the deferred PostCompletion call never runs and the status comment # remains in "Started" state. This step runs unconditionally (if: # always()) to detect and finalize orphaned comments. See #2149. - TOKEN="${STATUS_TOKEN:-${GITHUB_TOKEN:-}}" - if [[ -z "${TOKEN}" ]]; then - echo "::warning::No token available for status comment reconciliation" - exit 0 + RECONCILE_FLAGS=(--repo "${STATUS_REPO}" --number "${STATUS_NUMBER}" --run-id "${RUN_ID}") + if [[ -n "${MINT_URL}" ]]; then + RECONCILE_FLAGS+=(--mint-url "${MINT_URL}" --role "${AGENT}") + elif [[ -n "${STATUS_TOKEN}" ]]; then + RECONCILE_FLAGS+=(--token "${STATUS_TOKEN}") fi - echo "::add-mask::${TOKEN}" - RECONCILE_FLAGS=(--repo "${STATUS_REPO}" --number "${STATUS_NUMBER}" --run-id "${RUN_ID}" --token "${TOKEN}") if [[ -n "${RUN_URL}" ]]; then RECONCILE_FLAGS+=(--run-url "${RUN_URL}") fi diff --git a/docs/agents/triage.md b/docs/agents/triage.md index aa526068a..a14dbb3ce 100644 --- a/docs/agents/triage.md +++ b/docs/agents/triage.md @@ -40,7 +40,7 @@ outcome and the post-script applies the corresponding label. | `ready-to-code` | The issue is fully specified and low-risk (bug, documentation, performance). Triggers the [code agent](code.md). | | `triaged` | The issue is fully specified but is a feature or other category that requires human prioritization before coding. | | `duplicate` | The issue duplicates an existing one. The agent identified the original and the post-script closes the issue. | -| `blocked` | The issue depends on another issue or external condition. The agent identified the blocker. | +| `blocked` | The issue depends on prerequisites — existing issues/PRs or newly created upstream issues. The agent identified or created the blockers. | | `question` | The issue is a support request or question, not an actionable bug or feature. The agent attempted to answer it. | The `issue-labels` skill may also apply contextual labels (e.g., `area/api`, @@ -48,6 +48,37 @@ The `issue-labels` skill may also apply contextual labels (e.g., `area/api`, ## Configuration and extension +### Cross-repo issue creation + +The triage agent can create prerequisite issues in other repositories when it +identifies upstream dependencies that don't have tracking issues yet. This is +controlled by the `create_issues` section in `config.yaml`: + +```yaml +create_issues: + allow_targets: + orgs: + - my-org + repos: + - upstream-org/specific-repo +``` + +**Defaults:** At install time, fullsend populates this with your org (in org mode) +or your repo (in per-repo mode), plus `fullsend-ai/fullsend` as an upstream target. + +**When to expand the allowlist:** If your project depends on libraries or services +in other GitHub orgs and you want the triage agent to automatically file +prerequisite issues there, add those orgs or repos to `allow_targets`. + +**When to restrict the allowlist:** If you don't want agents creating issues +outside your org, remove entries. If `allow_targets` is empty, automatic +prerequisite creation is disabled entirely — the agent will still identify +the dependency and include a draft issue body in its comment for a human to +file manually. + +The source repo (where triage is running) is always implicitly allowed +regardless of the allowlist. + ### Skill: `issue-labels` The triage agent includes a built-in `issue-labels` skill that discovers your diff --git a/docs/architecture.md b/docs/architecture.md index 7a0bfa0f2..c6f229b0b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -236,7 +236,7 @@ ADR 0002: [Building block 3](ADRs/0002-initial-fullsend-design.md#3-label-state- ### 4. triage agent runtime -Runs triage from issue `title`/`body` + GitHub-native attachments only; each run starts with **`duplicate`** and other reset labels cleared; duplicate detection, blocking dependency detection (cross-repo), readiness, reproducibility, test handoff; can close as duplicate again if still a match, or label **`blocked`** when progress depends on another open issue or PR. +Runs triage from issue `title`/`body` + GitHub-native attachments only; each run starts with **`duplicate`** and other reset labels cleared; duplicate detection, prerequisite detection (cross-repo), readiness, reproducibility, test handoff; can close as duplicate again if still a match, label **`blocked`** when progress depends on another open issue or PR, or create upstream prerequisite issues when no tracking issue exists (controlled by `create_issues.allow_targets` config). ADR 0002: [Building block 4](ADRs/0002-initial-fullsend-design.md#4-triage-agent-runtime). ### 5. Duplicate / similarity search diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index c4b51914c..97af2fd96 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -58,7 +58,7 @@ fullsend │ ├── --run-url # CI/CD run URL for status comments │ ├── --status-repo # Repository for status comments │ ├── --status-number # Issue/PR number for status comments -│ └── --status-token # Token for status comments (default: GH_TOKEN) +│ └── --mint-url # Mint service URL for on-demand status tokens ├── fetch-skill # Fetch a skill at runtime (in-sandbox) ├── scan # Run security scanner on input/output │ ├── input # Scan event payload for prompt injection @@ -74,7 +74,8 @@ fullsend ├── --run-url # Workflow run URL (optional) ├── --sha # Commit SHA (optional) ├── --reason # Termination reason: terminated or cancelled (default: terminated) - └── --token # GitHub token (default: $GITHUB_TOKEN) + ├── --mint-url # Mint service URL for on-demand token (default: $FULLSEND_MINT_URL) + └── --role # Agent role for minting (required with --mint-url) ``` ### Command Decomposition diff --git a/docs/guides/user/bugfix-workflow.md b/docs/guides/user/bugfix-workflow.md index b5ec7594e..6124121f0 100644 --- a/docs/guides/user/bugfix-workflow.md +++ b/docs/guides/user/bugfix-workflow.md @@ -102,7 +102,7 @@ Every push to a PR in the review stage triggers a new review round. This means ` The triage agent: 1. **Checks for duplicates.** Searches existing issues by title, body, and metadata. If it finds a match with high confidence, it labels `duplicate`, posts a comment linking the canonical issue, and closes this one. -2. **Checks for blocking dependencies.** Searches for open issues or PRs (in this repo or upstream) that must be resolved before work can start. If a blocker is found, it labels `blocked` and posts a comment linking to the blocking issue or PR. On re-triage, it checks whether existing blockers have been resolved. +2. **Checks for blocking dependencies.** Searches for open issues or PRs (in this repo or upstream) that must be resolved before work can start. If a prerequisite is found, it labels `blocked` and posts a comment linking to it. When no upstream tracking issue exists, the triage agent can also create one in the upstream repo (controlled by `create_issues.allow_targets` in config). On re-triage, it checks whether existing prerequisites have been resolved. 3. **Checks information sufficiency.** If the issue body is missing steps to reproduce, expected behavior, or other critical details, it labels `needs-info` and posts a comment explaining what's missing. 4. **Produces a test artifact.** When possible, writes a failing test case aligned with the repo's test framework. 5. **Hands off.** Labels `ready-to-code` with a summary comment. diff --git a/docs/guides/user/running-agents-locally.md b/docs/guides/user/running-agents-locally.md index 969f47689..33a83dbc6 100644 --- a/docs/guides/user/running-agents-locally.md +++ b/docs/guides/user/running-agents-locally.md @@ -235,7 +235,7 @@ target issue/PR. These flags mirror what the CI workflows pass automatically: | `--run-url` | URL of the CI/CD run shown in the status comment | | `--status-repo` | Repository (`owner/repo`) to post status comments on | | `--status-number` | Issue or PR number for status comments | -| `--status-token` | Token for posting comments (defaults to `GH_TOKEN`) | +| `--mint-url` | Mint service URL for on-demand status comment tokens (default: `$FULLSEND_MINT_URL`) | Example: diff --git a/docs/reference/installation.md b/docs/reference/installation.md index a1364a4f9..ea92333b5 100644 --- a/docs/reference/installation.md +++ b/docs/reference/installation.md @@ -732,7 +732,8 @@ The composite action accepts four optional inputs for status notifications: | `run-url` | URL of the CI/CD run shown in the status comment | | `status-repo` | Repository (`owner/repo`) to post status comments on | | `status-number` | Issue or PR number for status comments | -| `status-token` | Token for posting comments (defaults to `GH_TOKEN`) | +| `mint-url` | URL of the token mint service used to obtain fresh tokens for posting comments | +| `status-token` | **Deprecated.** Static token for posting comments; use `mint-url` instead | All reusable workflows pass these inputs automatically. diff --git a/docs/superpowers/plans/2026-06-11-triage-prerequisites.md b/docs/superpowers/plans/2026-06-11-triage-prerequisites.md new file mode 100644 index 000000000..777c65fd2 --- /dev/null +++ b/docs/superpowers/plans/2026-06-11-triage-prerequisites.md @@ -0,0 +1,865 @@ +# Triage Prerequisites Action 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:** Replace the triage agent's `blocked` action with a `prerequisites` action that can both reference existing blockers and create new upstream issues. + +**Architecture:** Add `CreateIssuesConfig` to the config structs, update the triage result JSON schema, modify the agent prompt, and extend the post-script to create issues and handle the allowlist. The post-script reads `config.yaml` from `$GITHUB_WORKSPACE` (the config repo checkout) via `yq`. + +**Tech Stack:** Go (config structs + tests), JSON Schema, bash (post-script), markdown (agent prompt + docs) + +--- + +### Task 1: Add `CreateIssuesConfig` to config structs + +**Files:** +- Modify: `internal/config/config.go` +- Test: `internal/config/config_test.go` + +- [ ] **Step 1: Write failing tests for the new config types** + +Add to `internal/config/config_test.go`: + +```go +func TestOrgConfig_CreateIssues_ParseYAML(t *testing.T) { + yamlData := ` +version: "1" +dispatch: + platform: github-actions +defaults: + roles: + - fullsend + max_implementation_retries: 2 +agents: [] +repos: {} +create_issues: + allow_targets: + orgs: + - my-org + - upstream-org + repos: + - other-org/specific-repo +` + cfg, err := ParseOrgConfig([]byte(yamlData)) + require.NoError(t, err) + require.NotNil(t, cfg.CreateIssues) + assert.Equal(t, []string{"my-org", "upstream-org"}, cfg.CreateIssues.AllowTargets.Orgs) + assert.Equal(t, []string{"other-org/specific-repo"}, cfg.CreateIssues.AllowTargets.Repos) +} + +func TestOrgConfig_CreateIssues_OmittedWhenEmpty(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + Agents: []AgentEntry{}, + Repos: map[string]RepoConfig{}, + } + data, err := cfg.Marshal() + require.NoError(t, err) + assert.NotContains(t, string(data), "create_issues") +} + +func TestOrgConfig_CreateIssues_Marshal(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + Agents: []AgentEntry{}, + Repos: map[string]RepoConfig{}, + CreateIssues: &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Orgs: []string{"my-org"}, + Repos: []string{"fullsend-ai/fullsend"}, + }, + }, + } + data, err := cfg.Marshal() + require.NoError(t, err) + assert.Contains(t, string(data), "create_issues:") + assert.Contains(t, string(data), "my-org") + assert.Contains(t, string(data), "fullsend-ai/fullsend") +} + +func TestOrgConfigValidate_CreateIssues_InvalidRepoFormat(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + CreateIssues: &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Repos: []string{"no-slash"}, + }, + }, + } + err := cfg.Validate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "create_issues") +} + +func TestOrgConfigValidate_CreateIssues_EmptyOrg(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + CreateIssues: &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Orgs: []string{""}, + }, + }, + } + err := cfg.Validate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "create_issues") +} + +func TestOrgConfigValidate_CreateIssues_Valid(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + CreateIssues: &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Orgs: []string{"my-org"}, + Repos: []string{"other/repo"}, + }, + }, + } + assert.NoError(t, cfg.Validate()) +} + +func TestOrgConfigValidate_CreateIssues_Nil(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + } + assert.NoError(t, cfg.Validate()) +} + +func TestNewOrgConfig_CreateIssuesDefaults(t *testing.T) { + cfg := NewOrgConfig([]string{"repo-a"}, []string{"repo-a"}, []string{"fullsend"}, nil, "", "my-org") + require.NotNil(t, cfg.CreateIssues) + assert.Contains(t, cfg.CreateIssues.AllowTargets.Orgs, "my-org") + assert.Contains(t, cfg.CreateIssues.AllowTargets.Repos, "fullsend-ai/fullsend") +} + +func TestPerRepoConfig_CreateIssues_ParseYAML(t *testing.T) { + yamlData := ` +version: "1" +roles: + - triage +create_issues: + allow_targets: + repos: + - owner/target-repo + - fullsend-ai/fullsend +` + cfg, err := ParsePerRepoConfig([]byte(yamlData)) + require.NoError(t, err) + require.NotNil(t, cfg.CreateIssues) + assert.Equal(t, []string{"owner/target-repo", "fullsend-ai/fullsend"}, cfg.CreateIssues.AllowTargets.Repos) +} + +func TestNewPerRepoConfig_CreateIssuesDefaults(t *testing.T) { + cfg := NewPerRepoConfig(nil, "owner/my-repo") + require.NotNil(t, cfg.CreateIssues) + assert.Contains(t, cfg.CreateIssues.AllowTargets.Repos, "owner/my-repo") + assert.Contains(t, cfg.CreateIssues.AllowTargets.Repos, "fullsend-ai/fullsend") +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd internal/config && go test -v -run 'CreateIssues' ./...` +Expected: compilation errors — types `CreateIssuesConfig`, `AllowTargets` not defined, `NewOrgConfig`/`NewPerRepoConfig` wrong arg count. + +- [ ] **Step 3: Add the new types and update struct fields** + +In `internal/config/config.go`, add the new types: + +```go +// AllowTargets defines which orgs and repos agents may create issues in. +type AllowTargets struct { + Orgs []string `yaml:"orgs,omitempty"` + Repos []string `yaml:"repos,omitempty"` +} + +// CreateIssuesConfig controls cross-repo issue creation by agents. +type CreateIssuesConfig struct { + AllowTargets AllowTargets `yaml:"allow_targets"` +} +``` + +Add `CreateIssues` field to `OrgConfig`: + +```go +CreateIssues *CreateIssuesConfig `yaml:"create_issues,omitempty"` +``` + +Add `CreateIssues` field to `PerRepoConfig`: + +```go +CreateIssues *CreateIssuesConfig `yaml:"create_issues,omitempty"` +``` + +- [ ] **Step 4: Update `NewOrgConfig` to accept org name and set defaults** + +Change `NewOrgConfig` signature to add `org string` parameter: + +```go +func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry, inferenceProvider, org string) *OrgConfig { +``` + +Inside the function, after the existing config construction, add: + +```go +if org != "" { + cfg.CreateIssues = &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Orgs: []string{org}, + Repos: []string{"fullsend-ai/fullsend"}, + }, + } +} +``` + +- [ ] **Step 5: Update `NewPerRepoConfig` to accept target repo and set defaults** + +Change `NewPerRepoConfig` signature: + +```go +func NewPerRepoConfig(roles []string, targetRepo string) *PerRepoConfig { +``` + +Inside the function, after the existing config construction, add: + +```go +if targetRepo != "" { + cfg.CreateIssues = &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Repos: []string{targetRepo, "fullsend-ai/fullsend"}, + }, + } +} +``` + +- [ ] **Step 6: Add validation for CreateIssues in `OrgConfig.Validate()`** + +Before the `return nil` at the end of `Validate()`: + +```go +if err := validateCreateIssues(c.CreateIssues); err != nil { + return err +} +``` + +Add the helper: + +```go +func validateCreateIssues(cfg *CreateIssuesConfig) error { + if cfg == nil { + return nil + } + for _, org := range cfg.AllowTargets.Orgs { + if org == "" { + return fmt.Errorf("create_issues.allow_targets.orgs contains empty string") + } + } + for _, repo := range cfg.AllowTargets.Repos { + if repo == "" || !strings.Contains(repo, "/") { + return fmt.Errorf("create_issues.allow_targets.repos entry %q must be owner/name format", repo) + } + } + return nil +} +``` + +Add the same `validateCreateIssues` call to `PerRepoConfig.Validate()`. + +- [ ] **Step 7: Run tests to verify they pass** + +Run: `cd internal/config && go test -v ./...` +Expected: all tests pass including new `CreateIssues` tests. + +- [ ] **Step 8: Commit** + +```bash +git add internal/config/config.go internal/config/config_test.go +git commit -S -s -m "feat(config): add create_issues allowlist config (#401) + +Add CreateIssuesConfig and AllowTargets types to both OrgConfig and +PerRepoConfig. NewOrgConfig populates defaults with the org and +fullsend-ai/fullsend. NewPerRepoConfig populates with the target repo +and fullsend-ai/fullsend. + +Assisted-by: Claude Opus 4.6 " +``` + +### Task 2: Fix callers of `NewOrgConfig` and `NewPerRepoConfig` + +**Files:** +- Modify: `internal/cli/admin.go` +- Modify: `internal/cli/github.go` +- Modify: `internal/cli/admin_test.go` +- Modify: `internal/cli/github_test.go` +- Modify: `internal/layers/configrepo_test.go` + +Task 1 changed the signatures of `NewOrgConfig` (added `org string`) and `NewPerRepoConfig` (added `targetRepo string`). All callers must be updated. + +- [ ] **Step 1: Find all call sites and update them** + +Update each `NewOrgConfig(...)` call to pass the `org` variable as the final argument. The `org` variable is already in scope at every call site in `admin.go` and `github.go`. + +In `internal/cli/github.go:464`: +```go +orgCfg := config.NewOrgConfig(repoNames, enabledRepos, roles, dummyAgents, inferenceProviderName, org) +``` + +In `internal/cli/github.go:513`: +```go +orgCfg = config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName, org) +``` + +In `internal/cli/admin.go:1174`: +```go +cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, nil, inferenceProviderName, org) +``` + +In `internal/cli/admin.go:1502`: +```go +cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName, org) +``` + +In `internal/cli/admin.go:1640`: +```go +emptyCfg := config.NewOrgConfig(nil, nil, nil, nil, "", "") +``` + +In `internal/cli/admin.go:1781`: +```go +cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, nil, "", org) +``` + +Update each `NewPerRepoConfig(...)` call to pass `cfg.target` (the `owner/repo` string): + +In `internal/cli/github.go:210`: +```go +perRepoCfg := config.NewPerRepoConfig(roles, cfg.target) +``` + +In `internal/cli/admin.go:647`: +```go +cfg := config.NewPerRepoConfig(roles, target) +``` +(Check the variable name — it may be `cfg.target` or `target` depending on the function scope.) + +Update test call sites — these typically pass `""` for the new parameters since tests don't care about create_issues defaults: + +In `internal/cli/admin_test.go:583`: +```go +return config.NewOrgConfig(repoNames, enabledRepos, []string{"triage"}, nil, "", "") +``` + +In `internal/cli/admin_test.go:1082`, `1123`: +```go +config.NewOrgConfig(..., "") +``` + +In `internal/cli/github_test.go:395`: +```go +cfg := config.NewOrgConfig([]string{"widget"}, []string{"widget"}, []string{"triage"}, nil, "", "") +``` + +In `internal/config/config_test.go`, update existing tests that call `NewOrgConfig` without the org param: + +`TestNewOrgConfig`: add `""` as last arg. +`TestNewOrgConfig_WithInferenceProvider`: change to `NewOrgConfig(nil, nil, nil, nil, "vertex", "")`. +`TestNewOrgConfig_WithoutInferenceProvider`: change to `NewOrgConfig(nil, nil, nil, nil, "", "")`. +`TestNewOrgConfig_KillSwitchDefaultFalse`: change to `NewOrgConfig(nil, nil, []string{"fullsend"}, nil, "", "")`. + +In `internal/config/config_test.go`, update existing tests for `NewPerRepoConfig`: + +`TestNewPerRepoConfig_DefaultRoles`: change to `NewPerRepoConfig(nil, "")`. +`TestNewPerRepoConfig_CustomRoles`: change to `NewPerRepoConfig([]string{"triage", "review"}, "")`. +`TestPerRepoConfig_RoundTrip`: change to `NewPerRepoConfig([]string{...}, "")`. + +In `internal/layers/configrepo_test.go`, update any `NewOrgConfig` / `NewPerRepoConfig` calls similarly. + +- [ ] **Step 2: Run full test suite to verify** + +Run: `make go-test` +Expected: all tests pass. + +- [ ] **Step 3: Commit** + +```bash +git add internal/cli/admin.go internal/cli/github.go internal/cli/admin_test.go internal/cli/github_test.go internal/config/config_test.go internal/layers/configrepo_test.go +git commit -S -s -m "refactor: update NewOrgConfig/NewPerRepoConfig callers for create_issues (#401) + +Pass org name and target repo to config constructors so create_issues +defaults are populated at install time. + +Assisted-by: Claude Opus 4.6 " +``` + +### Task 3: Update triage result JSON schema + +**Files:** +- Modify: `internal/scaffold/fullsend-repo/schemas/triage-result.schema.json` +- Test: `internal/scaffold/fullsend-repo/scripts/validate-output-schema-test.sh` (if it exists) + +- [ ] **Step 1: Replace `blocked` with `prerequisites` in action enum** + +In `triage-result.schema.json`, change line 12: + +```json +"enum": ["insufficient", "duplicate", "sufficient", "prerequisites", "question"] +``` + +- [ ] **Step 2: Remove the `blocked_by` property** + +Delete lines 33-37 (the `blocked_by` property). + +- [ ] **Step 3: Add the `prerequisites` property definition** + +Add to the `properties` object: + +```json +"prerequisites": { + "type": "object", + "required": ["existing", "create"], + "properties": { + "existing": { + "type": "array", + "items": { + "type": "object", + "required": ["url"], + "properties": { + "url": { + "type": "string", + "pattern": "^https://github\\.com/[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+/(issues|pull)/[0-9]+$" + } + }, + "additionalProperties": false + } + }, + "create": { + "type": "array", + "items": { + "type": "object", + "required": ["repo", "title", "body"], + "properties": { + "repo": { + "type": "string", + "pattern": "^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$" + }, + "title": { + "type": "string", + "minLength": 1 + }, + "body": { + "type": "string", + "minLength": 1 + } + }, + "additionalProperties": false + } + } + }, + "additionalProperties": false +} +``` + +- [ ] **Step 4: Update the conditional validation** + +Replace the `blocked` conditional (the `allOf` entry at lines 55-58): + +```json +{ + "if": { "properties": { "action": { "const": "prerequisites" } }, "required": ["action"] }, + "then": { + "required": ["prerequisites"], + "properties": { + "prerequisites": { + "anyOf": [ + { "properties": { "existing": { "minItems": 1 } } }, + { "properties": { "create": { "minItems": 1 } } } + ] + } + } + } +} +``` + +- [ ] **Step 5: Validate the schema is valid JSON** + +Run: `jq empty internal/scaffold/fullsend-repo/schemas/triage-result.schema.json` +Expected: no output (valid JSON). + +- [ ] **Step 6: Test with sample inputs** + +Create a temp file `/tmp/test-prereq.json`: + +```json +{ + "action": "prerequisites", + "reasoning": "Blocked by upstream work", + "comment": "This needs upstream changes first.", + "prerequisites": { + "existing": [{"url": "https://github.com/org/repo/issues/42"}], + "create": [{"repo": "org/upstream", "title": "Add X", "body": "Need X for downstream."}] + } +} +``` + +Run the schema validator if available: +```bash +fullsend-check-output /tmp/test-prereq.json 2>&1 || echo "Manual validation needed" +``` + +Also test that a `prerequisites` result with both arrays empty is rejected, and that the old `blocked` action is rejected. + +- [ ] **Step 7: Commit** + +```bash +git add internal/scaffold/fullsend-repo/schemas/triage-result.schema.json +git commit -S -s -m "feat(schema): replace blocked with prerequisites action (#401) + +Replace the blocked action and blocked_by field with a prerequisites +action containing existing[] and create[] arrays. At least one array +must be non-empty. + +Assisted-by: Claude Opus 4.6 " +``` + +### Task 4: Update the triage agent prompt + +**Files:** +- Modify: `internal/scaffold/fullsend-repo/agents/triage.md` + +- [ ] **Step 1: Replace the `blocked` action section** + +Replace the "Action: `blocked`" section (lines 182-195) with: + +```markdown +### Action: `prerequisites` + +Progress on this issue depends on work that must happen first — either in this repository or another. Use this action when you identify specific blocking dependencies: existing issues/PRs that must be resolved, or upstream work that needs a tracking issue created. + +**HARD CONSTRAINT:** Never emit `sufficient` if unresolved prerequisites exist. Use `prerequisites` instead. + +The `prerequisites` object contains two arrays: + +- `existing` — issues or PRs that already exist and block this work. Include the full HTML URL. +- `create` — issues that need to be filed in other repos before this work can proceed. Include the target `repo` (owner/name format), a `title`, and a `body`. Write the body for the target repo's audience — include enough technical context for upstream maintainers to understand what is needed. Use your judgment on whether to include a back-reference to the originating issue; sometimes it provides helpful context, sometimes it leaks internal details. + +At least one of the two arrays must have entries. + +```json +{ + "action": "prerequisites", + "reasoning": "Brief explanation of the dependencies and why this issue cannot proceed", + "prerequisites": { + "existing": [ + { "url": "https://github.com/org/repo/issues/99" } + ], + "create": [ + { + "repo": "org/upstream-lib", + "title": "Add support for X", + "body": "Technical description of what is needed and why, written for the upstream repo's maintainers." + } + ] + }, + "comment": "A professional comment explaining the blocking dependencies. Link to existing blockers and describe what new issues need to be created upstream. Be specific about why each dependency must be resolved before this issue can proceed." +} +``` +``` + +- [ ] **Step 2: Update the anti-premature-resolution rule** + +In the "Anti-premature-resolution rule" paragraph (line 125), add after the existing hard constraint: + +```markdown +**Anti-premature-prerequisites rule (HARD CONSTRAINT):** If your assessment identifies unresolved prerequisites — dependencies on work in other repos or unmerged changes that must land first — you MUST use `action: "prerequisites"`. Do NOT emit `action: "sufficient"` when prerequisites exist. The `sufficient` action means there are zero blockers and zero open questions. +``` + +- [ ] **Step 3: Update Step 3 Phase 3 to reference prerequisites** + +In Phase 3 (line 108), update the last bullet: + +```markdown +- **Is progress blocked on other work?** Consider whether the fix depends on an unresolved issue or unmerged PR — in this repo or another. If a developer cannot meaningfully start work until some other issue is resolved, this issue has prerequisites regardless of how clear the problem description is. If the blocking work has no tracking issue yet, you can recommend creating one via the `prerequisites` action's `create` array. +``` + +- [ ] **Step 4: Update Step 2c to reference prerequisites instead of blocked** + +In section 2c (line 66-77), update the heading and text to say "Check existing prerequisites" instead of "Check existing blockers", and reference the `prerequisites` action instead of `blocked`. + +- [ ] **Step 5: Commit** + +```bash +git add internal/scaffold/fullsend-repo/agents/triage.md +git commit -S -s -m "feat(triage): replace blocked action with prerequisites in agent prompt (#401) + +The triage agent can now recommend creating upstream issues via the +prerequisites action's create array, in addition to referencing existing +blockers. Adds hard constraint against emitting sufficient when +prerequisites exist. + +Assisted-by: Claude Opus 4.6 " +``` + +### Task 5: Update the post-script to handle `prerequisites` + +**Files:** +- Modify: `internal/scaffold/fullsend-repo/scripts/post-triage.sh` + +- [ ] **Step 1: Replace the `blocked)` case with `prerequisites)`** + +Replace the entire `blocked)` case (lines 122-141) with: + +```bash + prerequisites) + if [[ -z "${COMMENT}" ]]; then + echo "ERROR: action is 'prerequisites' but no comment provided" + exit 1 + fi + + # Read the allowlist from config.yaml. The config repo is checked out + # at $GITHUB_WORKSPACE by the reusable workflow. + CONFIG_FILE="${GITHUB_WORKSPACE}/config.yaml" + if [[ ! -f "${CONFIG_FILE}" ]]; then + # Per-repo mode: config is under .fullsend/ + CONFIG_FILE="${GITHUB_WORKSPACE}/.fullsend/config.yaml" + fi + + ALLOWED_ORGS="" + ALLOWED_REPOS="" + if [[ -f "${CONFIG_FILE}" ]] && command -v yq &>/dev/null; then + ALLOWED_ORGS=$(yq -r '.create_issues.allow_targets.orgs // [] | .[]' "${CONFIG_FILE}" 2>/dev/null || true) + ALLOWED_REPOS=$(yq -r '.create_issues.allow_targets.repos // [] | .[]' "${CONFIG_FILE}" 2>/dev/null || true) + fi + + # The source repo is always implicitly allowed. + SOURCE_ORG="${REPO%%/*}" + + is_target_allowed() { + local target_repo="$1" + local target_org="${target_repo%%/*}" + + # Source repo is always allowed. + if [[ "${target_repo}" == "${REPO}" ]]; then + return 0 + fi + + # Check org allowlist. + if [[ -n "${ALLOWED_ORGS}" ]] && echo "${ALLOWED_ORGS}" | grep -qFx "${target_org}"; then + return 0 + fi + + # Check repo allowlist. + if [[ -n "${ALLOWED_REPOS}" ]] && echo "${ALLOWED_REPOS}" | grep -qFx "${target_repo}"; then + return 0 + fi + + return 1 + } + + # Process create entries: create issues, collect URLs. + CREATE_COUNT=$(jq '.prerequisites.create // [] | length' "${RESULT_FILE}") + CREATED_URLS="" + FAILED_CREATES="" + + for i in $(seq 0 $((CREATE_COUNT - 1))); do + TARGET_REPO=$(jq -r ".prerequisites.create[${i}].repo" "${RESULT_FILE}") + ISSUE_TITLE=$(jq -r ".prerequisites.create[${i}].title" "${RESULT_FILE}") + ISSUE_BODY=$(jq -r ".prerequisites.create[${i}].body" "${RESULT_FILE}") + + if ! is_target_allowed "${TARGET_REPO}"; then + echo "::warning::Skipping issue creation in '${TARGET_REPO}' — not in create_issues.allow_targets" + FAILED_CREATES="${FAILED_CREATES} +
+Prerequisite: ${TARGET_REPO} — ${ISSUE_TITLE} + +${ISSUE_BODY} + +
" + continue + fi + + echo "Creating prerequisite issue in ${TARGET_REPO}..." + CREATED_URL=$(gh issue create --repo "${TARGET_REPO}" --title "${ISSUE_TITLE}" --body "${ISSUE_BODY}" 2>&1) || { + echo "::warning::Failed to create issue in '${TARGET_REPO}': ${CREATED_URL}" + FAILED_CREATES="${FAILED_CREATES} +
+Prerequisite: ${TARGET_REPO} — ${ISSUE_TITLE} + +${ISSUE_BODY} + +
" + continue + } + echo "Created: ${CREATED_URL}" + CREATED_URLS="${CREATED_URLS} ${CREATED_URL}" + done + + # Collect existing URLs. + EXISTING_COUNT=$(jq '.prerequisites.existing // [] | length' "${RESULT_FILE}") + EXISTING_URLS="" + for i in $(seq 0 $((EXISTING_COUNT - 1))); do + URL=$(jq -r ".prerequisites.existing[${i}].url" "${RESULT_FILE}") + EXISTING_URLS="${EXISTING_URLS} ${URL}" + done + + # Merge all blocker URLs for the comment. + ALL_URLS="${EXISTING_URLS} ${CREATED_URLS}" + ALL_URLS=$(echo "${ALL_URLS}" | xargs) # trim whitespace + + if [[ -n "${ALL_URLS}" ]]; then + BLOCKER_LIST="" + for url in ${ALL_URLS}; do + BLOCKER_LIST="${BLOCKER_LIST} +- ${url}" + done + COMMENT="${COMMENT} + +**Blocked by:**${BLOCKER_LIST}" + fi + + if [[ -n "${FAILED_CREATES}" ]]; then + COMMENT="${COMMENT} + +**Could not create automatically** (file manually or update \`create_issues.allow_targets\` in config.yaml): +${FAILED_CREATES}" + fi + + remove_label "ready-to-code" + remove_label "needs-info" + add_label "blocked" + ;; +``` + +- [ ] **Step 2: Verify the script is syntactically valid** + +Run: `bash -n internal/scaffold/fullsend-repo/scripts/post-triage.sh` +Expected: no output (valid syntax). + +- [ ] **Step 3: Commit** + +```bash +git add internal/scaffold/fullsend-repo/scripts/post-triage.sh +git commit -S -s -m "feat(triage): handle prerequisites action in post-script (#401) + +Replace the blocked handler with prerequisites. The post-script reads +the create_issues allowlist from config.yaml, creates permitted upstream +issues via gh, and includes collapsed draft bodies for disallowed or +failed creates so humans can file them manually. + +Assisted-by: Claude Opus 4.6 " +``` + +### Task 6: Update user-facing triage docs + +**Files:** +- Modify: `docs/agents/triage.md` + +- [ ] **Step 1: Update control labels table** + +Replace the `blocked` row: + +```markdown +| `blocked` | The issue depends on prerequisites — existing issues/PRs or newly created upstream issues. The agent identified or created the blockers. | +``` + +- [ ] **Step 2: Add new section on `create_issues` configuration** + +After the "Configuration and extension" heading, add: + +```markdown +### Cross-repo issue creation + +The triage agent can create prerequisite issues in other repositories when it +identifies upstream dependencies that don't have tracking issues yet. This is +controlled by the `create_issues` section in `config.yaml`: + +```yaml +create_issues: + allow_targets: + orgs: + - my-org + repos: + - upstream-org/specific-repo +``` + +**Defaults:** At install time, fullsend populates this with your org (in org mode) +or your repo (in per-repo mode), plus `fullsend-ai/fullsend` as an upstream target. + +**When to expand the allowlist:** If your project depends on libraries or services +in other GitHub orgs and you want the triage agent to automatically file +prerequisite issues there, add those orgs or repos to `allow_targets`. + +**When to restrict the allowlist:** If you don't want agents creating issues +outside your org, remove entries. If `allow_targets` is empty, automatic +prerequisite creation is disabled entirely — the agent will still identify +the dependency and include a draft issue body in its comment for a human to +file manually. + +The source repo (where triage is running) is always implicitly allowed +regardless of the allowlist. +``` + +- [ ] **Step 3: Commit** + +```bash +git add docs/agents/triage.md +git commit -S -s -m "docs: document prerequisites action and create_issues config (#401) + +Update triage agent docs to explain the new prerequisites action and the +create_issues.allow_targets configuration surface. + +Assisted-by: Claude Opus 4.6 " +``` + +### Task 7: Run linters and full test suite + +**Files:** +- All modified files from Tasks 1-6 + +- [ ] **Step 1: Run linter** + +Run: `make lint` +Expected: no failures. + +- [ ] **Step 2: Run Go tests** + +Run: `make go-test` +Expected: all tests pass. + +- [ ] **Step 3: Run vet** + +Run: `make go-vet` +Expected: no issues. + +- [ ] **Step 4: Fix any issues found and commit fixes** + +If lint or tests reveal issues, fix them and commit. diff --git a/docs/superpowers/specs/2026-06-11-triage-prerequisites-design.md b/docs/superpowers/specs/2026-06-11-triage-prerequisites-design.md new file mode 100644 index 000000000..899deebf5 --- /dev/null +++ b/docs/superpowers/specs/2026-06-11-triage-prerequisites-design.md @@ -0,0 +1,147 @@ +# Triage Agent Prerequisites Action + +**Date:** 2026-06-11 +**Issue:** [#401](https://github.com/fullsend-ai/fullsend/issues/401) +**Status:** Draft + +## Problem + +The triage agent can detect that an issue is blocked by existing work elsewhere, but it cannot create the missing tracking issue when no such issue exists yet. A common scenario: triage evaluates a bug in a Tekton task and determines the root cause is a missing feature in an upstream container image defined in a different repo. Today the agent can only say "blocked" and point to an existing issue. If no upstream issue exists, the agent has no way to express "this needs to be filed first." + +This forces humans to manually identify, draft, and file prerequisite issues in other repos before the original issue can make progress. + +## Scope + +This design covers **one** of three decomposition strategies identified during brainstorming: + +| Strategy | Description | This design? | +|---|---|---| +| **Spin out dependency** | Original stays open + `blocked`. Agent creates upstream prerequisite issues. | Yes | +| **Split muddled issue** | Original closed. N independent successor issues replace it. | No (future work) | +| **Parent/child decompose** | Original stays open as parent. N child issues for incremental delivery. | No (future work) | + +## Key discovery: cross-repo issue creation works today + +A GitHub App installation token scoped to one repository can create issues in any public repo on GitHub, including repos in orgs where the app is not installed. GitHub confirmed this as a known behavior (not a vulnerability). This means the triage agent's existing token already supports cross-repo issue creation without any changes to the mint or auth infrastructure. See #402 for the original assumption that cross-installation auth would be needed. + +## Design + +### New `prerequisites` action + +The existing `blocked` action is replaced by `prerequisites`. The triage agent's action set becomes five actions: `sufficient`, `insufficient`, `duplicate`, `question`, `prerequisites`. + +The `prerequisites` action unifies two cases: +- **Existing blockers** the agent found during its search (today's `blocked` behavior) +- **New blockers** that need to be filed as issues before progress can happen + +The triage result schema: + +```json +{ + "action": "prerequisites", + "prerequisites": { + "existing": [ + { "url": "https://github.com/org/repo/issues/42" } + ], + "create": [ + { + "repo": "org/upstream-lib", + "title": "Add support for X", + "body": "Technical description for the upstream audience..." + } + ] + }, + "comment": "This issue requires upstream changes before it can proceed.", + "label_actions": [] +} +``` + +Constraints: +- At least one of `existing` or `create` must be non-empty. +- Both arrays can be populated in the same result (mixed existing + new blockers). +- The `blocked_by` field (singular URL, current schema) is removed. + +### Hard constraint in agent prompt + +> Never emit `sufficient` if unresolved prerequisites exist. Use `prerequisites` instead. + +This mirrors the existing constraint: "Never emit `sufficient` with open questions." + +### Agent prompt guidance for `create` entries + +The agent uses its judgment on issue body content. Sometimes a back-reference to the originating issue is helpful for upstream maintainers; sometimes it leaks internal context. The agent writes the body for the upstream repo's audience, not the source repo's. + +### Allowlist configuration + +A new `create_issues` config field controls which repos and orgs agents are permitted to create issues in. This applies to both triage and retro agents. + +```yaml +create_issues: + allow_targets: + orgs: + - "my-org" + - "upstream-org" + repos: + - "other-org/specific-repo" +``` + +Validation rules: +- If `allow_targets` is absent or empty, prerequisite creation is disabled (safe default). +- A target repo is permitted if its org appears in `orgs` OR the exact `owner/repo` appears in `repos`. +- The source repo (where triage is running) is always implicitly allowed. +- Entries in `repos` must be `owner/name` format. Empty strings are rejected. + +### Install-time defaults + +The admin setup flow populates `create_issues.allow_targets` with sensible defaults: + +- **Org mode:** `allow_targets.orgs` includes the org. `allow_targets.repos` includes `fullsend-ai/fullsend`. +- **Per-repo mode:** `allow_targets.repos` includes the target repo and `fullsend-ai/fullsend`. + +### Post-script behavior + +When the post-script receives `action: "prerequisites"`: + +1. **Process `create` entries:** For each entry, validate `repo` against `create_issues.allow_targets`. If allowed, create the issue using existing `forge.Client.CreateIssue` plumbing. Collect the resulting URL. If disallowed or the API call fails, record the failure. + +2. **Merge URLs:** Combine URLs from successfully created issues with the `existing` array to produce the full blocker list. + +3. **Apply labels:** Remove `ready-to-code` and `needs-info`. Add `blocked` label. (Same as current `blocked` action behavior.) + +4. **Post comment:** Sticky comment (via `fullsend post-comment`) summarizing the prerequisites. Links to all blockers (existing and newly created). For entries that could not be filed (allowlist rejection or API failure), include the agent's draft in a collapsed section so a human can file it manually: + + ```html +
+ Prerequisite: org_a/repo -- Add support for X + + [the full body the agent drafted for the upstream issue] + +
+ ``` + +5. **Partial success:** If some creates succeed and others fail, the issue still gets `blocked` with whatever blockers were established. The comment notes which prerequisites could not be created and why. + +The existing `blocked` action handler in the post-script is removed. `prerequisites` fully replaces it. + +### Re-triage flow + +When a prerequisite issue is resolved and the original issue is re-triaged, the agent discovers blocker URLs from the sticky comment posted by the post-script (which contains links to all prerequisite issues). The existing blocker-checking logic in the agent prompt (Step 2) already inspects linked issues and checks their state. If all prerequisites are resolved, the agent can emit `sufficient` or another appropriate action. No changes needed to the re-triage flow. + +## Changes required + +| Component | File | Change | +|---|---|---| +| Config structs | `internal/config/config.go` | Add `CreateIssues` struct with `AllowTargets` (Orgs `[]string`, Repos `[]string`) to both `OrgConfig` and `PerRepoConfig`. Update constructors with install-time defaults. Add validation. | +| Triage result schema | `internal/scaffold/fullsend-repo/schemas/triage-result.schema.json` | Replace `blocked` with `prerequisites` in action enum. Add `prerequisites` object schema. Remove `blocked_by`. | +| Agent prompt | `internal/scaffold/fullsend-repo/agents/triage.md` | Replace `blocked` action with `prerequisites`. Add hard constraint. Add guidance for `create` entry content. | +| Post-script | `internal/scaffold/fullsend-repo/scripts/post-triage.sh` | Replace `blocked` handler with `prerequisites` handler. Add allowlist validation, issue creation, degraded path with collapsed draft. | +| Pre-script | `internal/scaffold/fullsend-repo/scripts/pre-triage.sh` | No change. `blocked` label stripping stays the same. | +| User docs | `docs/agents/triage.md` | New section documenting `create_issues` config surface: what it does, defaults, when to expand or restrict. | +| Config constructors | `internal/config/config.go` | `NewOrgConfig` and `NewPerRepoConfig` populate `create_issues.allow_targets` defaults. Callers in `internal/cli/admin.go` and `internal/cli/github.go` pass the org/repo context. | + +## Out of scope + +- **Split muddled issues** (close original, create N independent successors) +- **Parent/child decomposition** (original stays open, create N children) +- **Cross-repo issue editing** (GitHub enforces scope on edits, only creation bypasses it) +- **Retro agent integration** (uses the same `create_issues` config, but prompt/post-script changes are separate work) diff --git a/internal/cli/admin.go b/internal/cli/admin.go index fcc9af3fc..c9c99cc9e 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -644,7 +644,7 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error { printer.StepWarn("Using provided WIF provider value — skipping inference provider auto-provisioning") } - cfg := config.NewPerRepoConfig(roles) + cfg := config.NewPerRepoConfig(roles, repoFullName) if err := cfg.Validate(); err != nil { return fmt.Errorf("invalid config: %w", err) } @@ -1188,7 +1188,7 @@ func runDryRun(ctx context.Context, client forge.Client, printer *ui.Printer, or } // Build config with empty agents for analysis. - cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, nil, inferenceProviderName) + cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, nil, inferenceProviderName, org) cfg.Dispatch.Mode = "oidc-mint" user, err := client.GetAuthenticatedUser(ctx) @@ -1524,7 +1524,7 @@ func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, o agents[i] = ac.AgentEntry } - cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName) + cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName, org) cfg.Dispatch.Mode = "oidc-mint" user, err := client.GetAuthenticatedUser(ctx) @@ -1663,7 +1663,8 @@ func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer, } // Build a minimal stack for uninstall. - emptyCfg := config.NewOrgConfig(nil, nil, nil, nil, "") + // Only ConfigRepoLayer matters for uninstall since other layers are no-ops. + emptyCfg := config.NewOrgConfig(nil, nil, nil, nil, "", "") stack := layers.NewStack( layers.NewConfigRepoLayer(org, client, emptyCfg, printer, false), layers.NewWorkflowsLayer(org, client, printer, "", version), @@ -1804,7 +1805,7 @@ func runAnalyze(ctx context.Context, client forge.Client, printer *ui.Printer, o }) } - cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, nil, "") + cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, nil, "", org) user, err := client.GetAuthenticatedUser(ctx) if err != nil { diff --git a/internal/cli/admin_test.go b/internal/cli/admin_test.go index 3363b574f..14deaa012 100644 --- a/internal/cli/admin_test.go +++ b/internal/cli/admin_test.go @@ -580,7 +580,7 @@ func setupTestConfig(repos map[string]bool) *config.OrgConfig { // Sort to ensure deterministic order despite map iteration being non-deterministic. sort.Strings(repoNames) sort.Strings(enabledRepos) - return config.NewOrgConfig(repoNames, enabledRepos, []string{"triage"}, nil, "") + return config.NewOrgConfig(repoNames, enabledRepos, []string{"triage"}, nil, "", "") } func setupTestClient(org string, cfg *config.OrgConfig, orgRepos []string) *forge.FakeClient { @@ -1085,6 +1085,7 @@ func TestBuildLayerStack_NilEnabledRepos_SkipsDisabledRepos(t *testing.T) { []string{"triage"}, nil, "", + "", ) printer := ui.New(&discardWriter{}) @@ -1127,6 +1128,7 @@ func TestBuildLayerStack_EmptyEnabledRepos_IncludesDisabledRepos(t *testing.T) { []string{"triage"}, nil, "", + "", ) printer := ui.New(&discardWriter{}) diff --git a/internal/cli/github.go b/internal/cli/github.go index 2dd31b06a..bfc475199 100644 --- a/internal/cli/github.go +++ b/internal/cli/github.go @@ -207,7 +207,7 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui printer.StepInfo("Reusing existing FULLSEND_GCP_WIF_PROVIDER from " + cfg.target) } - perRepoCfg := config.NewPerRepoConfig(roles) + perRepoCfg := config.NewPerRepoConfig(roles, cfg.target) if err := perRepoCfg.Validate(); err != nil { return fmt.Errorf("invalid config: %w", err) } @@ -434,7 +434,7 @@ func runGitHubSetupPerOrg(ctx context.Context, client forge.Client, printer *ui. for i, ac := range agentCreds { dummyAgents[i] = ac.AgentEntry } - orgCfg := config.NewOrgConfig(repoNames, enabledRepos, roles, dummyAgents, inferenceProviderName) + orgCfg := config.NewOrgConfig(repoNames, enabledRepos, roles, dummyAgents, inferenceProviderName, org) orgCfg.Dispatch.Mode = "oidc-mint" user, err := client.GetAuthenticatedUser(ctx) @@ -483,7 +483,7 @@ func runGitHubSetupPerOrg(ctx context.Context, client forge.Client, printer *ui. for i, ac := range agentCreds { agents[i] = ac.AgentEntry } - orgCfg = config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName) + orgCfg = config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName, org) orgCfg.Dispatch.Mode = "oidc-mint" stack = buildLayerStack(org, client, orgCfg, printer, user, privateRepo, enabledRepos, agentCreds, enrolledRepoIDs, inferenceProvider, cfg.vendorBinary, vendorFn, dispatcher, commitSHA) diff --git a/internal/cli/github_test.go b/internal/cli/github_test.go index 105f588dc..99804e2c9 100644 --- a/internal/cli/github_test.go +++ b/internal/cli/github_test.go @@ -392,7 +392,7 @@ func TestRunGitHubStatus_BasicReport(t *testing.T) { client.Repos = []forge.Repository{ {Name: ".fullsend", FullName: "acme/.fullsend"}, } - cfg := config.NewOrgConfig([]string{"widget"}, []string{"widget"}, []string{"triage"}, nil, "") + cfg := config.NewOrgConfig([]string{"widget"}, []string{"widget"}, []string{"triage"}, nil, "", "") cfgData, _ := cfg.Marshal() client.FileContents["acme/.fullsend/config.yaml"] = cfgData client.OrgVariables = map[string]bool{"acme/FULLSEND_MINT_URL": true} diff --git a/internal/cli/mint.go b/internal/cli/mint.go index 6588bf5e1..7c7808d4b 100644 --- a/internal/cli/mint.go +++ b/internal/cli/mint.go @@ -40,9 +40,10 @@ func defaultMintRoles() []string { } // roleAlias maps role aliases to their canonical names. -// The fix role reuses the coder app — same PEM, same app ID. +// The code and fix roles both reuse the coder app — same PEM, same app ID. var roleAlias = map[string]string{ - "fix": "coder", + "code": "coder", + "fix": "coder", } // resolveRole returns the canonical role name, resolving aliases. diff --git a/internal/cli/mint_test.go b/internal/cli/mint_test.go index 9652e2418..7f009aa9e 100644 --- a/internal/cli/mint_test.go +++ b/internal/cli/mint_test.go @@ -588,6 +588,7 @@ func TestMintStatusCmd_TooManyArgs(t *testing.T) { // --- role aliasing tests --- func TestResolveRole(t *testing.T) { + assert.Equal(t, "coder", resolveRole("code")) assert.Equal(t, "coder", resolveRole("fix")) assert.Equal(t, "coder", resolveRole("coder")) assert.Equal(t, "triage", resolveRole("triage")) diff --git a/internal/cli/reconcilestatus.go b/internal/cli/reconcilestatus.go index 3e3b78653..c636fff82 100644 --- a/internal/cli/reconcilestatus.go +++ b/internal/cli/reconcilestatus.go @@ -7,19 +7,27 @@ import ( "github.com/spf13/cobra" + "github.com/fullsend-ai/fullsend/internal/forge" gh "github.com/fullsend-ai/fullsend/internal/forge/github" + "github.com/fullsend-ai/fullsend/internal/mintclient" "github.com/fullsend-ai/fullsend/internal/statuscomment" ) +var newForgeClient = func(token string) forge.Client { + return gh.New(token) +} + func newReconcileStatusCmd() *cobra.Command { var ( - repo string - number int - runID string - runURL string - sha string - token string - reason string + repo string + number int + runID string + runURL string + sha string + reason string + mintURL string + role string + token string // deprecated: use mintURL ) cmd := &cobra.Command{ @@ -35,13 +43,6 @@ terminal tag (). If found, updates it to an "Interrupted" state and adds the terminal tag. If already finalized, this is a no-op.`, RunE: func(cmd *cobra.Command, args []string) error { - if token == "" { - token = os.Getenv("GITHUB_TOKEN") - } - if token == "" { - return fmt.Errorf("--token or GITHUB_TOKEN required") - } - if number <= 0 { return fmt.Errorf("--number must be a positive integer, got %d", number) } @@ -52,6 +53,34 @@ finalized, this is a no-op.`, } owner, repoName := parts[0], parts[1] + if mintURL == "" { + mintURL = os.Getenv("FULLSEND_MINT_URL") + } + + var client forge.Client + if mintURL != "" { + if role == "" { + return fmt.Errorf("--role is required when using --mint-url") + } + result, err := mintclient.MintToken(cmd.Context(), mintclient.MintRequest{ + MintURL: mintURL, + Role: resolveRole(role), + Repos: []string{repoName}, + }) + if err != nil { + return fmt.Errorf("minting status token: %w", err) + } + if os.Getenv("GITHUB_ACTIONS") == "true" && mintTokenPattern.MatchString(result.Token) { + fmt.Fprintf(os.Stderr, "::add-mask::%s\n", result.Token) + } + client = newForgeClient(result.Token) + } else if token != "" { + fmt.Fprintf(os.Stderr, "WARNING: --token is deprecated; use --mint-url instead\n") + client = newForgeClient(token) + } else { + return fmt.Errorf("--mint-url or FULLSEND_MINT_URL required (--token is deprecated)") + } + var termReason statuscomment.TerminationReason switch reason { case "cancelled": @@ -59,8 +88,6 @@ finalized, this is a no-op.`, default: termReason = statuscomment.ReasonTerminated } - - client := gh.New(token) return statuscomment.ReconcileOrphaned(cmd.Context(), client, owner, repoName, number, runID, runURL, sha, termReason) }, } @@ -70,8 +97,12 @@ finalized, this is a no-op.`, cmd.Flags().StringVar(&runID, "run-id", "", "workflow run ID used in the status comment marker (required)") cmd.Flags().StringVar(&runURL, "run-url", "", "URL to the workflow run (optional)") cmd.Flags().StringVar(&sha, "sha", "", "commit SHA (optional, shown as short hash)") - cmd.Flags().StringVar(&token, "token", "", "GitHub token (default: $GITHUB_TOKEN)") cmd.Flags().StringVar(&reason, "reason", "terminated", "termination reason: terminated or cancelled") + cmd.Flags().StringVar(&mintURL, "mint-url", "", "mint service URL for on-demand token (default: $FULLSEND_MINT_URL)") + cmd.Flags().StringVar(&role, "role", "", "agent role for minting (required with --mint-url)") + cmd.Flags().StringVar(&token, "token", "", "DEPRECATED: use --mint-url instead") + _ = cmd.Flags().MarkDeprecated("token", "use --mint-url instead") + _ = cmd.Flags().MarkHidden("token") _ = cmd.MarkFlagRequired("repo") _ = cmd.MarkFlagRequired("number") _ = cmd.MarkFlagRequired("run-id") diff --git a/internal/cli/reconcilestatus_test.go b/internal/cli/reconcilestatus_test.go index 93875cedd..5c201dfa4 100644 --- a/internal/cli/reconcilestatus_test.go +++ b/internal/cli/reconcilestatus_test.go @@ -1,10 +1,15 @@ package cli import ( + "net/http" + "net/http/httptest" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" + gh "github.com/fullsend-ai/fullsend/internal/forge/github" ) func TestNewReconcileStatusCmd_RequiredFlags(t *testing.T) { @@ -31,20 +36,25 @@ func TestNewReconcileStatusCmd_ValidationErrors(t *testing.T) { wantErr string }{ { - name: "missing token", + name: "missing mint-url", args: []string{"--repo", "org/repo", "--number", "7", "--run-id", "run-1"}, - wantErr: "--token or GITHUB_TOKEN required", + wantErr: "--mint-url or FULLSEND_MINT_URL required", }, { name: "invalid number", - args: []string{"--repo", "org/repo", "--number", "0", "--run-id", "run-1", "--token", "tok"}, + args: []string{"--repo", "org/repo", "--number", "0", "--run-id", "run-1"}, wantErr: "--number must be a positive integer", }, { name: "invalid repo format", - args: []string{"--repo", "noslash", "--number", "7", "--run-id", "run-1", "--token", "tok"}, + args: []string{"--repo", "noslash", "--number", "7", "--run-id", "run-1"}, wantErr: "--repo must be in owner/repo format", }, + { + name: "mint-url without role", + args: []string{"--repo", "org/repo", "--number", "7", "--run-id", "run-1", "--mint-url", "https://mint.example.com"}, + wantErr: "--role is required when using --mint-url", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -56,3 +66,92 @@ func TestNewReconcileStatusCmd_ValidationErrors(t *testing.T) { }) } } + +func TestNewReconcileStatusCmd_MintURLFlags(t *testing.T) { + cmd := newReconcileStatusCmd() + + for _, name := range []string{"mint-url", "role"} { + f := cmd.Flags().Lookup(name) + require.NotNil(t, f, "flag %q should exist", name) + } + + mintURL := cmd.Flags().Lookup("mint-url") + assert.Equal(t, "", mintURL.DefValue) + + role := cmd.Flags().Lookup("role") + assert.Equal(t, "", role.DefValue) +} + +func TestNewReconcileStatusCmd_MintURLFromEnv(t *testing.T) { + t.Setenv("FULLSEND_MINT_URL", "https://mint.example.com") + + cmd := newReconcileStatusCmd() + cmd.SetArgs([]string{"--repo", "org/repo", "--number", "7", "--run-id", "run-1", "--role", "review"}) + err := cmd.Execute() + // Will fail at the OIDC exchange (no ACTIONS_ID_TOKEN_REQUEST_URL), but + // proves the env var was picked up and --role validation passed. + require.Error(t, err) + assert.Contains(t, err.Error(), "minting status token") +} + +func TestNewReconcileStatusCmd_TokenFlagDeprecated(t *testing.T) { + cmd := newReconcileStatusCmd() + f := cmd.Flags().Lookup("token") + require.NotNil(t, f, "--token flag should exist for backwards compatibility") + assert.NotEmpty(t, f.Deprecated, "--token flag should be marked deprecated") +} + +func TestNewReconcileStatusCmd_DeprecatedTokenExecution(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("[]")) + })) + defer srv.Close() + + origNew := newForgeClient + newForgeClient = func(token string) forge.Client { + return gh.New(token).WithBaseURL(srv.URL) + } + defer func() { newForgeClient = origNew }() + + t.Setenv("FULLSEND_MINT_URL", "") + + cmd := newReconcileStatusCmd() + cmd.SetArgs([]string{ + "--repo", "org/repo", + "--number", "7", + "--run-id", "run-1", + "--token", "test-token", + }) + + err := cmd.Execute() + require.NoError(t, err) +} + +func TestNewReconcileStatusCmd_DeprecatedTokenCancelledReason(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("[]")) + })) + defer srv.Close() + + origNew := newForgeClient + newForgeClient = func(token string) forge.Client { + return gh.New(token).WithBaseURL(srv.URL) + } + defer func() { newForgeClient = origNew }() + + t.Setenv("FULLSEND_MINT_URL", "") + + cmd := newReconcileStatusCmd() + cmd.SetArgs([]string{ + "--repo", "org/repo", + "--number", "7", + "--run-id", "run-1", + "--reason", "cancelled", + "--token", "test-token", + }) + + err := cmd.Execute() + require.NoError(t, err) +} diff --git a/internal/cli/run.go b/internal/cli/run.go index a5ff8cd35..ad9d6153f 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -26,6 +26,7 @@ import ( gh "github.com/fullsend-ai/fullsend/internal/forge/github" "github.com/fullsend-ai/fullsend/internal/harness" "github.com/fullsend-ai/fullsend/internal/lock" + "github.com/fullsend-ai/fullsend/internal/mintclient" "github.com/fullsend-ai/fullsend/internal/resolve" agentruntime "github.com/fullsend-ai/fullsend/internal/runtime" "github.com/fullsend-ai/fullsend/internal/sandbox" @@ -63,7 +64,8 @@ type statusOpts struct { runURL string statusRepo string statusNum int - statusToken string + mintURL string + statusToken string // deprecated: use mintURL } func newRunCmd() *cobra.Command { @@ -107,7 +109,10 @@ func newRunCmd() *cobra.Command { cmd.Flags().StringVar(&sOpts.runURL, "run-url", "", "URL of the CI/CD run for status comments") cmd.Flags().StringVar(&sOpts.statusRepo, "status-repo", "", "repository (owner/repo) for status comments") cmd.Flags().IntVar(&sOpts.statusNum, "status-number", 0, "issue/PR number for status comments") - cmd.Flags().StringVar(&sOpts.statusToken, "status-token", "", "token for status comments (defaults to GH_TOKEN)") + cmd.Flags().StringVar(&sOpts.mintURL, "mint-url", "", "mint service URL for on-demand status tokens (default: $FULLSEND_MINT_URL)") + cmd.Flags().StringVar(&sOpts.statusToken, "status-token", "", "DEPRECATED: use --mint-url instead") + _ = cmd.Flags().MarkDeprecated("status-token", "use --mint-url instead") + _ = cmd.Flags().MarkHidden("status-token") _ = cmd.MarkFlagRequired("fullsend-dir") _ = cmd.MarkFlagRequired("target-repo") @@ -400,7 +405,7 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep // post-script — and can report cancellation/failure even when the // sandbox never starts. See #1859. if sOpts.statusRepo != "" && sOpts.statusNum > 0 { - notifier, notifyErr := setupStatusNotifier(absFullsendDir, sOpts, printer) + notifier, notifyErr := setupStatusNotifier(absFullsendDir, agentName, sOpts, printer) if notifyErr != nil { printer.StepWarn("Status notifications disabled: " + notifyErr.Error()) } else { @@ -1840,19 +1845,22 @@ func titleCase(s string) string { return strings.Join(words, " ") } -func setupStatusNotifier(fullsendDir string, sOpts statusOpts, printer *ui.Printer) (*statuscomment.Notifier, error) { +func setupStatusNotifier(fullsendDir string, agentName string, sOpts statusOpts, printer *ui.Printer) (*statuscomment.Notifier, error) { parts := strings.SplitN(sOpts.statusRepo, "/", 2) if len(parts) != 2 { return nil, fmt.Errorf("--status-repo must be in owner/repo format, got %q", sOpts.statusRepo) } owner, repo := parts[0], parts[1] - token := sOpts.statusToken - if token == "" { - token = os.Getenv("GH_TOKEN") + mintURL := sOpts.mintURL + if mintURL == "" { + mintURL = os.Getenv("FULLSEND_MINT_URL") } - if token == "" { - return nil, fmt.Errorf("no status token available (set --status-token or GH_TOKEN)") + + staticToken := sOpts.statusToken + + if mintURL == "" && staticToken == "" { + return nil, fmt.Errorf("no mint URL available (set --mint-url or FULLSEND_MINT_URL)") } var notifyCfg config.StatusNotificationConfig @@ -1868,8 +1876,6 @@ func setupStatusNotifier(fullsendDir string, sOpts statusOpts, printer *ui.Print printer.StepWarn("Failed to read config.yaml for status notifications: " + err.Error()) } - client := gh.New(token) - sha := os.Getenv("GITHUB_SHA") // In cross-repo workflow_dispatch mode, GITHUB_SHA is the dispatching // repo's default branch HEAD — not the PR's head commit. Prefer the @@ -1882,10 +1888,34 @@ func setupStatusNotifier(fullsendDir string, sOpts statusOpts, printer *ui.Print runID = fmt.Sprintf("%d", time.Now().UnixNano()) } - n := statuscomment.New(client, notifyCfg, owner, repo, sOpts.statusNum, sOpts.runURL, sha, runID) + var initialClient forge.Client + if staticToken != "" { + initialClient = gh.New(staticToken) + } + + n := statuscomment.New(initialClient, notifyCfg, owner, repo, sOpts.statusNum, sOpts.runURL, sha, runID) n.SetWarnFunc(func(format string, args ...any) { printer.StepWarn(fmt.Sprintf(format, args...)) }) + + if mintURL != "" { + role := resolveRole(agentName) + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + result, err := mintclient.MintToken(ctx, mintclient.MintRequest{ + MintURL: mintURL, + Role: role, + Repos: []string{repo}, + }) + if err != nil { + return nil, fmt.Errorf("minting status token: %w", err) + } + if os.Getenv("GITHUB_ACTIONS") == "true" && mintTokenPattern.MatchString(result.Token) { + fmt.Fprintf(os.Stderr, "::add-mask::%s\n", result.Token) + } + return gh.New(result.Token), nil + }) + } + return n, nil } diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index 10fdb2a76..e939c9850 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -1311,7 +1311,6 @@ func TestSetupFetchService_ResolvesTokenWhenNoForgeClient(t *testing.T) { h := &harness.Harness{ Agent: "agents/test.md", AllowedRemoteResources: []string{"https://github.com/org/"}, - AllowRuntimeFetch: true, } tokenResolved := false @@ -1356,63 +1355,62 @@ func TestSetupFetchService_NoForgeClientNoRemoteResources(t *testing.T) { assert.NotEmpty(t, env.addr) } -func TestSetupFetchService_CustomMaxFetches(t *testing.T) { +func TestSetupFetchService_TokenResolutionFails(t *testing.T) { tmpDir := t.TempDir() - maxFetches := 50 h := &harness.Harness{ Agent: "agents/test.md", - AllowRuntimeFetch: true, AllowedRemoteResources: []string{"https://github.com/org/"}, - MaxRuntimeFetches: &maxFetches, - } - - cfg := fetchsvc.ServiceConfig{ - Harness: h, - WorkspaceRoot: tmpDir, - MaxFetches: h.EffectiveMaxRuntimeFetches(), } - assert.Equal(t, 50, cfg.MaxFetches) + var warned string env, shutdown, err := setupFetchService( context.Background(), nil, h, - func() (string, error) { return "ghp_test", nil }, - cfg, - func(string) {}, + func() (string, error) { return "", fmt.Errorf("no token available") }, + fetchsvc.ServiceConfig{ + Harness: h, + WorkspaceRoot: tmpDir, + MaxFetches: 10, + }, + func(msg string) { warned = msg }, ) require.NoError(t, err) defer shutdown() assert.NotEmpty(t, env.addr) + assert.Contains(t, warned, "no token available") } -func TestSetupFetchService_TokenResolutionFails(t *testing.T) { +func TestSetupFetchService_CustomMaxFetches(t *testing.T) { tmpDir := t.TempDir() + maxFetches := 50 h := &harness.Harness{ Agent: "agents/test.md", - AllowedRemoteResources: []string{"https://github.com/org/"}, AllowRuntimeFetch: true, + AllowedRemoteResources: []string{"https://github.com/org/"}, + MaxRuntimeFetches: &maxFetches, } - var warned string + cfg := fetchsvc.ServiceConfig{ + Harness: h, + WorkspaceRoot: tmpDir, + MaxFetches: h.EffectiveMaxRuntimeFetches(), + } + assert.Equal(t, 50, cfg.MaxFetches) + env, shutdown, err := setupFetchService( context.Background(), nil, h, - func() (string, error) { return "", fmt.Errorf("no token available") }, - fetchsvc.ServiceConfig{ - Harness: h, - WorkspaceRoot: tmpDir, - MaxFetches: 10, - }, - func(msg string) { warned = msg }, + func() (string, error) { return "ghp_test", nil }, + cfg, + func(string) {}, ) require.NoError(t, err) defer shutdown() assert.NotEmpty(t, env.addr) - assert.Contains(t, warned, "no token available") } func TestEffectiveMaxRuntimeFetches_MatchesFetchsvcDefault(t *testing.T) { @@ -1426,3 +1424,186 @@ func TestEffectiveMaxRuntimeFetches_MatchesFetchsvcDefault(t *testing.T) { type mockForgeClient struct { forge.Client } + +func TestSetupStatusNotifier_MintURL(t *testing.T) { + tmpDir := t.TempDir() + printer := ui.New(io.Discard) + + sOpts := statusOpts{ + statusRepo: "org/repo", + statusNum: 7, + mintURL: "https://mint.example.com", + } + + t.Setenv("GITHUB_RUN_ID", "run-42") + + n, err := setupStatusNotifier(tmpDir, "review", sOpts, printer) + require.NoError(t, err) + assert.NotNil(t, n) + assert.True(t, n.HasClientFactory(), "client factory should be set when mint URL provided") +} + +func TestSetupStatusNotifier_MintURLFromEnv(t *testing.T) { + tmpDir := t.TempDir() + printer := ui.New(io.Discard) + + sOpts := statusOpts{ + statusRepo: "org/repo", + statusNum: 7, + } + + t.Setenv("FULLSEND_MINT_URL", "https://mint.example.com") + t.Setenv("GITHUB_RUN_ID", "run-42") + + n, err := setupStatusNotifier(tmpDir, "code", sOpts, printer) + require.NoError(t, err) + assert.NotNil(t, n) + assert.True(t, n.HasClientFactory(), "client factory should be set from FULLSEND_MINT_URL env var") +} + +func TestSetupStatusNotifier_NoMintURL(t *testing.T) { + tmpDir := t.TempDir() + printer := ui.New(io.Discard) + + sOpts := statusOpts{ + statusRepo: "org/repo", + statusNum: 7, + } + + t.Setenv("GITHUB_RUN_ID", "run-42") + t.Setenv("FULLSEND_MINT_URL", "") + t.Setenv("GITHUB_TOKEN", "") + + _, err := setupStatusNotifier(tmpDir, "review", sOpts, printer) + require.Error(t, err) + assert.Contains(t, err.Error(), "no mint URL available") +} + +func TestSetupStatusNotifier_DeprecatedToken(t *testing.T) { + tmpDir := t.TempDir() + printer := ui.New(io.Discard) + + sOpts := statusOpts{ + statusRepo: "org/repo", + statusNum: 7, + statusToken: "test-static-token", + } + + t.Setenv("GITHUB_RUN_ID", "run-42") + t.Setenv("FULLSEND_MINT_URL", "") + + n, err := setupStatusNotifier(tmpDir, "code", sOpts, printer) + require.NoError(t, err) + assert.NotNil(t, n) + assert.False(t, n.HasClientFactory(), "client factory should not be set when using deprecated static token") +} + +func TestSetupStatusNotifier_InvalidRepo(t *testing.T) { + tmpDir := t.TempDir() + printer := ui.New(io.Discard) + + sOpts := statusOpts{ + statusRepo: "noslash", + statusNum: 7, + } + + _, err := setupStatusNotifier(tmpDir, "review", sOpts, printer) + require.Error(t, err) + assert.Contains(t, err.Error(), "--status-repo must be in owner/repo format") +} + +func TestRunCommand_HasMintURLFlag(t *testing.T) { + cmd := newRunCmd() + + f := cmd.Flags().Lookup("mint-url") + require.NotNil(t, f, "run command should have --mint-url flag") + assert.Equal(t, "", f.DefValue) +} + +func TestRunCommand_StatusTokenFlagDeprecated(t *testing.T) { + cmd := newRunCmd() + + f := cmd.Flags().Lookup("status-token") + require.NotNil(t, f, "run command should have --status-token flag for backwards compatibility") + assert.NotEmpty(t, f.Deprecated, "--status-token flag should be marked deprecated") +} + +func TestTitleCase(t *testing.T) { + tests := []struct { + in, want string + }{ + {"hello world", "Hello World"}, + {"code", "Code"}, + {"", ""}, + {"already Title", "Already Title"}, + } + for _, tt := range tests { + assert.Equal(t, tt.want, titleCase(tt.in)) + } +} + +func TestSetupStatusNotifier_ConfigYAML(t *testing.T) { + tmpDir := t.TempDir() + printer := ui.New(io.Discard) + + configData := `defaults: + status_notifications: + comment: + start: enabled + completion: disabled +` + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "config.yaml"), []byte(configData), 0o644)) + + sOpts := statusOpts{ + statusRepo: "org/repo", + statusNum: 7, + mintURL: "https://mint.example.com", + } + + t.Setenv("GITHUB_RUN_ID", "run-42") + + n, err := setupStatusNotifier(tmpDir, "review", sOpts, printer) + require.NoError(t, err) + assert.NotNil(t, n) +} + +func TestSetupStatusNotifier_RunIDFallback(t *testing.T) { + tmpDir := t.TempDir() + printer := ui.New(io.Discard) + + sOpts := statusOpts{ + statusRepo: "org/repo", + statusNum: 7, + statusToken: "test-static-token", + } + + t.Setenv("GITHUB_RUN_ID", "") + t.Setenv("FULLSEND_MINT_URL", "") + + n, err := setupStatusNotifier(tmpDir, "code", sOpts, printer) + require.NoError(t, err) + assert.NotNil(t, n) +} + +func TestSetupStatusNotifier_PRHeadSHA(t *testing.T) { + tmpDir := t.TempDir() + printer := ui.New(io.Discard) + + eventPayload := `{"inputs":{"event_payload":"{\"pull_request\":{\"head\":{\"sha\":\"abc123def456\"}}}"}}` + eventFile := filepath.Join(tmpDir, "event.json") + require.NoError(t, os.WriteFile(eventFile, []byte(eventPayload), 0o644)) + + sOpts := statusOpts{ + statusRepo: "org/repo", + statusNum: 7, + statusToken: "test-static-token", + } + + t.Setenv("GITHUB_EVENT_PATH", eventFile) + t.Setenv("GITHUB_RUN_ID", "run-42") + t.Setenv("FULLSEND_MINT_URL", "") + + n, err := setupStatusNotifier(tmpDir, "code", sOpts, printer) + require.NoError(t, err) + assert.NotNil(t, n) +} diff --git a/internal/config/config.go b/internal/config/config.go index 01340cb5d..276f3f802 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -58,6 +58,17 @@ type RepoConfig struct { Enabled bool `yaml:"enabled"` } +// AllowTargets defines which orgs and repos agents may create issues in. +type AllowTargets struct { + Orgs []string `yaml:"orgs,omitempty"` + Repos []string `yaml:"repos,omitempty"` +} + +// CreateIssuesConfig controls cross-repo issue creation by agents. +type CreateIssuesConfig struct { + AllowTargets AllowTargets `yaml:"allow_targets"` +} + // OrgConfig is the top-level configuration for a fullsend organization. type OrgConfig struct { Version string `yaml:"version"` @@ -68,6 +79,7 @@ type OrgConfig struct { Agents []AgentEntry `yaml:"agents"` Repos map[string]RepoConfig `yaml:"repos"` AllowedRemoteResources []string `yaml:"allowed_remote_resources,omitempty"` + CreateIssues *CreateIssuesConfig `yaml:"create_issues,omitempty"` } // ValidRoles returns the set of recognized agent roles. @@ -95,7 +107,7 @@ func PerRepoDefaultRoles() []string { } // NewOrgConfig creates a new OrgConfig with sensible defaults. -func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry, inferenceProvider string) *OrgConfig { +func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry, inferenceProvider, org string) *OrgConfig { repos := make(map[string]RepoConfig, len(allRepos)) for _, r := range allRepos { repos[r] = RepoConfig{ @@ -123,6 +135,14 @@ func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry, i if inferenceProvider != "" { cfg.Inference = InferenceConfig{Provider: inferenceProvider} } + if org != "" { + cfg.CreateIssues = &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Orgs: []string{org}, + Repos: []string{"fullsend-ai/fullsend"}, + }, + } + } return cfg } @@ -184,6 +204,9 @@ func (c *OrgConfig) Validate() error { if err := validateStatusNotifications(c.Defaults.StatusNotifications); err != nil { return err } + if err := validateCreateIssues(c.CreateIssues); err != nil { + return err + } return nil } @@ -242,9 +265,10 @@ func (c *OrgConfig) DefaultRoles() []string { // PerRepoConfig holds configuration for per-repo installation mode. // Stored in .fullsend/config.yaml within the target repository. type PerRepoConfig struct { - Version string `yaml:"version"` - KillSwitch bool `yaml:"kill_switch,omitempty"` - Roles []string `yaml:"roles,omitempty"` + Version string `yaml:"version"` + KillSwitch bool `yaml:"kill_switch,omitempty"` + Roles []string `yaml:"roles,omitempty"` + CreateIssues *CreateIssuesConfig `yaml:"create_issues,omitempty"` } const perRepoConfigHeader = `# fullsend per-repo configuration @@ -255,14 +279,22 @@ const perRepoConfigHeader = `# fullsend per-repo configuration ` // NewPerRepoConfig creates a new PerRepoConfig with the given roles. -func NewPerRepoConfig(roles []string) *PerRepoConfig { +func NewPerRepoConfig(roles []string, targetRepo string) *PerRepoConfig { if roles == nil { roles = DefaultAgentRoles() } - return &PerRepoConfig{ + cfg := &PerRepoConfig{ Version: "1", Roles: roles, } + if targetRepo != "" { + cfg.CreateIssues = &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Repos: []string{targetRepo, "fullsend-ai/fullsend"}, + }, + } + } + return cfg } // ParsePerRepoConfig parses YAML bytes into a PerRepoConfig. @@ -299,5 +331,26 @@ func (c *PerRepoConfig) Validate() error { } seen[role] = true } + if err := validateCreateIssues(c.CreateIssues); err != nil { + return err + } + return nil +} + +func validateCreateIssues(cfg *CreateIssuesConfig) error { + if cfg == nil { + return nil + } + for _, org := range cfg.AllowTargets.Orgs { + if org == "" { + return fmt.Errorf("create_issues: empty org in allow_targets.orgs") + } + } + for _, repo := range cfg.AllowTargets.Repos { + parts := strings.SplitN(repo, "/", 2) + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return fmt.Errorf("create_issues: repo %q in allow_targets.repos must contain owner/name", repo) + } + } return nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c1145ac40..a9ce98b57 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -41,7 +41,7 @@ func TestNewOrgConfig(t *testing.T) { {Role: "fullsend", Name: "test", Slug: "test-slug"}, } - cfg := NewOrgConfig(allRepos, enabledRepos, roles, agents, "") + cfg := NewOrgConfig(allRepos, enabledRepos, roles, agents, "", "") assert.Equal(t, "1", cfg.Version) assert.Equal(t, "github-actions", cfg.Dispatch.Platform) @@ -285,12 +285,12 @@ repos: } func TestNewOrgConfig_WithInferenceProvider(t *testing.T) { - cfg := NewOrgConfig(nil, nil, nil, nil, "vertex") + cfg := NewOrgConfig(nil, nil, nil, nil, "vertex", "") assert.Equal(t, "vertex", cfg.Inference.Provider) } func TestNewOrgConfig_WithoutInferenceProvider(t *testing.T) { - cfg := NewOrgConfig(nil, nil, nil, nil, "") + cfg := NewOrgConfig(nil, nil, nil, nil, "", "") assert.Empty(t, cfg.Inference.Provider) } @@ -447,7 +447,7 @@ func TestOrgConfigValidate_FixRole(t *testing.T) { } func TestNewOrgConfig_KillSwitchDefaultFalse(t *testing.T) { - cfg := NewOrgConfig(nil, nil, []string{"fullsend"}, nil, "") + cfg := NewOrgConfig(nil, nil, []string{"fullsend"}, nil, "", "") assert.False(t, cfg.KillSwitch) } @@ -563,14 +563,14 @@ func TestOrgConfigMarshal_WithDispatchMode(t *testing.T) { } func TestNewPerRepoConfig_DefaultRoles(t *testing.T) { - cfg := NewPerRepoConfig(nil) + cfg := NewPerRepoConfig(nil, "") assert.Equal(t, "1", cfg.Version) assert.Equal(t, DefaultAgentRoles(), cfg.Roles) assert.False(t, cfg.KillSwitch) } func TestNewPerRepoConfig_CustomRoles(t *testing.T) { - cfg := NewPerRepoConfig([]string{"triage", "review"}) + cfg := NewPerRepoConfig([]string{"triage", "review"}, "") assert.Equal(t, []string{"triage", "review"}, cfg.Roles) } @@ -666,7 +666,7 @@ func TestPerRepoConfigMarshal_KillSwitchOmitted(t *testing.T) { } func TestPerRepoConfig_RoundTrip(t *testing.T) { - original := NewPerRepoConfig([]string{"fullsend", "triage", "coder", "review", "fix"}) + original := NewPerRepoConfig([]string{"fullsend", "triage", "coder", "review", "fix"}, "") data, err := original.Marshal() require.NoError(t, err) @@ -881,3 +881,195 @@ func TestOrgConfigMarshal_WithoutStatusNotifications(t *testing.T) { require.NoError(t, err) assert.NotContains(t, string(data), "status_notifications") } + +// --- CreateIssues tests --- + +func TestOrgConfig_CreateIssues_ParseYAML(t *testing.T) { + yamlData := ` +version: "1" +dispatch: + platform: github-actions +defaults: + roles: + - fullsend + max_implementation_retries: 2 +agents: [] +repos: {} +create_issues: + allow_targets: + orgs: + - my-org + - other-org + repos: + - external-org/some-repo +` + cfg, err := ParseOrgConfig([]byte(yamlData)) + require.NoError(t, err) + require.NotNil(t, cfg.CreateIssues) + assert.Equal(t, []string{"my-org", "other-org"}, cfg.CreateIssues.AllowTargets.Orgs) + assert.Equal(t, []string{"external-org/some-repo"}, cfg.CreateIssues.AllowTargets.Repos) +} + +func TestOrgConfig_CreateIssues_OmittedWhenEmpty(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + Agents: []AgentEntry{}, + Repos: map[string]RepoConfig{}, + } + data, err := cfg.Marshal() + require.NoError(t, err) + assert.NotContains(t, string(data), "create_issues") +} + +func TestOrgConfig_CreateIssues_Marshal(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + Agents: []AgentEntry{}, + Repos: map[string]RepoConfig{}, + CreateIssues: &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Orgs: []string{"my-org"}, + Repos: []string{"other/repo"}, + }, + }, + } + data, err := cfg.Marshal() + require.NoError(t, err) + assert.Contains(t, string(data), "create_issues:") + assert.Contains(t, string(data), "allow_targets:") + assert.Contains(t, string(data), "my-org") + assert.Contains(t, string(data), "other/repo") +} + +func TestOrgConfigValidate_CreateIssues_InvalidRepoFormat(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + CreateIssues: &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Repos: []string{"no-slash-here"}, + }, + }, + } + err := cfg.Validate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "no-slash-here") +} + +func TestOrgConfigValidate_CreateIssues_MalformedRepoFormat(t *testing.T) { + malformed := []string{"/", "/repo", "owner/", "//"} + for _, repo := range malformed { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + CreateIssues: &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Repos: []string{repo}, + }, + }, + } + err := cfg.Validate() + assert.Error(t, err, "expected error for repo %q", repo) + assert.Contains(t, err.Error(), "owner/name", "expected owner/name message for repo %q", repo) + } +} + +func TestOrgConfigValidate_CreateIssues_EmptyOrg(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + CreateIssues: &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Orgs: []string{"valid-org", ""}, + }, + }, + } + err := cfg.Validate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "empty org") +} + +func TestOrgConfigValidate_CreateIssues_Valid(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + CreateIssues: &CreateIssuesConfig{ + AllowTargets: AllowTargets{ + Orgs: []string{"my-org"}, + Repos: []string{"other/repo"}, + }, + }, + } + err := cfg.Validate() + assert.NoError(t, err) +} + +func TestOrgConfigValidate_CreateIssues_Nil(t *testing.T) { + cfg := &OrgConfig{ + Version: "1", + Dispatch: DispatchConfig{Platform: "github-actions"}, + Defaults: RepoDefaults{ + Roles: []string{"fullsend"}, + MaxImplementationRetries: 2, + }, + } + err := cfg.Validate() + assert.NoError(t, err) +} + +func TestNewOrgConfig_CreateIssuesDefaults(t *testing.T) { + cfg := NewOrgConfig(nil, nil, []string{"fullsend"}, nil, "", "my-org") + require.NotNil(t, cfg.CreateIssues) + assert.Equal(t, []string{"my-org"}, cfg.CreateIssues.AllowTargets.Orgs) + assert.Equal(t, []string{"fullsend-ai/fullsend"}, cfg.CreateIssues.AllowTargets.Repos) +} + +func TestPerRepoConfig_CreateIssues_ParseYAML(t *testing.T) { + yamlData := ` +version: "1" +roles: + - fullsend + - triage +create_issues: + allow_targets: + repos: + - my-org/my-repo + - fullsend-ai/fullsend +` + cfg, err := ParsePerRepoConfig([]byte(yamlData)) + require.NoError(t, err) + require.NotNil(t, cfg.CreateIssues) + assert.Equal(t, []string{"my-org/my-repo", "fullsend-ai/fullsend"}, cfg.CreateIssues.AllowTargets.Repos) +} + +func TestNewPerRepoConfig_CreateIssuesDefaults(t *testing.T) { + cfg := NewPerRepoConfig(nil, "my-org/my-repo") + require.NotNil(t, cfg.CreateIssues) + assert.Equal(t, []string{"my-org/my-repo", "fullsend-ai/fullsend"}, cfg.CreateIssues.AllowTargets.Repos) +} diff --git a/internal/harness/scaffold_integration_test.go b/internal/harness/scaffold_integration_test.go new file mode 100644 index 000000000..519355f03 --- /dev/null +++ b/internal/harness/scaffold_integration_test.go @@ -0,0 +1,344 @@ +package harness + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "os" + "path/filepath" + "sort" + "testing" + + "github.com/fullsend-ai/fullsend/internal/scaffold" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// extractScaffoldHarnessDir writes all embedded scaffold files to dir and +// returns the harness subdirectory path. +func extractScaffoldHarnessDir(t *testing.T, dir string) string { + t.Helper() + err := scaffold.WalkFullsendRepoAll(func(path string, content []byte) error { + dest := filepath.Join(dir, path) + if mkErr := os.MkdirAll(filepath.Dir(dest), 0o755); mkErr != nil { + return mkErr + } + return os.WriteFile(dest, content, 0o644) + }) + require.NoError(t, err, "extracting scaffold") + return filepath.Join(dir, "harness") +} + +// TestLoadWithBase_WrapperMergesScaffold verifies the full pipeline: a thin +// wrapper harness with base: pointing to a local scaffold harness loads and +// merges correctly, producing the expected role/slug overrides and inherited fields. +func TestLoadWithBase_WrapperMergesScaffold(t *testing.T) { + dir := t.TempDir() + harnessDir := extractScaffoldHarnessDir(t, dir) + + wrapperPath := writeTestHarness(t, harnessDir, "wrapper-triage.yaml", ` +base: triage.yaml +role: triage +slug: test-triage +`) + + h, deps, err := LoadWithBase(context.Background(), wrapperPath, ComposeOpts{ + ForgePlatform: "github", + }) + require.NoError(t, err) + + // Role and slug come from wrapper (overrides base). + assert.Equal(t, "triage", h.Role) + assert.Equal(t, "test-triage", h.Slug) + + // Agent, model, image, policy inherited from base. + assert.Equal(t, "agents/triage.md", h.Agent) + assert.Equal(t, "opus", h.Model) + assert.Equal(t, "ghcr.io/fullsend-ai/fullsend-sandbox:latest", h.Image) + assert.Equal(t, "policies/triage.yaml", h.Policy) + + // PreScript and PostScript populated after forge.github resolution. + assert.NotEmpty(t, h.PreScript, "PreScript should be set after forge resolution") + assert.NotEmpty(t, h.PostScript, "PostScript should be set after forge resolution") + + // RunnerEnv contains both top-level keys and forge.github keys after merge. + assert.Contains(t, h.RunnerEnv, "FULLSEND_OUTPUT_SCHEMA", "should have top-level runner_env key") + assert.Contains(t, h.RunnerEnv, "GH_TOKEN", "should have forge.github runner_env key") + assert.Contains(t, h.RunnerEnv, "GITHUB_ISSUE_URL", "should have forge.github runner_env key") + + // Skills includes base top-level skills (forge skills are concatenated by ResolveForge, + // but the triage template has no forge-specific skills — only runner_env and scripts). + assert.Contains(t, h.Skills, "skills/issue-labels") + + // Forge map is nil (consumed by ResolveForge). + assert.Nil(t, h.Forge) + + // Base field is empty (consumed by LoadWithBase). + assert.Empty(t, h.Base) + + // Local base -> no URL deps. + assert.Nil(t, deps) + + // ValidationLoop inherited from base. + assert.NotNil(t, h.ValidationLoop) + assert.Equal(t, "scripts/validate-output-schema.sh", h.ValidationLoop.Script) + assert.Equal(t, 2, h.ValidationLoop.MaxIterations) +} + +// TestLoadWithBase_WrapperOverridesBaseFields verifies that wrapper-level +// overrides (model, slug) take precedence over base values while other fields inherit. +func TestLoadWithBase_WrapperOverridesBaseFields(t *testing.T) { + dir := t.TempDir() + harnessDir := extractScaffoldHarnessDir(t, dir) + + wrapperPath := writeTestHarness(t, harnessDir, "wrapper-custom.yaml", ` +base: code.yaml +role: coder +slug: my-org-coder +model: sonnet +`) + + h, _, err := LoadWithBase(context.Background(), wrapperPath, ComposeOpts{ + ForgePlatform: "github", + }) + require.NoError(t, err) + + assert.Equal(t, "coder", h.Role) + assert.Equal(t, "my-org-coder", h.Slug) + assert.Equal(t, "sonnet", h.Model, "wrapper model should override base model") + assert.Equal(t, "agents/code.md", h.Agent, "agent should be inherited from base") + assert.Equal(t, "ghcr.io/fullsend-ai/fullsend-code:latest", h.Image, "image should be inherited from base") +} + +// TestLoadWithOpts_ScaffoldTemplatesForgeResolution loads every scaffold harness +// template with ForgePlatform: "github" and verifies the merged state is +// consistent — pre/post scripts populated, runner_env merged, forge consumed. +func TestLoadWithOpts_ScaffoldTemplatesForgeResolution(t *testing.T) { + dir := t.TempDir() + harnessDir := extractScaffoldHarnessDir(t, dir) + + names, err := scaffold.HarnessNames() + require.NoError(t, err) + require.NotEmpty(t, names) + + for _, name := range names { + t.Run(name, func(t *testing.T) { + path := filepath.Join(harnessDir, name+".yaml") + + h, loadErr := LoadWithOpts(path, LoadOpts{ForgePlatform: "github"}) + require.NoError(t, loadErr) + + assert.NotEmpty(t, h.PreScript, "PreScript should be set after forge resolution") + assert.NotEmpty(t, h.PostScript, "PostScript should be set after forge resolution") + assert.NotEmpty(t, h.RunnerEnv, "RunnerEnv should be non-empty after merge") + assert.Nil(t, h.Forge, "Forge should be nil after resolution") + assert.NotEmpty(t, h.Role, "Role should be set in scaffold template") + assert.NotEmpty(t, h.Slug, "Slug should be set in scaffold template") + }) + } +} + +// TestLoad_ScaffoldTemplatesBackwardCompat loads every scaffold harness template +// via Load() (no forge platform) and verifies backward compatibility: the +// harness loads without error, top-level defaults are present, and the forge +// map is retained (not consumed). +func TestLoad_ScaffoldTemplatesBackwardCompat(t *testing.T) { + dir := t.TempDir() + harnessDir := extractScaffoldHarnessDir(t, dir) + + names, err := scaffold.HarnessNames() + require.NoError(t, err) + + for _, name := range names { + t.Run(name, func(t *testing.T) { + path := filepath.Join(harnessDir, name+".yaml") + + h, loadErr := Load(path) + require.NoError(t, loadErr) + + // Top-level pre/post scripts serve as defaults. + assert.NotEmpty(t, h.PreScript, "PreScript should be set at top level as default") + assert.NotEmpty(t, h.PostScript, "PostScript should be set at top level as default") + + // Forge map is present and has "github" key. + assert.NotNil(t, h.Forge, "Forge map should be present") + assert.Contains(t, h.Forge, "github", "Forge should have a github key") + }) + } +} + +// TestDiscoverAgents_ScaffoldDirectory extracts the scaffold to a temp dir, +// runs DiscoverAgents on the harness directory, and verifies all agents are +// discovered with correct role/slug pairs. +func TestDiscoverAgents_ScaffoldDirectory(t *testing.T) { + dir := t.TempDir() + harnessDir := extractScaffoldHarnessDir(t, dir) + + agents, err := DiscoverAgents(harnessDir) + require.NoError(t, err) + + // Expect all 6 scaffold harnesses discovered. + require.Len(t, agents, 6, "should discover all 6 scaffold harnesses") + + // Build a map of filename -> AgentInfo for easier assertion. + byFilename := make(map[string]AgentInfo, len(agents)) + for _, a := range agents { + byFilename[a.Filename] = a + } + + expected := map[string]struct{ role, slug string }{ + "code.yaml": {"coder", "fullsend-ai-coder"}, + "fix.yaml": {"coder", "fullsend-ai-coder"}, + "prioritize.yaml": {"prioritize", "fullsend-ai-prioritize"}, + "retro.yaml": {"retro", "fullsend-ai-retro"}, + "review.yaml": {"review", "fullsend-ai-review"}, + "triage.yaml": {"triage", "fullsend-ai-triage"}, + } + + for filename, want := range expected { + got, ok := byFilename[filename] + require.True(t, ok, "should discover %s", filename) + assert.Equal(t, want.role, got.Role, "%s role", filename) + assert.Equal(t, want.slug, got.Slug, "%s slug", filename) + assert.True(t, filepath.IsAbs(got.Path), "%s path should be absolute", filename) + } + + // Verify sort order: by role, then by filename. + sorted := make([]AgentInfo, len(agents)) + copy(sorted, agents) + sort.Slice(sorted, func(i, j int) bool { + if sorted[i].Role != sorted[j].Role { + return sorted[i].Role < sorted[j].Role + } + return sorted[i].Filename < sorted[j].Filename + }) + assert.Equal(t, sorted, agents, "results should be sorted by role then filename") +} + +// TestHarnessContentHash_MatchesEmbeddedContent verifies that HarnessContentHash +// produces correct SHA-256 hashes matching the embedded file content, and that +// HarnessBaseURLWithHash produces well-formed URLs with matching hash fragments. +func TestHarnessContentHash_MatchesEmbeddedContent(t *testing.T) { + names, err := scaffold.HarnessNames() + require.NoError(t, err) + + fakeCommitSHA := "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2" + + for _, name := range names { + t.Run(name, func(t *testing.T) { + // Compute hash via the scaffold package. + hash, err := scaffold.HarnessContentHash(name) + require.NoError(t, err) + assert.Len(t, hash, 64, "SHA-256 hex digest should be 64 characters") + + // Independently compute hash from the embedded file content. + content, err := scaffold.FullsendRepoFile("harness/" + name + ".yaml") + require.NoError(t, err) + sum := sha256.Sum256(content) + independentHash := hex.EncodeToString(sum[:]) + assert.Equal(t, independentHash, hash, + "HarnessContentHash should match sha256 of embedded file content") + + // Verify HarnessBaseURLWithHash produces a valid URL with matching hash. + fullURL, err := scaffold.HarnessBaseURLWithHash(name, fakeCommitSHA) + require.NoError(t, err) + assert.Contains(t, fullURL, fakeCommitSHA) + assert.Contains(t, fullURL, name+".yaml") + assert.Contains(t, fullURL, "#sha256="+hash) + }) + } +} + +// TestLoadRaw_GeneratedWrapperFormat verifies that the wrapper YAML format +// produced by HarnessWrappersLayer (base + role + slug) parses correctly via +// LoadRaw and contains the expected identity fields. +func TestLoadRaw_GeneratedWrapperFormat(t *testing.T) { + names, err := scaffold.HarnessNames() + require.NoError(t, err) + + fakeCommitSHA := "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2" + + for _, name := range names { + t.Run(name, func(t *testing.T) { + baseURL, err := scaffold.HarnessBaseURLWithHash(name, fakeCommitSHA) + require.NoError(t, err) + + // Simulate the wrapper format produced by HarnessWrappersLayer. + wrapperYAML := "base: " + baseURL + "\n" + + "role: " + name + "\n" + + "slug: test-" + name + "\n" + + dir := t.TempDir() + path := writeTestHarness(t, dir, name+".yaml", wrapperYAML) + + h, err := LoadRaw(path) + require.NoError(t, err) + + assert.Equal(t, baseURL, h.Base, "base should be the full URL with hash") + assert.Equal(t, name, h.Role) + assert.Equal(t, "test-"+name, h.Slug) + }) + } +} + +// TestResolveForge_ScaffoldRunnerEnvMerge verifies that forge resolution +// produces the expected merged runner_env for each scaffold template, with +// both top-level (platform-neutral) and forge.github (platform-specific) +// keys present in the final merged state. +func TestResolveForge_ScaffoldRunnerEnvMerge(t *testing.T) { + dir := t.TempDir() + harnessDir := extractScaffoldHarnessDir(t, dir) + + tests := []struct { + file string + topLevelKeys []string + forgeGithubKeys []string + }{ + { + file: "triage.yaml", + topLevelKeys: []string{"FULLSEND_OUTPUT_SCHEMA"}, + forgeGithubKeys: []string{"GITHUB_ISSUE_URL", "GH_TOKEN"}, + }, + { + file: "code.yaml", + topLevelKeys: []string{"TARGET_BRANCH"}, + forgeGithubKeys: []string{"PUSH_TOKEN", "PUSH_TOKEN_SOURCE", "REPO_FULL_NAME", "ISSUE_NUMBER", "REPO_DIR"}, + }, + { + file: "review.yaml", + topLevelKeys: []string{"FULLSEND_OUTPUT_SCHEMA"}, + forgeGithubKeys: []string{"REVIEW_TOKEN", "REPO_FULL_NAME", "PR_NUMBER", "GITHUB_PR_URL"}, + }, + { + file: "fix.yaml", + topLevelKeys: []string{"TARGET_BRANCH", "TRIGGER_SOURCE", "HUMAN_INSTRUCTION", "FIX_ITERATION", "REVIEW_BODY_FILE", "PRE_AGENT_HEAD", "FULLSEND_OUTPUT_SCHEMA", "FULLSEND_OUTPUT_FILE"}, + forgeGithubKeys: []string{"PUSH_TOKEN", "PUSH_TOKEN_SOURCE", "REPO_FULL_NAME", "PR_NUMBER", "REPO_DIR"}, + }, + { + file: "retro.yaml", + topLevelKeys: []string{"FULLSEND_OUTPUT_SCHEMA"}, + forgeGithubKeys: []string{"ORIGINATING_URL", "REPO_FULL_NAME", "GH_TOKEN"}, + }, + { + file: "prioritize.yaml", + topLevelKeys: []string{"FULLSEND_OUTPUT_SCHEMA"}, + forgeGithubKeys: []string{"GITHUB_ISSUE_URL", "GH_TOKEN", "ORG", "PROJECT_NUMBER"}, + }, + } + + for _, tt := range tests { + t.Run(tt.file, func(t *testing.T) { + path := filepath.Join(harnessDir, tt.file) + + h, loadErr := LoadWithOpts(path, LoadOpts{ForgePlatform: "github"}) + require.NoError(t, loadErr) + + for _, key := range tt.topLevelKeys { + assert.Contains(t, h.RunnerEnv, key, "merged RunnerEnv should contain top-level key %s", key) + } + for _, key := range tt.forgeGithubKeys { + assert.Contains(t, h.RunnerEnv, key, "merged RunnerEnv should contain forge.github key %s", key) + } + }) + } +} diff --git a/internal/layers/configrepo_test.go b/internal/layers/configrepo_test.go index ebf807956..3277fa5e7 100644 --- a/internal/layers/configrepo_test.go +++ b/internal/layers/configrepo_test.go @@ -22,6 +22,7 @@ func newTestConfig(t *testing.T) *config.OrgConfig { []string{"coder"}, []config.AgentEntry{{Role: "coder", Name: "Bot", Slug: "bot-slug"}}, "", + "", ) } diff --git a/internal/layers/enrollment.go b/internal/layers/enrollment.go index d418ec442..4e00aef04 100644 --- a/internal/layers/enrollment.go +++ b/internal/layers/enrollment.go @@ -15,6 +15,17 @@ const ( // repoMaintenanceWorkflow is the workflow file that handles enrollment. repoMaintenanceWorkflow = "repo-maintenance.yml" + + // enrollmentWaitTimeout is the maximum time to wait for the + // repo-maintenance workflow run to appear and complete. + enrollmentWaitTimeout = 3 * time.Minute + + // enrollmentPollInitial is the initial polling interval for + // workflow run status checks. + enrollmentPollInitial = 2 * time.Second + + // enrollmentPollMax is the maximum polling interval (backoff cap). + enrollmentPollMax = 15 * time.Second ) // EnrollmentLayer monitors workflow-driven enrollment of target repos. @@ -82,11 +93,11 @@ func (l *EnrollmentLayer) Install(ctx context.Context) error { } l.ui.StepDone("dispatched repo-maintenance workflow") - // Wait for the workflow run to complete. + // Wait for the workflow run to complete (bounded by enrollmentWaitTimeout). + l.ui.StepStart("waiting for enrollment workflow to complete") run, err := l.awaitWorkflowRun(ctx, dispatchTime) if err != nil { l.ui.StepWarn(fmt.Sprintf("could not confirm enrollment: %v", err)) - l.ui.StepInfo("check the repo-maintenance workflow in .fullsend for results") return nil // non-fatal — enrollment may still succeed } @@ -105,18 +116,35 @@ func (l *EnrollmentLayer) Install(ctx context.Context) error { } // awaitWorkflowRun polls for a repo-maintenance workflow run created after -// dispatchTime and waits for it to complete. +// dispatchTime and waits for it to complete. It uses exponential backoff +// and a bounded timeout to avoid long silent waits. func (l *EnrollmentLayer) awaitWorkflowRun(ctx context.Context, dispatchTime time.Time) (*forge.WorkflowRun, error) { - for attempt := range 36 { // 3 minutes max + deadline := time.Now().Add(enrollmentWaitTimeout) + interval := enrollmentPollInitial + start := time.Now() + + for { + if time.Now().After(deadline) { + elapsed := time.Since(start).Round(time.Second) + return nil, fmt.Errorf( + "timed out after %s waiting for repo-maintenance workflow; "+ + "check the workflow in .fullsend and re-run install if needed", + elapsed, + ) + } + select { case <-ctx.Done(): return nil, ctx.Err() - case <-time.After(5 * time.Second): + case <-time.After(interval): } + elapsed := time.Since(start).Round(time.Second) + runs, err := l.client.ListWorkflowRuns(ctx, l.org, forge.ConfigRepoName, repoMaintenanceWorkflow) if err != nil { - l.ui.StepInfo(fmt.Sprintf("waiting for workflow run (attempt %d)...", attempt+1)) + l.ui.StepInfo(fmt.Sprintf("waiting for workflow registration (%s elapsed)...", elapsed)) + interval = nextInterval(interval) continue } @@ -133,11 +161,21 @@ func (l *EnrollmentLayer) awaitWorkflowRun(ctx context.Context, dispatchTime tim if run.Status == "completed" { return run, nil } - l.ui.StepInfo(fmt.Sprintf("workflow run: %s (%s)", run.HTMLURL, run.Status)) + l.ui.StepInfo(fmt.Sprintf("workflow run %s (%s, %s elapsed)", run.HTMLURL, run.Status, elapsed)) break // found our run, keep waiting } + + interval = nextInterval(interval) + } +} + +// nextInterval doubles the polling interval up to enrollmentPollMax. +func nextInterval(current time.Duration) time.Duration { + next := current * 2 + if next > enrollmentPollMax { + return enrollmentPollMax } - return nil, fmt.Errorf("timed out waiting for repo-maintenance workflow") + return next } // showWorkflowLogs fetches and displays workflow run logs locally so the user diff --git a/internal/layers/enrollment_test.go b/internal/layers/enrollment_test.go index 2d243af95..701f58715 100644 --- a/internal/layers/enrollment_test.go +++ b/internal/layers/enrollment_test.go @@ -470,3 +470,40 @@ func TestEnrollmentLayer_Analyze_PerRepoGuardCheckError(t *testing.T) { assert.Contains(t, report.Details[0], "all 1 repos failed guard check") assert.Contains(t, report.Details[1], "guard check failed, skipped") } + +func TestEnrollmentLayer_Install_ContextCancelled(t *testing.T) { + // No workflow runs configured — awaitWorkflowRun will poll until + // context is cancelled. + client := &forge.FakeClient{} + repos := []string{"repo-a"} + layer, buf := newEnrollmentLayer(t, client, repos, nil) + + ctx, cancel := context.WithCancel(context.Background()) + // Cancel immediately so the first poll iteration exits. + cancel() + + err := layer.Install(ctx) + require.NoError(t, err) // Install treats timeout/cancel as non-fatal + + output := buf.String() + assert.Contains(t, output, "could not confirm enrollment") +} + +func TestNextInterval(t *testing.T) { + tests := []struct { + name string + current time.Duration + expected time.Duration + }{ + {"doubles small interval", 2 * time.Second, 4 * time.Second}, + {"doubles again", 4 * time.Second, 8 * time.Second}, + {"caps at max", 8 * time.Second, enrollmentPollMax}, + {"stays at max", enrollmentPollMax, enrollmentPollMax}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := nextInterval(tt.current) + assert.Equal(t, tt.expected, got) + }) + } +} diff --git a/internal/scaffold/fullsend-repo/agents/triage.md b/internal/scaffold/fullsend-repo/agents/triage.md index 01f4dcf68..7749861fb 100644 --- a/internal/scaffold/fullsend-repo/agents/triage.md +++ b/internal/scaffold/fullsend-repo/agents/triage.md @@ -63,18 +63,18 @@ gh pr list --repo OTHER-ORG/OTHER-REPO --state open --search "relevant keywords" If a cross-repo search fails or returns an error (e.g., due to access restrictions), note this in your reasoning as an information gap rather than concluding no blocking work exists. -### 2c. Check existing blockers +### 2c. Check existing prerequisites -If the issue already has a `blocked` label, check whether the previously identified blocker (linked in prior triage comments) is still open. Fetch the full context of the blocking issue or PR to understand its current state: +If the issue already has a `blocked` label, check whether the previously identified prerequisites (linked in prior triage comments) are still open. Fetch the full context of each prerequisite issue or PR to understand its current state: ``` -# For blocking issues: -gh issue view BLOCKING_URL --json state,title,body,comments,labels -# For blocking PRs: -gh pr view BLOCKING_URL --json state,title,body,comments,labels,mergedAt +# For prerequisite issues: +gh issue view PREREQUISITE_URL --json state,title,body,comments,labels +# For prerequisite PRs: +gh pr view PREREQUISITE_URL --json state,title,body,comments,labels,mergedAt ``` -Use `gh issue view` for `/issues/` URLs and `gh pr view` for `/pull/` URLs. Review the blocker's state, recent comments, and labels to determine whether the dependency has been resolved, is making progress, or remains stalled. If the blocker has been closed or merged, the block may be resolved — proceed with a fresh assessment. +Use `gh issue view` for `/issues/` URLs and `gh pr view` for `/pull/` URLs. Review the prerequisite's state, recent comments, and labels to determine whether the dependency has been resolved, is making progress, or remains stalled. If the prerequisite has been closed or merged, the dependency may be resolved — proceed with a fresh assessment. ### 2d. Review prior triage analysis @@ -126,7 +126,7 @@ Before forming any clarifying question, classify it: ### Phase 3 — Hypothesis formation and dependency analysis - Can you form a plausible root cause hypothesis from the available information? - Could a developer start investigating without contacting the reporter? -- **Is progress blocked on other work?** Consider whether the fix depends on an unresolved issue or unmerged PR — in this repo or another. If a developer cannot meaningfully start work until some other issue is resolved, this issue is blocked regardless of how clear the problem description is. +- **Is progress blocked on other work?** Consider whether the fix depends on an unresolved issue or unmerged PR — in this repo or another. If a developer cannot meaningfully start work until some other issue is resolved, this issue has prerequisites regardless of how clear the problem description is. If the blocking work has no tracking issue yet, you can recommend creating one via the `prerequisites` action's `create` array. ### Clarity scoring @@ -145,6 +145,8 @@ Calculate overall clarity: `symptom*0.35 + cause*0.30 + reproduction*0.20 + impa **Anti-premature-resolution rule (HARD CONSTRAINT):** If your assessment identifies ANY open *user-facing* questions or information gaps — regardless of whether they seem minor — you MUST use `action: "insufficient"` and ask a clarifying question. Do NOT emit `action: "sufficient"` with user-facing information gaps. The `sufficient` action means there are zero open user-facing questions that could affect implementation. When in doubt, ask. Implementation-facing questions that cannot be self-resolved from repository context should be noted in `reasoning` but do not require `action: "insufficient"` unless they materially prevent triage — see the question classification rules above. +**Anti-premature-prerequisites rule (HARD CONSTRAINT):** If your assessment identifies unresolved prerequisites — dependencies on work in other repos or unmerged changes that must land first — you MUST use `action: "prerequisites"`. Do NOT emit `action: "sufficient"` when prerequisites exist. The `sufficient` action means there are zero blockers and zero open questions. + ## Step 4: Decide and write result Based on your assessment, choose exactly one action and write the result as JSON to `$FULLSEND_OUTPUT_DIR/agent-result.json`. @@ -200,18 +202,36 @@ This issue describes the same problem as an existing open issue. } ``` -### Action: `blocked` +### Action: `prerequisites` + +Progress on this issue depends on work that must happen first — either in this repository or another. Use this action when you identify specific blocking dependencies: existing issues/PRs that must be resolved, or upstream work that needs a tracking issue created. + +**HARD CONSTRAINT:** Never emit `sufficient` if unresolved prerequisites exist. Use `prerequisites` instead. -Progress on this issue is blocked by another issue or PR — either in this repository or a different one. The blocking issue must be resolved before work on this issue can proceed. Do NOT apply `ready-to-code` for blocked issues. +The `prerequisites` object contains two arrays: -Only use `blocked` when you can identify a specific open issue or PR that must be resolved first. If you suspect a dependency but cannot find a concrete blocking issue, use `insufficient` to ask the reporter whether there is a blocking dependency and to provide its URL. +- `existing` — issues or PRs that already exist and block this work. Include the full HTML URL. +- `create` — issues that need to be filed in other repos before this work can proceed. Include the target `repo` (owner/name format), a `title`, and a `body`. Write the body for the target repo's audience — include enough technical context for upstream maintainers to understand what is needed. Use your judgment on whether to include a back-reference to the originating issue; sometimes it provides helpful context, sometimes it leaks internal details. + +At least one of the two arrays must have entries. ```json { - "action": "blocked", - "reasoning": "Brief explanation of why this issue is blocked and what the dependency is", - "blocked_by": "https://github.com/org/repo/issues/99", - "comment": "A professional comment explaining the blocking dependency. Link to the blocking issue or PR and explain why this issue cannot proceed until it is resolved. Be specific about the dependency — what does the blocking issue provide or unblock?" + "action": "prerequisites", + "reasoning": "Brief explanation of the dependencies and why this issue cannot proceed", + "prerequisites": { + "existing": [ + { "url": "https://github.com/org/repo/issues/99" } + ], + "create": [ + { + "repo": "org/upstream-lib", + "title": "Add support for X", + "body": "Technical description of what is needed and why, written for the upstream repo's maintainers." + } + ] + }, + "comment": "A professional comment explaining the blocking dependencies. Link to existing blockers and describe what new issues need to be created upstream. Be specific about why each dependency must be resolved before this issue can proceed." } ``` diff --git a/internal/scaffold/fullsend-repo/schemas/triage-result.schema.json b/internal/scaffold/fullsend-repo/schemas/triage-result.schema.json index a80948d30..73616cab7 100644 --- a/internal/scaffold/fullsend-repo/schemas/triage-result.schema.json +++ b/internal/scaffold/fullsend-repo/schemas/triage-result.schema.json @@ -9,7 +9,7 @@ "properties": { "action": { "type": "string", - "enum": ["insufficient", "duplicate", "sufficient", "blocked", "question"] + "enum": ["insufficient", "duplicate", "sufficient", "prerequisites", "question"] }, "reasoning": { "type": "string", @@ -30,10 +30,48 @@ "triage_summary": { "$ref": "#/$defs/triage_summary" }, - "blocked_by": { - "type": "string", - "pattern": "^https://github\\.com/[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+/(issues|pull)/[0-9]+$", - "description": "HTML URL of the blocking issue or PR (e.g., https://github.com/org/repo/issues/99 or https://github.com/org/repo/pull/55)" + "prerequisites": { + "type": "object", + "required": ["existing", "create"], + "properties": { + "existing": { + "type": "array", + "items": { + "type": "object", + "required": ["url"], + "properties": { + "url": { + "type": "string", + "pattern": "^https://github\\.com/[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+/(issues|pull)/[0-9]+$" + } + }, + "additionalProperties": false + } + }, + "create": { + "type": "array", + "items": { + "type": "object", + "required": ["repo", "title", "body"], + "properties": { + "repo": { + "type": "string", + "pattern": "^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$" + }, + "title": { + "type": "string", + "minLength": 1 + }, + "body": { + "type": "string", + "minLength": 1 + } + }, + "additionalProperties": false + } + } + }, + "additionalProperties": false }, "label_actions": { "$ref": "#/$defs/label_actions" @@ -53,8 +91,18 @@ "then": { "required": ["clarity_scores", "triage_summary"] } }, { - "if": { "properties": { "action": { "const": "blocked" } }, "required": ["action"] }, - "then": { "required": ["blocked_by"] } + "if": { "properties": { "action": { "const": "prerequisites" } }, "required": ["action"] }, + "then": { + "required": ["prerequisites"], + "properties": { + "prerequisites": { + "anyOf": [ + { "properties": { "existing": { "minItems": 1 } } }, + { "properties": { "create": { "minItems": 1 } } } + ] + } + } + } } ], "$defs": { diff --git a/internal/scaffold/fullsend-repo/scripts/lib/github-api-csma.sh b/internal/scaffold/fullsend-repo/scripts/lib/github-api-csma.sh index a281397e2..f3870ad1a 100644 --- a/internal/scaffold/fullsend-repo/scripts/lib/github-api-csma.sh +++ b/internal/scaffold/fullsend-repo/scripts/lib/github-api-csma.sh @@ -14,6 +14,7 @@ # GITHUB_CSMA_MIN_REMAINING_GRAPHQL — default 100 # GITHUB_CSMA_SLOT_MIN_MS — default 250 # GITHUB_CSMA_SLOT_MAX_MS — default 750 (0 disables jitter) +# GITHUB_CSMA_SPREAD_MAX_SEC — default 60 (post-reset desync spread) # GITHUB_CSMA_BACKOFF_CAP_SEC — default 120 # shellcheck shell=bash @@ -41,10 +42,26 @@ _github_csma_slot_max_ms() { echo "${GITHUB_CSMA_SLOT_MAX_MS:-750}" } +_github_csma_spread_max_sec() { + echo "${GITHUB_CSMA_SPREAD_MAX_SEC:-60}" +} + _github_csma_backoff_cap_sec() { echo "${GITHUB_CSMA_BACKOFF_CAP_SEC:-120}" } +# Add a random spread delay after a rate-limit sleep to desynchronize runners. +# Called from both github_csma_sense and _github_csma_sleep_after_rate_limit. +_github_csma_post_reset_spread() { + local spread_max + spread_max=$(_github_csma_spread_max_sec) + if (( spread_max > 0 )); then + local spread_secs=$(( RANDOM % spread_max )) + echo "Rate limit reset — spreading ${spread_secs}s to desync from other runners..." >&2 + sleep "${spread_secs}" + fi +} + _github_csma_emit_failure() { printf '%s\n' "$1" >&2 } @@ -85,6 +102,10 @@ github_csma_sense() { echo "Rate limit sense: ${resource} remaining=${remaining} (min=${min_remaining}); waiting ${wait_secs}s until reset..." >&2 sleep "${wait_secs}" + + # After a rate-limit sleep, all runners wake at the same reset timestamp. + # Spread them over a wide window to avoid a thundering herd. + _github_csma_post_reset_spread } # Random inter-call delay (slot time) to reduce synchronized collisions. @@ -161,6 +182,9 @@ _github_csma_sleep_after_rate_limit() { fi echo "GitHub API rate limit (attempt $(( attempt + 1 ))); backing off ${delay}s..." >&2 sleep "${delay}" + + # After backing off, spread runners to avoid thundering herd on wake. + _github_csma_post_reset_spread } # Run gh with CSMA/CD. First argument: rate_limit resource (core|graphql). diff --git a/internal/scaffold/fullsend-repo/scripts/post-triage-test.sh b/internal/scaffold/fullsend-repo/scripts/post-triage-test.sh index c8b4eb29e..1cf26237e 100755 --- a/internal/scaffold/fullsend-repo/scripts/post-triage-test.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-triage-test.sh @@ -27,6 +27,12 @@ if [[ "\$1" == "api" ]] && [[ "\$2" == *"/labels" ]] && [[ "\$*" == *"--paginate printf '%s\n' "area/api" "area/cli" "priority/high" "component/parser" exit 0 fi +# For issue create, return a fake URL on stdout so callers can capture it. +if [[ "\$1" == "issue" ]] && [[ "\$2" == "create" ]]; then + echo "gh \$*" >> "${GH_LOG}" + echo "https://github.com/mock-org/mock-repo/issues/999" + exit 0 +fi echo "gh \$*" >> "${GH_LOG}" MOCKEOF chmod +x "${MOCK_BIN}/gh" @@ -53,6 +59,22 @@ export PATH="${MOCK_BIN}:${PATH}" export GITHUB_ISSUE_URL="https://github.com/test-org/test-repo/issues/42" export GH_TOKEN="fake-token" +# prerequisites handler reads config.yaml from GITHUB_WORKSPACE. +# Create a minimal workspace with an allowlist so the test can exercise +# both the allowed and disallowed paths. +WORKSPACE="${TMPDIR}/workspace" +mkdir -p "${WORKSPACE}" +cat > "${WORKSPACE}/config.yaml" </dev/null; then + echo "::warning::yq not found — cannot read create_issues.allow_targets from config; cross-repo issue creation disabled" + fi + if [[ -f "${CONFIG_FILE}" ]] && command -v yq &>/dev/null; then + ALLOWED_ORGS=$(yq -r '.create_issues.allow_targets.orgs // [] | .[]' "${CONFIG_FILE}" 2>/dev/null || true) + ALLOWED_REPOS=$(yq -r '.create_issues.allow_targets.repos // [] | .[]' "${CONFIG_FILE}" 2>/dev/null || true) fi - echo "Blocked by: ${BLOCKED_BY}" + + # The source repo is always implicitly allowed. + is_target_allowed() { + local target_repo="$1" + local target_org="${target_repo%%/*}" + + # Source repo is always allowed. + if [[ "${target_repo}" == "${REPO}" ]]; then + return 0 + fi + + # Check org allowlist. + if [[ -n "${ALLOWED_ORGS}" ]] && echo "${ALLOWED_ORGS}" | grep -qFx "${target_org}"; then + return 0 + fi + + # Check repo allowlist. + if [[ -n "${ALLOWED_REPOS}" ]] && echo "${ALLOWED_REPOS}" | grep -qFx "${target_repo}"; then + return 0 + fi + + return 1 + } + + # Process create entries: create issues, collect URLs. + CREATE_COUNT=$(jq '.prerequisites.create // [] | length' "${RESULT_FILE}") + CREATED_URLS="" + FAILED_CREATES="" + + for i in $(seq 0 $((CREATE_COUNT - 1))); do + TARGET_REPO=$(jq -r ".prerequisites.create[${i}].repo" "${RESULT_FILE}") + ISSUE_TITLE=$(jq -r ".prerequisites.create[${i}].title" "${RESULT_FILE}") + ISSUE_BODY=$(jq -r ".prerequisites.create[${i}].body" "${RESULT_FILE}") + + if ! is_target_allowed "${TARGET_REPO}"; then + echo "::warning::Skipping issue creation in '${TARGET_REPO}' — not in create_issues.allow_targets" + FAILED_CREATES="${FAILED_CREATES} +
+Prerequisite: ${TARGET_REPO} — ${ISSUE_TITLE} + +${ISSUE_BODY} + +
" + continue + fi + + echo "Creating prerequisite issue in ${TARGET_REPO}..." + CREATED_URL=$(gh issue create --repo "${TARGET_REPO}" --title "${ISSUE_TITLE}" --body "${ISSUE_BODY}" 2>&1) || { + echo "::warning::Failed to create issue in '${TARGET_REPO}': ${CREATED_URL}" + FAILED_CREATES="${FAILED_CREATES} +
+Prerequisite: ${TARGET_REPO} — ${ISSUE_TITLE} + +${ISSUE_BODY} + +
" + continue + } + echo "Created: ${CREATED_URL}" + CREATED_URLS="${CREATED_URLS} ${CREATED_URL}" + done + + # Collect existing URLs. + EXISTING_COUNT=$(jq '.prerequisites.existing // [] | length' "${RESULT_FILE}") + EXISTING_URLS="" + for i in $(seq 0 $((EXISTING_COUNT - 1))); do + URL=$(jq -r ".prerequisites.existing[${i}].url" "${RESULT_FILE}") + EXISTING_URLS="${EXISTING_URLS} ${URL}" + done + + # Merge all blocker URLs for the comment. + ALL_URLS="${EXISTING_URLS} ${CREATED_URLS}" + ALL_URLS=$(echo "${ALL_URLS}" | xargs) # trim whitespace + + if [[ -n "${ALL_URLS}" ]]; then + BLOCKER_LIST="" + for url in ${ALL_URLS}; do + BLOCKER_LIST="${BLOCKER_LIST} +- ${url}" + done + COMMENT="${COMMENT} + +**Blocked by:**${BLOCKER_LIST}" + fi + + if [[ -n "${FAILED_CREATES}" ]]; then + COMMENT="${COMMENT} + +**Could not create automatically** (file manually or update \`create_issues.allow_targets\` in config.yaml): +${FAILED_CREATES}" + fi + remove_label "ready-to-code" remove_label "needs-info" add_label "blocked" diff --git a/internal/scaffold/fullsend-repo/scripts/validate-output-schema-test.sh b/internal/scaffold/fullsend-repo/scripts/validate-output-schema-test.sh index 6c43fe044..44bd813ac 100755 --- a/internal/scaffold/fullsend-repo/scripts/validate-output-schema-test.sh +++ b/internal/scaffold/fullsend-repo/scripts/validate-output-schema-test.sh @@ -70,12 +70,12 @@ run_test "valid-question" \ '{"action":"question","reasoning":"this is a support question","comment":"Based on the docs, Python 4 is not supported. Would you like to open a feature request?"}' \ "true" -run_test "valid-blocked-issue" \ - '{"action":"blocked","reasoning":"upstream dependency","blocked_by":"https://github.com/org/repo/issues/99","comment":"Blocked on upstream."}' \ +run_test "valid-prerequisites-existing" \ + '{"action":"prerequisites","reasoning":"upstream dependency","prerequisites":{"existing":[{"url":"https://github.com/org/repo/issues/99"}],"create":[]},"comment":"Blocked on upstream."}' \ "true" -run_test "valid-blocked-pr" \ - '{"action":"blocked","reasoning":"waiting on PR","blocked_by":"https://github.com/org/repo/pull/55","comment":"Blocked on a PR."}' \ +run_test "valid-prerequisites-create" \ + '{"action":"prerequisites","reasoning":"needs upstream issue","prerequisites":{"existing":[],"create":[{"repo":"org/upstream","title":"Add X","body":"Need X."}]},"comment":"Blocked on upstream."}' \ "true" # --- Conditional requirement failures --- @@ -92,12 +92,16 @@ run_test "sufficient-missing-triage-summary" \ '{"action":"sufficient","reasoning":"ok","clarity_scores":{"symptom":0.9,"cause":0.8,"reproduction":0.9,"impact":0.7,"overall":0.85},"comment":"Done."}' \ "false" -run_test "blocked-missing-blocked-by" \ - '{"action":"blocked","reasoning":"upstream dependency","comment":"Blocked."}' \ +run_test "prerequisites-missing-prerequisites-field" \ + '{"action":"prerequisites","reasoning":"upstream dependency","comment":"Blocked."}' \ "false" -run_test "blocked-malformed-url" \ - '{"action":"blocked","reasoning":"upstream dependency","blocked_by":"not-a-url","comment":"Blocked."}' \ +run_test "prerequisites-both-arrays-empty" \ + '{"action":"prerequisites","reasoning":"upstream dependency","prerequisites":{"existing":[],"create":[]},"comment":"Blocked."}' \ + "false" + +run_test "prerequisites-malformed-url-in-existing" \ + '{"action":"prerequisites","reasoning":"upstream dependency","prerequisites":{"existing":[{"url":"not-a-url"}],"create":[]},"comment":"Blocked."}' \ "false" # --- FULLSEND_OUTPUT_FILE override --- @@ -288,7 +292,7 @@ run_test_output "additional-properties-shows-allowed" \ run_test_output "additional-properties-lists-known-keys" \ '{"action":"sufficient","reasoning":"ok","clarity_scores":{"symptom":0.9,"cause":0.8,"reproduction":0.9,"impact":0.7,"overall":0.85},"triage_summary":{"title":"Bug","severity":"high","category":"bug","problem":"crash","root_cause_hypothesis":"null ptr","reproduction_steps":["step 1"],"impact":"all users","recommended_fix":"fix","proposed_test_case":"test"},"comment":"Done.","injected_field":"malicious"}' \ "false" \ - "action, blocked_by, clarity_scores, comment, duplicate_of, label_actions, reasoning, triage_summary" + "action, clarity_scores, comment, duplicate_of, label_actions, prerequisites, reasoning, triage_summary" run_test_output "valid-output-no-allowed-line" \ '{"action":"insufficient","reasoning":"missing repro","clarity_scores":{"symptom":0.6,"cause":0.3,"reproduction":0.1,"impact":0.5,"overall":0.39},"comment":"Can you share repro steps?"}' \ diff --git a/internal/statuscomment/statuscomment.go b/internal/statuscomment/statuscomment.go index fc24655fe..2cef62463 100644 --- a/internal/statuscomment/statuscomment.go +++ b/internal/statuscomment/statuscomment.go @@ -38,15 +38,20 @@ const ( // now is overridable in tests to fix the current time for ReconcileOrphaned. var now = time.Now +// ClientFactory returns a fresh forge.Client. It is called before each +// API operation so the underlying token is never stale. +type ClientFactory func(ctx context.Context) (forge.Client, error) + // Notifier manages status comment lifecycle for a single agent run. type Notifier struct { - client forge.Client - cfg config.StatusNotificationConfig - owner, repo string - number int - runURL string - sha string - marker string + client forge.Client + clientFactory ClientFactory + cfg config.StatusNotificationConfig + owner, repo string + number int + runURL string + sha string + marker string startCommentID int startTime time.Time @@ -79,6 +84,32 @@ func (n *Notifier) SetWarnFunc(f func(string, ...any)) { n.warnf = f } +// SetClientFactory sets a factory that mints a fresh forge.Client before +// each API operation. When set, the static client passed to New is only +// used if the factory is nil. +func (n *Notifier) SetClientFactory(f ClientFactory) { + n.clientFactory = f +} + +// HasClientFactory reports whether a client factory has been configured. +func (n *Notifier) HasClientFactory() bool { + return n.clientFactory != nil +} + +// refreshClient replaces n.client with a freshly minted client when a +// factory is configured. Returns an error only if the factory itself fails. +func (n *Notifier) refreshClient(ctx context.Context) error { + if n.clientFactory == nil { + return nil + } + c, err := n.clientFactory(ctx) + if err != nil { + return fmt.Errorf("minting fresh client: %w", err) + } + n.client = c + return nil +} + func commentEnabled(val string) bool { return val == "" || val == "enabled" } @@ -88,6 +119,9 @@ func (n *Notifier) PostStart(ctx context.Context, description string) error { n.startTime = n.now().UTC() if commentEnabled(n.cfg.Comment.Start) { + if err := n.refreshClient(ctx); err != nil { + return err + } body := n.buildStartBody(description) comment, err := n.client.CreateIssueComment(ctx, n.owner, n.repo, n.number, body) if err != nil { @@ -119,13 +153,19 @@ func (n *Notifier) PostCompletion(ctx context.Context, description, status strin // Completion comments disabled — clean up the start comment so it // doesn't remain orphaned in its "Started" state. if n.startCommentID != 0 { - if err := n.client.DeleteIssueComment(ctx, n.owner, n.repo, n.startCommentID); err != nil { + if err := n.refreshClient(ctx); err != nil { + n.warnf("failed to mint token for start comment cleanup: %v", err) + } else if err := n.client.DeleteIssueComment(ctx, n.owner, n.repo, n.startCommentID); err != nil { n.warnf("failed to delete start comment when completion disabled: %v", err) } } return nil } + if err := n.refreshClient(ctx); err != nil { + return err + } + body := n.buildCompletionBody(description, status, completionTime) if n.startCommentID != 0 { diff --git a/internal/statuscomment/statuscomment_test.go b/internal/statuscomment/statuscomment_test.go index 26e349a40..c68e9b895 100644 --- a/internal/statuscomment/statuscomment_test.go +++ b/internal/statuscomment/statuscomment_test.go @@ -869,3 +869,215 @@ func TestReconcileOrphaned_UnknownReasonDefaultsToTerminated(t *testing.T) { assert.Contains(t, body, "Started 6:43 AM UTC") assert.Contains(t, body, "Ended 2:47 PM UTC") } + +func TestClientFactory_CalledBeforePostStart(t *testing.T) { + fc1 := forge.NewFakeClient() + fc2 := forge.NewFakeClient() + fc2.AuthenticatedUser = "mint-bot[bot]" + cfg := config.StatusNotificationConfig{} + + n := New(fc1, cfg, "org", "repo", 7, "https://ci/run/42", "a1b2c3d", "run-42") + n.now = fixedTime + + factoryCalled := false + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + factoryCalled = true + return fc2, nil + }) + + err := n.PostStart(context.Background(), "Working") + require.NoError(t, err) + assert.True(t, factoryCalled, "factory should be called before PostStart API calls") + assert.Len(t, fc2.IssueComments["org/repo/7"], 1, "comment should be on factory-returned client") + assert.Empty(t, fc1.IssueComments, "original client should not be used") +} + +func TestClientFactory_CalledBeforePostCompletion(t *testing.T) { + fc := forge.NewFakeClient() + fc.AuthenticatedUser = "bot[bot]" + cfg := config.StatusNotificationConfig{ + Comment: config.CommentNotificationConfig{Start: "enabled", Completion: "enabled"}, + } + + n := newTestNotifier(fc, cfg) + err := n.PostStart(context.Background(), "Working") + require.NoError(t, err) + + fc2 := forge.NewFakeClient() + fc2.AuthenticatedUser = "bot[bot]" + // Pre-populate fc2 with the same comments so analyzeTimeline works. + fc2.IssueComments = map[string][]forge.IssueComment{ + "org/repo/7": {fc.IssueComments["org/repo/7"][0]}, + } + + completionFactoryCalled := false + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + completionFactoryCalled = true + return fc2, nil + }) + + n.now = func() time.Time { return fixedTime().Add(5 * time.Minute) } + err = n.PostCompletion(context.Background(), "Working", "success") + require.NoError(t, err) + assert.True(t, completionFactoryCalled, "factory should be called before PostCompletion API calls") +} + +func TestClientFactory_ErrorPropagated(t *testing.T) { + fc := forge.NewFakeClient() + cfg := config.StatusNotificationConfig{} + n := New(fc, cfg, "org", "repo", 7, "", "", "run-42") + n.now = fixedTime + + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + return nil, fmt.Errorf("mint service unavailable") + }) + + err := n.PostStart(context.Background(), "Working") + require.Error(t, err) + assert.Contains(t, err.Error(), "mint service unavailable") +} + +func TestClientFactory_NilUsesStaticClient(t *testing.T) { + fc := forge.NewFakeClient() + cfg := config.StatusNotificationConfig{} + n := newTestNotifier(fc, cfg) + + err := n.PostStart(context.Background(), "Working") + require.NoError(t, err) + assert.Len(t, fc.IssueComments["org/repo/7"], 1, "static client should be used when no factory set") +} + +func TestClientFactory_ErrorOnPostCompletion(t *testing.T) { + fc := forge.NewFakeClient() + cfg := config.StatusNotificationConfig{ + Comment: config.CommentNotificationConfig{Start: "enabled", Completion: "enabled"}, + } + n := newTestNotifier(fc, cfg) + + err := n.PostStart(context.Background(), "Working") + require.NoError(t, err) + + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + return nil, fmt.Errorf("token expired") + }) + + n.now = func() time.Time { return fixedTime().Add(5 * time.Minute) } + err = n.PostCompletion(context.Background(), "Working", "success") + require.Error(t, err) + assert.Contains(t, err.Error(), "token expired") +} + +func TestClientFactory_CompletionDisabled_DeletePath(t *testing.T) { + fc := forge.NewFakeClient() + cfg := config.StatusNotificationConfig{ + Comment: config.CommentNotificationConfig{Start: "enabled", Completion: "disabled"}, + } + n := newTestNotifier(fc, cfg) + + err := n.PostStart(context.Background(), "Working") + require.NoError(t, err) + require.Equal(t, 1, n.startCommentID) + + fc2 := forge.NewFakeClient() + fc2.AuthenticatedUser = "fullsend-bot[bot]" + fc2.IssueComments = map[string][]forge.IssueComment{ + "org/repo/7": {fc.IssueComments["org/repo/7"][0]}, + } + + factoryCalled := false + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + factoryCalled = true + return fc2, nil + }) + + n.now = func() time.Time { return fixedTime().Add(time.Minute) } + err = n.PostCompletion(context.Background(), "Working", "success") + require.NoError(t, err) + assert.True(t, factoryCalled, "factory should be called even when completion disabled (for delete)") + require.Len(t, fc2.DeletedComments, 1) + assert.Equal(t, 1, fc2.DeletedComments[0]) +} + +func TestClientFactory_BothDisabled_NoMint(t *testing.T) { + fc := forge.NewFakeClient() + cfg := config.StatusNotificationConfig{ + Comment: config.CommentNotificationConfig{Start: "disabled", Completion: "disabled"}, + } + n := newTestNotifier(fc, cfg) + + factoryCalled := false + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + factoryCalled = true + return nil, fmt.Errorf("should not be called") + }) + + err := n.PostCompletion(context.Background(), "Working", "success") + require.NoError(t, err, "should not error when no API call is needed") + assert.False(t, factoryCalled, "factory should not be called when both disabled and no start comment") +} + +func TestHasClientFactory(t *testing.T) { + fc := forge.NewFakeClient() + cfg := config.StatusNotificationConfig{} + n := newTestNotifier(fc, cfg) + + assert.False(t, n.HasClientFactory(), "should be false when no factory set") + + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + return fc, nil + }) + assert.True(t, n.HasClientFactory(), "should be true after SetClientFactory") +} + +func TestClientFactory_CompletionDisabled_MintError(t *testing.T) { + fc := forge.NewFakeClient() + cfg := config.StatusNotificationConfig{ + Comment: config.CommentNotificationConfig{Start: "enabled", Completion: "disabled"}, + } + n := newTestNotifier(fc, cfg) + + err := n.PostStart(context.Background(), "Working") + require.NoError(t, err) + require.NotZero(t, n.startCommentID) + + var warnings []string + n.SetWarnFunc(func(format string, args ...any) { + warnings = append(warnings, fmt.Sprintf(format, args...)) + }) + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + return nil, fmt.Errorf("mint service down") + }) + + err = n.PostCompletion(context.Background(), "Working", "success") + require.NoError(t, err, "should not return error — fail-open on cleanup") + require.Len(t, warnings, 1) + assert.Contains(t, warnings[0], "mint service down") +} + +func TestClientFactory_CompletionDisabled_DeleteError(t *testing.T) { + fc := forge.NewFakeClient() + cfg := config.StatusNotificationConfig{ + Comment: config.CommentNotificationConfig{Start: "enabled", Completion: "disabled"}, + } + n := newTestNotifier(fc, cfg) + + err := n.PostStart(context.Background(), "Working") + require.NoError(t, err) + require.NotZero(t, n.startCommentID) + + fc2 := forge.NewFakeClient() + fc2.Errors["DeleteIssueComment"] = fmt.Errorf("forbidden") + + var warnings []string + n.SetWarnFunc(func(format string, args ...any) { + warnings = append(warnings, fmt.Sprintf(format, args...)) + }) + n.SetClientFactory(func(ctx context.Context) (forge.Client, error) { + return fc2, nil + }) + + err = n.PostCompletion(context.Background(), "Working", "success") + require.NoError(t, err, "should not return error — fail-open on cleanup") + require.Len(t, warnings, 1) + assert.Contains(t, warnings[0], "forbidden") +} diff --git a/qf-tests/GH-2354/README.md b/qf-tests/GH-2354/README.md new file mode 100644 index 000000000..ad22d6d2b --- /dev/null +++ b/qf-tests/GH-2354/README.md @@ -0,0 +1,7 @@ +# QualityFlow Tests — GH-2354 + +Generated by the QualityFlow pipeline. + +| Directory | Count | Framework | +|-----------|-------|-----------| +| `go/` | 8 files | Go | diff --git a/qf-tests/GH-2354/go/enrollment_backoff_test.go b/qf-tests/GH-2354/go/enrollment_backoff_test.go new file mode 100644 index 000000000..41dbb7776 --- /dev/null +++ b/qf-tests/GH-2354/go/enrollment_backoff_test.go @@ -0,0 +1,104 @@ +//go:build e2e + +package layers + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +/* +Enrollment Exponential Backoff Tests + +STP Reference: outputs/stp/GH-2354/GH-2354_test_plan.md +Jira: GH-2354 + +These tests validate that enrollment polling uses exponential backoff to +avoid excessive API calls. The nextInterval helper is a pure function that +can be tested directly without mocking, providing reliable coverage of the +backoff logic. Integration-level backoff behaviour is covered by the timeout +bound tests which exercise the full polling loop. +*/ + +// TestEnrollmentExponentialBackoff validates the exponential backoff behaviour +// of enrollment polling intervals. +func TestEnrollmentExponentialBackoff(t *testing.T) { + t.Run("should increase wait time between status updates progressively", func(t *testing.T) { + // [test_id:TS-GH-2354-004] + // Verify that nextInterval doubles the polling interval on each call. + intervals := make([]time.Duration, 0, 5) + current := enrollmentPollInitial + intervals = append(intervals, current) + + // Simulate 4 backoff iterations + for i := 0; i < 4; i++ { + current = nextInterval(current) + intervals = append(intervals, current) + } + + // Assert: intervals increase monotonically (until cap is reached). + for i := 1; i < len(intervals); i++ { + assert.GreaterOrEqual(t, intervals[i], intervals[i-1], + "interval[%d] (%v) should be >= interval[%d] (%v)", + i, intervals[i], i-1, intervals[i-1]) + } + + // Assert: second interval is 2x the first (before cap). + assert.Equal(t, 2*enrollmentPollInitial, intervals[1], + "second interval should be 2x initial") + }) + + t.Run("should not exceed maximum poll interval", func(t *testing.T) { + // [test_id:TS-GH-2354-005] + // Verify that no interval exceeds enrollmentPollMax regardless of + // how many iterations occur. + current := enrollmentPollInitial + + // Run enough iterations to well exceed the cap + for i := 0; i < 20; i++ { + current = nextInterval(current) + assert.LessOrEqual(t, current, enrollmentPollMax, + "interval after %d doublings should not exceed enrollmentPollMax (%v), got %v", + i+1, enrollmentPollMax, current) + } + + // After many iterations, interval should be exactly at the cap. + assert.Equal(t, enrollmentPollMax, current, + "interval should stabilise at enrollmentPollMax") + }) + + t.Run("should execute first retry within expected timeframe", func(t *testing.T) { + // [test_id:TS-GH-2354-006] + // Verify that the initial polling interval is enrollmentPollInitial (2s), + // ensuring the first poll occurs promptly after dispatch. + assert.Equal(t, 2*time.Second, enrollmentPollInitial, + "initial poll interval should be 2 seconds") + + // The first interval used in awaitWorkflowRun is enrollmentPollInitial. + // After one nextInterval call, it should double. + firstRetry := enrollmentPollInitial + assert.LessOrEqual(t, firstRetry, 2*time.Second+500*time.Millisecond, + "first retry should occur within enrollmentPollInitial + 500ms tolerance") + }) +} + +// TestBackoffSequence validates the complete backoff sequence from initial +// to cap, ensuring the progression is correct. +func TestBackoffSequence(t *testing.T) { + expected := []time.Duration{ + 4 * time.Second, // 2s * 2 + 8 * time.Second, // 4s * 2 + 15 * time.Second, // 16s capped at 15s + 15 * time.Second, // stays at cap + 15 * time.Second, // stays at cap + } + + current := enrollmentPollInitial + for i, want := range expected { + current = nextInterval(current) + assert.Equal(t, want, current, + "iteration %d: expected %v, got %v", i+1, want, current) + } +} diff --git a/qf-tests/GH-2354/go/enrollment_dispatch_failure_test.go b/qf-tests/GH-2354/go/enrollment_dispatch_failure_test.go new file mode 100644 index 000000000..350c9cc2d --- /dev/null +++ b/qf-tests/GH-2354/go/enrollment_dispatch_failure_test.go @@ -0,0 +1,103 @@ +//go:build e2e + +package layers + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +/* +Enrollment Dispatch Failure Tests + +STP Reference: outputs/stp/GH-2354/GH-2354_test_plan.md +Jira: GH-2354 + +These tests validate that enrollment workflow dispatch failures are reported +clearly, do not block install, and are safe in concurrent contexts. +When DispatchWorkflow fails, Install returns the error immediately without +entering the polling loop. +*/ + +// TestEnrollmentDispatchFailure validates dispatch error handling. +func TestEnrollmentDispatchFailure(t *testing.T) { + dispatchErrMsg := "workflow file not found: .github/workflows/repo-maintenance.yml" + + t.Run("should return descriptive error on dispatch failure", func(t *testing.T) { + // [test_id:TS-GH-2354-019] + // Setup: FakeClient with a specific dispatch error. + client := &forge.FakeClient{ + Errors: map[string]error{ + "DispatchWorkflow": fmt.Errorf("%s", dispatchErrMsg), + }, + } + layer, _ := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + err := layer.Install(context.Background()) + + // Assert: error is returned (dispatch errors are fatal in Install). + require.Error(t, err) + // Assert: error contains the original dispatch error message. + assert.Contains(t, err.Error(), dispatchErrMsg, + "error should contain the original dispatch failure reason") + // Assert: error is wrapped with context about what was being done. + assert.Contains(t, err.Error(), "dispatching repo-maintenance", + "error should explain the operation that failed") + }) + + t.Run("should not block install on dispatch error", func(t *testing.T) { + // [test_id:TS-GH-2354-020] + // Setup: FakeClient with dispatch error. The static FakeClient + // records whether ListWorkflowRuns was called (if WorkflowRuns + // map is accessed). With dispatch error, Install returns immediately + // without calling awaitWorkflowRun. + client := &forge.FakeClient{ + Errors: map[string]error{ + "DispatchWorkflow": fmt.Errorf("%s", dispatchErrMsg), + }, + WorkflowRuns: map[string]*forge.WorkflowRun{}, + } + layer, _ := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + start := time.Now() + err := layer.Install(context.Background()) + elapsed := time.Since(start) + + // Assert: error returned promptly (no polling delay). + require.Error(t, err) + assert.Less(t, elapsed, 5*time.Second, + "dispatch failure should return promptly without entering polling loop") + + // The existing enrollment_test.go TestEnrollmentLayer_Install_DispatchError + // already verifies the error path, but this test adds the timing assertion + // to confirm no blocking occurs. + }) + + t.Run("should handle dispatch error safely in concurrent context", func(t *testing.T) { + // [test_id:TS-GH-2354-021] + // Verify that dispatch errors do not cause panics or data races. + // The FakeClient is thread-safe (uses sync.Mutex internally). + client := &forge.FakeClient{ + Errors: map[string]error{ + "DispatchWorkflow": fmt.Errorf("%s", dispatchErrMsg), + }, + } + layer, _ := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + // Assert: no panic on dispatch error. + require.NotPanics(t, func() { + err := layer.Install(context.Background()) + // Assert: error propagated cleanly. + assert.Error(t, err) + assert.Contains(t, err.Error(), dispatchErrMsg, + "error should propagate cleanly in concurrent context") + }) + }) +} diff --git a/qf-tests/GH-2354/go/enrollment_happy_path_test.go b/qf-tests/GH-2354/go/enrollment_happy_path_test.go new file mode 100644 index 000000000..896111c89 --- /dev/null +++ b/qf-tests/GH-2354/go/enrollment_happy_path_test.go @@ -0,0 +1,100 @@ +//go:build e2e + +package layers + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +/* +Enrollment Happy Path Tests + +STP Reference: outputs/stp/GH-2354/GH-2354_test_plan.md +Jira: GH-2354 + +These tests validate that enrollment install succeeds within expected time +when the workflow registers quickly, and reports success details including +workflow URL and reconciliation PRs. +*/ + +// TestEnrollmentHappyPath validates the happy-path enrollment flow. +func TestEnrollmentHappyPath(t *testing.T) { + // Shared setup: a FakeClient with immediate workflow success and PRs. + now := time.Now().UTC() + workflowURL := "https://github.com/test-org/.fullsend/actions/runs/99" + + makeClient := func() *forge.FakeClient { + return &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{ + "test-org/.fullsend/repo-maintenance.yml": { + ID: 99, + Status: "completed", + Conclusion: "success", + CreatedAt: now.Add(time.Minute).Format(time.RFC3339), + HTMLURL: workflowURL, + }, + }, + PullRequests: map[string][]forge.ChangeProposal{ + "test-org/repo-a": { + { + Title: "chore: connect to fullsend agent pipeline", + URL: "https://github.com/test-org/repo-a/pull/42", + }, + }, + }, + } + } + + t.Run("should complete fast enrollment without delay", func(t *testing.T) { + // [test_id:TS-GH-2354-009] + client := makeClient() + layer, _ := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + start := time.Now() + err := layer.Install(context.Background()) + elapsed := time.Since(start) + + // Assert: no error. + require.NoError(t, err) + // Assert: completes in under 5 seconds (first poll returns success). + assert.Less(t, elapsed, 5*time.Second, + "happy-path enrollment should complete in under 5 seconds, took %v", elapsed) + }) + + t.Run("should report success and workflow URL", func(t *testing.T) { + // [test_id:TS-GH-2354-010] + client := makeClient() + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + err := layer.Install(context.Background()) + require.NoError(t, err) + + output := buf.String() + // Assert: output contains the workflow URL. + assert.Contains(t, output, workflowURL, + "output should contain the workflow run URL") + assert.Contains(t, output, "https://github.com/", + "output should contain a GitHub URL") + }) + + t.Run("should report reconciliation PRs", func(t *testing.T) { + // [test_id:TS-GH-2354-011] + client := makeClient() + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + err := layer.Install(context.Background()) + require.NoError(t, err) + + output := buf.String() + // Assert: output mentions the reconciliation PR. + assert.Contains(t, output, "repo-a/pull/42", + "output should contain the reconciliation PR URL") + }) +} diff --git a/qf-tests/GH-2354/go/enrollment_progress_feedback_test.go b/qf-tests/GH-2354/go/enrollment_progress_feedback_test.go new file mode 100644 index 000000000..1f1f6a870 --- /dev/null +++ b/qf-tests/GH-2354/go/enrollment_progress_feedback_test.go @@ -0,0 +1,102 @@ +//go:build e2e + +package layers + +import ( + "context" + "regexp" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +/* +Enrollment Progress Feedback Tests + +STP Reference: outputs/stp/GH-2354/GH-2354_test_plan.md +Jira: GH-2354 + +These tests validate that enrollment provides progress feedback during each +polling phase, including elapsed time information. Progress messages are +emitted by awaitWorkflowRun when it finds an in_progress run or when +ListWorkflowRuns returns an error. We use FakeClient with an in_progress run +to trigger progress output, bounded by a short context timeout. +*/ + +// TestEnrollmentProgressFeedback validates progress output during polling. +func TestEnrollmentProgressFeedback(t *testing.T) { + t.Run("should emit progress messages during polling", func(t *testing.T) { + // [test_id:TS-GH-2354-007] + // Setup: FakeClient with an in_progress workflow run. The polling loop + // will find this run, see it's not completed, and log a progress message + // with its status and elapsed time before continuing to poll. + now := time.Now().UTC() + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{ + "test-org/.fullsend/repo-maintenance.yml": { + ID: 10, + Status: "in_progress", + Conclusion: "", + CreatedAt: now.Add(time.Minute).Format(time.RFC3339), + HTMLURL: "https://github.com/test-org/.fullsend/actions/runs/10", + }, + }, + } + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + // Use a short context to allow a few poll iterations before cancellation. + ctx, cancel := context.WithTimeout(context.Background(), 6*time.Second) + defer cancel() + + err := layer.Install(ctx) + // Install treats awaitWorkflowRun errors as non-fatal. + require.NoError(t, err) + + output := buf.String() + // Assert: progress messages are present. The code emits + // "workflow run (, elapsed)" for in-progress runs. + assert.Greater(t, len(output), 0, + "printer buffer should contain output") + assert.Contains(t, output, "in_progress", + "progress output should mention workflow status") + }) + + t.Run("should report elapsed time in status updates", func(t *testing.T) { + // [test_id:TS-GH-2354-008] + // Same setup as above: in_progress run triggers progress messages + // that include elapsed time. + now := time.Now().UTC() + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{ + "test-org/.fullsend/repo-maintenance.yml": { + ID: 10, + Status: "in_progress", + Conclusion: "", + CreatedAt: now.Add(time.Minute).Format(time.RFC3339), + HTMLURL: "https://github.com/test-org/.fullsend/actions/runs/10", + }, + }, + } + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + ctx, cancel := context.WithTimeout(context.Background(), 6*time.Second) + defer cancel() + + err := layer.Install(ctx) + require.NoError(t, err) + + output := buf.String() + // Assert: output contains elapsed time. The code formats elapsed as + // time.Duration.Round(time.Second), which produces strings like "2s", "4s". + // The "elapsed" keyword is also present in the format string. + assert.Contains(t, output, "elapsed", + "progress output should mention elapsed time") + matched, _ := regexp.MatchString(`\d+s`, output) + assert.True(t, matched, + "progress output should contain a duration value like '2s' or '4s', got: %s", output) + }) +} diff --git a/qf-tests/GH-2354/go/enrollment_timeout_bound_test.go b/qf-tests/GH-2354/go/enrollment_timeout_bound_test.go new file mode 100644 index 000000000..85e9fa982 --- /dev/null +++ b/qf-tests/GH-2354/go/enrollment_timeout_bound_test.go @@ -0,0 +1,188 @@ +//go:build e2e + +package layers + +import ( + "context" + "regexp" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +/* +Enrollment Timeout Bound Tests + +STP Reference: outputs/stp/GH-2354/GH-2354_test_plan.md +Jira: GH-2354 + +These tests validate that enrollment install completes or fails within a +bounded, predictable timeout (enrollmentWaitTimeout = 3 min). To keep tests +fast, we rely on FakeClient's static responses: an immediate-success client +returns a completed run on every poll (verifying the happy-path bound), and +an empty-client causes awaitWorkflowRun to poll until it hits the timeout +deadline (verifying the timeout path). The actual timeout is 3 minutes in +production, but the test assertions focus on behavioural correctness rather +than waiting the full duration. +*/ + +// TestEnrollmentTimeoutBound validates that enrollment install completes or +// fails within a bounded, predictable timeout. +func TestEnrollmentTimeoutBound(t *testing.T) { + t.Run("should complete within timeout bound", func(t *testing.T) { + // [test_id:TS-GH-2354-001] + // Setup: FakeClient with immediate workflow success. + now := time.Now().UTC() + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{ + "test-org/.fullsend/repo-maintenance.yml": { + ID: 1, + Status: "completed", + Conclusion: "success", + CreatedAt: now.Add(time.Minute).Format(time.RFC3339), + HTMLURL: "https://github.com/test-org/.fullsend/actions/runs/1", + }, + }, + } + layer, _ := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + start := time.Now() + err := layer.Install(context.Background()) + elapsed := time.Since(start) + + // Assert: no error returned + require.NoError(t, err) + // Assert: elapsed time is well under enrollmentWaitTimeout (3 min). + // With immediate success, the test should complete in seconds. + assert.Less(t, elapsed, enrollmentWaitTimeout, + "enrollment should complete within the timeout bound") + }) + + t.Run("should return actionable error on timeout", func(t *testing.T) { + // [test_id:TS-GH-2354-002] + // Setup: FakeClient with no workflow runs configured → awaitWorkflowRun + // will poll empty results until deadline, then return a timeout error. + // Install() treats this as non-fatal, so we check the output buffer + // for the warning message instead. + client := &forge.FakeClient{ + // Empty WorkflowRuns: ListWorkflowRuns returns nil for the key, + // causing awaitWorkflowRun to keep polling until timeout. + WorkflowRuns: map[string]*forge.WorkflowRun{}, + } + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + // Use a short-lived context to bound the test duration, since the + // real enrollmentWaitTimeout is 3 minutes. + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + err := layer.Install(ctx) + // Install returns nil (non-fatal) but logs the timeout/cancel warning. + require.NoError(t, err) + + output := buf.String() + // The warning from Install includes the awaitWorkflowRun error message. + assert.Contains(t, output, "could not confirm enrollment", + "should warn about enrollment confirmation failure") + }) + + t.Run("should handle slow workflow registration", func(t *testing.T) { + // [test_id:TS-GH-2354-003] + // Setup: FakeClient returns empty runs initially. Since FakeClient is + // static, we simulate "delayed registration" by having NO workflow run + // in the map initially, then using a context timeout to bound the test. + // The key insight is that when ListWorkflowRuns returns nil (no runs), + // awaitWorkflowRun continues polling without failing prematurely. + // + // For this test we verify that the code does NOT fail on empty results + // by configuring a client that eventually has a matching run. Since + // FakeClient is static, we pre-populate the run and verify the code + // finds it despite the polling loop. + now := time.Now().UTC() + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{ + "test-org/.fullsend/repo-maintenance.yml": { + ID: 1, + Status: "completed", + Conclusion: "success", + CreatedAt: now.Add(time.Minute).Format(time.RFC3339), + HTMLURL: "https://github.com/test-org/.fullsend/actions/runs/1", + }, + }, + } + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + err := layer.Install(context.Background()) + + // Assert: enrollment succeeds. + require.NoError(t, err) + output := buf.String() + assert.Contains(t, output, "enrollment completed successfully", + "should succeed when workflow registers") + // Verify no premature failure message. + assert.NotContains(t, output, "could not confirm enrollment", + "should not produce a timeout warning when workflow registers") + }) +} + +// TestNextInterval validates the exponential backoff helper directly. +// This complements the integration-level timeout bound tests. +func TestNextInterval_DoublesAndCaps(t *testing.T) { + // Verify the backoff doubles until it hits the cap. + interval := enrollmentPollInitial // 2s + assert.Equal(t, 2*time.Second, interval) + + interval = nextInterval(interval) // 4s + assert.Equal(t, 4*time.Second, interval) + + interval = nextInterval(interval) // 8s + assert.Equal(t, 8*time.Second, interval) + + interval = nextInterval(interval) // 16s → capped at 15s + assert.Equal(t, enrollmentPollMax, interval) + + interval = nextInterval(interval) // stays at 15s + assert.Equal(t, enrollmentPollMax, interval) +} + +// TestAwaitWorkflowRunTimeoutMessage verifies the timeout error message +// contains actionable guidance and elapsed time. This uses a minimal +// context timeout to avoid waiting the full 3-minute enrollmentWaitTimeout. +func TestAwaitWorkflowRunTimeoutMessage(t *testing.T) { + // Use a very short context timeout so we hit the context cancellation + // path, then verify the Install output contains the warning. + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{}, + } + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + + err := layer.Install(ctx) + require.NoError(t, err) // Install swallows awaitWorkflowRun errors + + output := buf.String() + // Verify the warning was logged + assert.Contains(t, output, "could not confirm enrollment") + + // The error message from awaitWorkflowRun should contain either: + // - "timed out after Xs" if deadline was hit, or + // - "context" if context was cancelled + // In both cases, Install logs it as a warning. + hasTimedOut := strings.Contains(output, "timed out") + hasContext := strings.Contains(output, "context") + assert.True(t, hasTimedOut || hasContext, + "warning should mention timeout or context cancellation, got: %s", output) + + // If timed out (not context cancelled), verify elapsed time is included + if hasTimedOut { + matched, _ := regexp.MatchString(`\d+s`, output) + assert.True(t, matched, "timeout message should include elapsed time duration") + } +} diff --git a/qf-tests/GH-2354/go/enrollment_timeout_error_quality_test.go b/qf-tests/GH-2354/go/enrollment_timeout_error_quality_test.go new file mode 100644 index 000000000..2eeb92efc --- /dev/null +++ b/qf-tests/GH-2354/go/enrollment_timeout_error_quality_test.go @@ -0,0 +1,112 @@ +//go:build e2e + +package layers + +import ( + "context" + "regexp" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +/* +Enrollment Timeout Error Quality Tests + +STP Reference: outputs/stp/GH-2354/GH-2354_test_plan.md +Jira: GH-2354 + +These tests validate that enrollment timeout errors produce actionable +guidance for manual recovery, including specific check instructions and +elapsed time duration. Since Install() swallows awaitWorkflowRun errors as +non-fatal warnings, we verify the error quality by inspecting the printer +output buffer. + +The awaitWorkflowRun timeout error message is: + "timed out after Xs waiting for repo-maintenance workflow; + check the workflow in .fullsend and re-run install if needed" +*/ + +// TestEnrollmentTimeoutErrorQuality validates the quality of timeout error messages. +func TestEnrollmentTimeoutErrorQuality(t *testing.T) { + t.Run("should include manual check guidance in timeout error", func(t *testing.T) { + // [test_id:TS-GH-2354-012] + // Setup: FakeClient returns no matching workflow runs, so awaitWorkflowRun + // polls until deadline. Use a short context timeout to keep the test fast. + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{}, + } + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + // Short timeout so we hit the deadline quickly. + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + err := layer.Install(ctx) + require.NoError(t, err) // Install treats timeout as non-fatal + + output := buf.String() + // The timeout error message from awaitWorkflowRun contains: + // "check the workflow in .fullsend and re-run install if needed" + // Install logs this as: "could not confirm enrollment: timed out..." + // OR context cancellation triggers context.Canceled. + assert.Contains(t, output, "could not confirm enrollment", + "should warn about enrollment confirmation failure") + + // Verify actionable guidance is present. The error message contains + // "check" and/or "re-run" guidance. + hasCheck := assert.Condition(t, func() bool { + return containsAny(output, "check", "re-run", "context") + }, "timeout warning should contain actionable guidance") + _ = hasCheck + }) + + t.Run("should include elapsed time in timeout error", func(t *testing.T) { + // [test_id:TS-GH-2354-013] + // The awaitWorkflowRun error message includes "timed out after Xs". + // When context times out before the internal deadline, we get + // context.DeadlineExceeded instead. Either way, the output should + // contain timing information. + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{}, + } + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + err := layer.Install(ctx) + require.NoError(t, err) + + output := buf.String() + assert.Contains(t, output, "could not confirm enrollment", + "should log timeout warning") + + // The error message should contain either: + // - "timed out after Xs" (internal deadline), or + // - "context deadline exceeded" / "context canceled" + // Both provide timing context to the user. + hasTimedOut := regexp.MustCompile(`timed out after \d+s`).MatchString(output) + hasDeadline := regexp.MustCompile(`context`).MatchString(output) + assert.True(t, hasTimedOut || hasDeadline, + "timeout warning should include elapsed time or context info, got: %s", output) + }) +} + +// containsAny returns true if s contains any of the substrings. +func containsAny(s string, substrs ...string) bool { + for _, sub := range substrs { + if len(sub) > 0 && len(s) >= len(sub) { + for i := 0; i <= len(s)-len(sub); i++ { + if s[i:i+len(sub)] == sub { + return true + } + } + } + } + return false +} diff --git a/qf-tests/GH-2354/go/enrollment_unenrollment_parity_test.go b/qf-tests/GH-2354/go/enrollment_unenrollment_parity_test.go new file mode 100644 index 000000000..0657e83fc --- /dev/null +++ b/qf-tests/GH-2354/go/enrollment_unenrollment_parity_test.go @@ -0,0 +1,137 @@ +//go:build e2e + +package layers + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +/* +Enrollment Unenrollment Parity Tests + +STP Reference: outputs/stp/GH-2354/GH-2354_test_plan.md +Jira: GH-2354 + +These tests validate that the unenrollment (uninstall) workflow uses the +same bounded timeout and exponential backoff as enrollment install. Both +code paths share the awaitWorkflowRun function, so parity is enforced at +the code level. These tests confirm that parity through behavioural testing. +*/ + +// unenrollConfigYAML is a minimal config.yaml for unenrollment tests. +const unenrollConfigYAML = `version: "1" +dispatch: + platform: github-actions +defaults: + roles: [triage] + max_implementation_retries: 2 + auto_merge: false +agents: [] +repos: + repo-a: + enabled: true +` + +// TestUnenrollmentParity validates that unenrollment uses the same timeout +// and backoff behaviour as enrollment. +func TestUnenrollmentParity(t *testing.T) { + t.Run("should use bounded timeout for unenrollment", func(t *testing.T) { + // [test_id:TS-GH-2354-017] + // Setup: FakeClient with config but no workflow runs. + // Unenrollment dispatches the workflow and then awaits it. + // With no matching runs, awaitWorkflowRun will poll until timeout. + client := forge.NewFakeClient() + client.FileContents["test-org/.fullsend/config.yaml"] = []byte(unenrollConfigYAML) + // No workflow runs → awaitWorkflowRun will timeout + + layer, buf := newEnrollmentLayer(t, client, nil, []string{"repo-a"}) + + // Use a short context timeout to avoid waiting 3 minutes. + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + start := time.Now() + err := layer.Uninstall(ctx) + elapsed := time.Since(start) + + // Uninstall treats awaitWorkflowRun errors as non-fatal. + require.NoError(t, err) + + output := buf.String() + // Verify the timeout/cancellation warning was emitted. + assert.Contains(t, output, "could not confirm unenrollment", + "should warn about unenrollment confirmation failure") + + // Verify the operation was bounded by the context timeout. + assert.Less(t, elapsed, 10*time.Second, + "unenrollment should complete within the context timeout") + }) + + t.Run("should match enrollment backoff pattern", func(t *testing.T) { + // [test_id:TS-GH-2354-018] + // Both Install and Uninstall call awaitWorkflowRun, which uses + // nextInterval for backoff. Since nextInterval is a shared helper, + // parity is guaranteed at the code level. + // + // This test verifies that the same constants are used by checking + // that nextInterval produces the same sequence regardless of caller. + // We also verify that unenrollment exercises the polling path. + + // Verify backoff constants are shared (they're package-level consts). + assert.Equal(t, 2*time.Second, enrollmentPollInitial, + "initial poll interval should be 2s for both enroll and unenroll") + assert.Equal(t, 15*time.Second, enrollmentPollMax, + "max poll interval should be 15s for both enroll and unenroll") + assert.Equal(t, 3*time.Minute, enrollmentWaitTimeout, + "wait timeout should be 3m for both enroll and unenroll") + + // Verify the backoff sequence is consistent. + interval := enrollmentPollInitial + expectedSequence := []time.Duration{ + 4 * time.Second, + 8 * time.Second, + enrollmentPollMax, + enrollmentPollMax, + } + for i, expected := range expectedSequence { + interval = nextInterval(interval) + assert.Equal(t, expected, interval, + "backoff step %d should be %v (shared by enroll and unenroll)", i+1, expected) + } + + // Integration check: verify unenrollment actually enters the polling path. + client := forge.NewFakeClient() + client.FileContents["test-org/.fullsend/config.yaml"] = []byte(unenrollConfigYAML) + // In-progress run to trigger polling progress messages. + now := time.Now().UTC() + client.WorkflowRuns["test-org/.fullsend/repo-maintenance.yml"] = &forge.WorkflowRun{ + ID: 5, + Status: "in_progress", + Conclusion: "", + CreatedAt: now.Add(time.Minute).Format(time.RFC3339), + HTMLURL: "https://github.com/test-org/.fullsend/actions/runs/5", + } + + layer, buf := newEnrollmentLayer(t, client, nil, []string{"repo-a"}) + + ctx, cancel := context.WithTimeout(context.Background(), 6*time.Second) + defer cancel() + + err := layer.Uninstall(ctx) + require.NoError(t, err) + + output := buf.String() + // Verify that polling progress was emitted (same as enrollment). + assert.Contains(t, output, "in_progress", + "unenrollment should emit polling progress like enrollment") + assert.Contains(t, output, "elapsed", + "unenrollment should report elapsed time like enrollment") + }) +} diff --git a/qf-tests/GH-2354/go/enrollment_user_interruption_test.go b/qf-tests/GH-2354/go/enrollment_user_interruption_test.go new file mode 100644 index 000000000..6afb1a9f2 --- /dev/null +++ b/qf-tests/GH-2354/go/enrollment_user_interruption_test.go @@ -0,0 +1,115 @@ +//go:build e2e + +package layers + +import ( + "context" + "runtime" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/forge" +) + +/* +Enrollment User Interruption Tests + +STP Reference: outputs/stp/GH-2354/GH-2354_test_plan.md +Jira: GH-2354 + +These tests validate that enrollment handles user interruption (Ctrl+C / +context cancellation) gracefully during polling, treating it as a non-fatal +condition with no goroutine leaks. Install() treats awaitWorkflowRun errors +(including context.Canceled) as non-fatal warnings, so we verify the +behaviour via output assertions and timing. +*/ + +// TestEnrollmentUserInterruption validates graceful handling of context +// cancellation during enrollment polling. +func TestEnrollmentUserInterruption(t *testing.T) { + t.Run("should stop polling on user interruption", func(t *testing.T) { + // [test_id:TS-GH-2354-014] + // Setup: No workflow runs → polling loop runs indefinitely. + // Cancel context after a short delay to simulate Ctrl+C. + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{}, + } + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + ctx, cancel := context.WithCancel(context.Background()) + // Cancel after 1 second to simulate user interruption. + go func() { + time.Sleep(1 * time.Second) + cancel() + }() + + start := time.Now() + err := layer.Install(ctx) + elapsed := time.Since(start) + + // Install treats this as non-fatal. + require.NoError(t, err) + // Assert: returns promptly after cancellation (within a few seconds). + assert.Less(t, elapsed, 5*time.Second, + "enrollment should stop promptly after context cancellation") + // Assert: warning was logged about the failure. + output := buf.String() + assert.Contains(t, output, "could not confirm enrollment", + "should warn about enrollment confirmation failure on cancellation") + }) + + t.Run("should treat interruption as non-fatal", func(t *testing.T) { + // [test_id:TS-GH-2354-015] + // Install() returns nil even when awaitWorkflowRun returns + // context.Canceled. This is by design: enrollment is best-effort. + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{}, + } + layer, buf := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // Cancel immediately + + // Assert: no panic. + require.NotPanics(t, func() { + err := layer.Install(ctx) + // Install returns nil (non-fatal). + assert.NoError(t, err, "context cancellation should be non-fatal") + }) + + output := buf.String() + // The warning should reference context cancellation. + assert.Contains(t, output, "could not confirm enrollment", + "should warn about enrollment failure, not crash") + }) + + t.Run("should exit cleanly with no hanging processes", func(t *testing.T) { + // [test_id:TS-GH-2354-016] + // Record baseline goroutine count, run enrollment with cancellation, + // then verify goroutines settle back to baseline. + baseline := runtime.NumGoroutine() + + client := &forge.FakeClient{ + WorkflowRuns: map[string]*forge.WorkflowRun{}, + } + layer, _ := newEnrollmentLayer(t, client, []string{"repo-a"}, nil) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // Cancel immediately + + err := layer.Install(ctx) + require.NoError(t, err) + + // Allow goroutines to settle. + time.Sleep(200 * time.Millisecond) + + current := runtime.NumGoroutine() + // Allow a small margin for background goroutines from the test framework. + assert.LessOrEqual(t, current, baseline+2, + "goroutine count should return near baseline (%d) after cancellation, got %d", + baseline, current) + }) +} diff --git a/skills/e2e-health/SKILL.md b/skills/e2e-health/SKILL.md new file mode 100644 index 000000000..e2cb6b216 --- /dev/null +++ b/skills/e2e-health/SKILL.md @@ -0,0 +1,52 @@ +--- +name: e2e-health +description: > + Use when checking e2e test health or reviewing recent e2e failures on main. +allowed-tools: Bash(bash skills/e2e-health/scripts/list-runs.sh:*), Bash(gh run view:*) +--- + +# E2E Health + +Check the health of the E2E Tests workflow on `main` over the last 2 days, summarize results in a table, and explain any failures. + +## Procedure + +### 1. Fetch recent runs + +```bash +bash skills/e2e-health/scripts/list-runs.sh # default: last 2 days +bash skills/e2e-health/scripts/list-runs.sh "7 days ago" # custom lookback +``` + +The argument is any string `date -d` accepts. Returns JSON with fields: `databaseId`, `displayTitle`, `conclusion`, `status`, `createdAt`, `url`. + +### 2. Present a summary table + +Format the results as a markdown table with clickable links: + +| Status | Run | Commit Title | When | +|--------|-----|--------------|------| +| pass/fail/in_progress | run-id (linked) | displayTitle | relative time | + +Use a green checkmark for success, red X for failure, and a spinner for in-progress. + +To determine the Status column: check `status` first — if it is not `completed`, the run is in-progress (conclusion will be null). If `status` is `completed`, use `conclusion` (`success` or `failure`). + +### 3. Diagnose failures + +For each failed run, fetch the failed step logs: + +```bash +gh run view --log-failed 2>&1 | grep -iE "(FAIL|--- FAIL|Error|panic|timeout)" +``` + +Read the matched lines and provide a brief explanation of why the run failed. Common failure categories: + +- **Flaky test** — timing-dependent or non-deterministic failure +- **Session expired** — GitHub session token needs rotation +- **Infrastructure** — GCP auth, Playwright deps, runner issues +- **Real regression** — a code change broke e2e behavior + +### 4. Overall assessment + +End with a one-line verdict: whether `main` is healthy, degraded, or broken based on the pattern of results. diff --git a/skills/e2e-health/scripts/list-runs.sh b/skills/e2e-health/scripts/list-runs.sh new file mode 100755 index 000000000..7b9475e8c --- /dev/null +++ b/skills/e2e-health/scripts/list-runs.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash +set -euo pipefail + +SINCE=$(date -d "${1:-2 days ago}" +%Y-%m-%d) + +gh run list \ + --workflow=e2e.yml \ + --branch=main \ + --created=">=$SINCE" \ + --limit=500 \ + --json databaseId,displayTitle,conclusion,status,createdAt,url