feat(cli): add runtime fallback to agents repo for unconfigured agents

[ggallen]

Summary

• When an agent is not registered in config.yaml, tryAgentsRepoFallback attempts to fetch the harness from fullsend-ai/agents before falling back to the scaffold-embedded harness on disk
• The fallback resolves the v0 floating version tag SHA via forge.Client.GetRef, constructs a commit-pinned raw.githubusercontent.com URL, checks the org allowlist, fetches via fetch.FetchURL, and caches content directly
• Adds GetRef to the forge.Client interface for general ref resolution (branches, tags); GetBranchRef now delegates to it. GetRef dereferences annotated tags to their underlying commit SHA
• Only known first-party agents (triage, code, fix, review, retro, prioritize) are eligible for fallback; the fallback is skipped in offline mode or when no forge client is available

This enables existing fullsend users to transparently use the extracted agents repository without any config changes, completing step 7 of the agent extraction plan.

Test plan

• TestTryAgentsRepoFallback_UnknownAgent — unknown agents skip fallback
• TestTryAgentsRepoFallback_Offline — offline mode skips fallback
• TestTryAgentsRepoFallback_NilClient — nil forge client skips fallback
• TestTryAgentsRepoFallback_GetRefError — ref resolution error falls through gracefully
• TestTryAgentsRepoFallback_NotAllowlisted — URL not in allowlist is rejected
• TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist — empty allowlist ([]string{}) denies all
• TestTryAgentsRepoFallback_AllKnownAgents — all 6 first-party agents resolve
• TestTryAgentsRepoFallback_SuccessPath — end-to-end success with cache and audit log
• TestTryAgentsRepoFallback_FetchURLError — fetch failure falls through
• TestFakeClient_GetRef — success, not found, error injection
• TestFakeClient_GetBranchRef_DelegatesToGetRef — priority, fallthrough, independent error injection
• TestGetRef — lightweight tag returns commit SHA
• TestGetRef_AnnotatedTag — annotated tag dereferences to commit SHA
• TestGetRef_NotFound — 404 returns ErrNotFound
• TestGetBranchRef_DelegatesToGetRef — branch resolution via GetRef
• go vet ./... clean
• go test ./internal/forge/... ./internal/cli/ all pass

🤖 Generated with Claude Code

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:45 PM UTC · Completed 5:17 PM UTC
Commit: c3093c2 · View workflow run →

[qodo-code-review]

PR Summary by Qodo

cli: fall back to fullsend-ai/agents for unconfigured agents

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Add runtime resolution of first-party agents from fullsend-ai/agents when not configured.
• Pin fetched harnesses to a resolved commit SHA and SHA256 integrity hash.
• Extend test coverage for offline/no-token/unknown-agent fallback behavior.

Diagram

graph TD
  A["fullsend run"] --> B["resolveAgentSource()"] --> C{"Agent registered in config?"}
  C -- "yes" --> D["Resolve from config"] --> Z["Harness path"]
  C -- "no" --> E{"Agents repo fallback allowed?"} --> K["Disk harness lookup"] --> Z
  E -- "yes" --> F{{"GitHub API"}} --> G{{"raw.githubusercontent.com"}} --> H["Compute SHA256 + pin URL"] --> I["FetchAgentHarness()"] --> Z
  I --> J[("Local cache")]
  K --> L["Local harness file"]
  subgraph Legend
    direction LR
    _p["Process"] ~~~ _d{"Decision"} ~~~ _e{{"External"}} ~~~ _c[("Cache")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Pin to a release tag / version manifest (instead of main HEAD)

• ➕ More deterministic and reproducible agent resolution
• ➕ Simpler auditing/debugging when behavior changes
• ➖ Requires release/tag management or a manifest publishing pipeline
• ➖ May slow propagation of urgent harness fixes

2. Ship first-party harnesses embedded in the binary (go:embed)

• ➕ Works offline and without git tokens
• ➕ No external availability dependency at runtime
• ➖ Larger binaries and slower iteration cadence (needs binary release)
• ➖ Harder to patch harness content without redeploying

3. Require explicit config migration (no implicit fallback)

• ➕ No surprises; configuration is the single source of truth
• ➕ Avoids runtime network calls in previously-offline workflows
• ➖ Breaks backward compatibility / adds migration friction
• ➖ Undermines the goal of transparent agents repo extraction

Recommendation: The PR’s approach is a good compatibility bridge: it keeps config precedence, adds a guarded fallback only for known first-party agents, and reuses the existing FetchAgentHarness allowlist/integrity pipeline by pinning to commit SHA + SHA256. If long-term determinism becomes a concern, consider evolving the fallback from “main HEAD” to a tagged release or manifest-based pinning.

Files changed (2) +139 / -8

Enhancement (1) +85 / -3

run.goAdd agents repo fallback path to agent harness resolution +85/-3

Add agents repo fallback path to agent harness resolution

• Introduces a guarded fallback that fetches first-party agent harnesses from fullsend-ai/agents when the agent is not configured. Resolves main branch HEAD SHA via the GitHub API, pins the raw URL with a computed SHA256 integrity hash, and then routes through the existing FetchAgentHarness validation/caching path. Updates resolveAgentSource signature to accept a git token and wires it from runAgent.

internal/cli/run.go

Tests (1) +54 / -5

run_test.goExtend resolveAgentSource tests for agents repo fallback gating +54/-5

Extend resolveAgentSource tests for agents repo fallback gating

• Updates existing resolveAgentSource tests for the new gitToken argument. Adds new tests to ensure fallback is skipped for unknown agents, offline mode, and missing tokens, and to confirm disk harness resolution still works when fallback is unavailable.

internal/cli/run_test.go

[github-actions]

Site preview

Preview: https://9db7cf02-site.fullsend-ai.workers.dev

Commit: c9e24923a43be6a45c00d3db9f18a11094dbdab3

[codecov]

Codecov Report

❌ Patch coverage is 83.01887% with 18 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/run.go	84.05%	6 Missing and 5 partials ⚠️
internal/forge/github/github.go	73.68%	3 Missing and 2 partials ⚠️
internal/forge/fake.go	88.88%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

1. Allowlist checked too late ✓ Resolved 🐞 Bug ⛨ Security

Description

tryAgentsRepoFallback fetches the harness content from raw.githubusercontent.com before verifying
the URL is permitted by allowed_remote_resources, so an org config can block use but cannot prevent
the outbound request. This undermines the intended policy boundary by allowing unexpected network
traffic even when the agents repo is not allowlisted.

Code

internal/cli/run.go[R2664-2687]

+	rawURL := agentsRepoURLPrefix + branchSHA + "/harness/" + agentName + ".yaml"
+
+	content, err := fetch.FetchURL(ctx, rawURL, composeOpts.FetchPolicy)
+	if err != nil {
+		return "", nil, false
+	}
+
+	hash := fetch.ComputeSHA256(content)
+	pinnedURL := rawURL + "#sha256=" + hash
+
+	allowlist := composeOpts.OrgAllowlist
+	if len(allowlist) == 0 {
+		allowlist = config.DefaultAllowedRemoteResources()
+	}
+
+	printer.StepStart(fmt.Sprintf("Fetching agent %s from %s/%s@%s", agentName, defaultAgentsRepoOwner, defaultAgentsRepoName, branchSHA[:12]))
+	localPath, dep, err := harness.FetchAgentHarness(ctx, pinnedURL, harness.ComposeOpts{
+		WorkspaceRoot: composeOpts.WorkspaceRoot,
+		FetchPolicy:   composeOpts.FetchPolicy,
+		AuditLogPath:  composeOpts.AuditLogPath,
+		OrgAllowlist:  allowlist,
+		TreeFetcher:   composeOpts.TreeFetcher,
+		GitToken:      composeOpts.GitToken,
+	})

Relevance

⭐⭐⭐ High

Security patterns favor fail-closed allowlist/SSRF enforcement; likely want allowlist check before
any outbound fetch.

PR-#1555
PR-#1096
PR-#792

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The fallback fetches remote content (FetchURL) before any allowed_remote_resources check is applied;
allowlist enforcement happens later inside FetchAgentHarness/fetchBaseURL, so the initial request
cannot be prevented by config allowlist.

internal/cli/run.go[2646-2687]
internal/harness/compose.go[323-337]
internal/harness/compose.go[348-357]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`tryAgentsRepoFallback` performs an initial network fetch (`fetch.FetchURL`) to compute the SHA256 integrity hash before it applies the org’s `allowed_remote_resources` checks (which occur inside `harness.FetchAgentHarness`). This means an org can configure `allowed_remote_resources` to *deny* the agents repo, but the process will still make an outbound request to `raw.githubusercontent.com`.

### Issue Context
- `harness.FetchAgentHarness` enforces `allowed_remote_resources` and integrity hashes.
- The fallback currently does a pre-fetch to compute the hash, and only later calls `FetchAgentHarness` (where allowlist enforcement happens).

### Fix Focus Areas
- internal/cli/run.go[2646-2687]
- internal/harness/compose.go[323-357]
- internal/harness/harness.go[866-870]

### Implementation notes
1. Compute the effective allowlist *before* calling `fetch.FetchURL`.
2. Before any network call, check that `rawURL` is allowlisted (e.g., with `harness.MatchingAllowedPrefixInList(rawURL, allowlist)`), and if not, return `("", nil, false)`.
3. Only proceed to `fetch.FetchURL` (hash computation) when the URL is allowed.
4. (Optional) log a `StepInfo/StepWarn` that fallback was skipped due to allowlist, to aid debugging.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. tryAgentsRepoFallback uses gh.New ✓ Resolved 📘 Rule violation ⌂ Architecture

Description

The new agents-repo fallback constructs and uses a GitHub-specific client (internal/forge/github)
directly instead of routing the forge operation through the forge.Client abstraction. This couples
internal/cli to GitHub implementation details and violates the requirement that forge operations
flow through forge.Client.

Code

internal/cli/run.go[R2657-2659]

+	client := gh.New(gitToken)
+	branchSHA, err := client.GetBranchRef(ctx, defaultAgentsRepoOwner, defaultAgentsRepoName, defaultAgentsRepoBranch)
+	if err != nil {

Relevance

⭐⭐⭐ High

Recent PRs enforce forge-neutral CLI; GetBranchRef added to forge.Client and GitHub coupling removed
elsewhere.

PR-#2770
PR-#2736
PR-#2415

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Compliance rule 1062052 requires forge operations to be routed through the forge.Client interface.
The new fallback code in internal/cli/run.go constructs a GitHub-specific client with
gh.New(gitToken) and calls GetBranchRef on it directly, rather than receiving and using a
forge.Client instance.

Rule 1062052: Route all git forge operations through forge.Client
internal/cli/run.go[2657-2659]
internal/forge/forge.go[303-307]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`internal/cli/run.go` introduces new forge behavior (resolving the `main` branch HEAD SHA for `fullsend-ai/agents`) but does so by constructing a GitHub-specific client (`gh.New(...)`) and calling `GetBranchRef` on it directly. Compliance requires forge operations to go through the `forge.Client` interface so non-forge packages remain forge-agnostic.

## Issue Context
`forge.Client` already defines `GetBranchRef(...)`, so this fallback can use the interface without importing/depending on `internal/forge/github` directly.

## Fix Focus Areas
- internal/cli/run.go[2583-2694]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Unguarded SHA truncation ✓ Resolved 🐞 Bug ☼ Reliability

Description

tryAgentsRepoFallback slices branchSHA[:12] for logging without checking length, which can panic and
crash the CLI if GetBranchRef ever returns an empty/short SHA (e.g., unexpected API
response/decoding). This turns a non-fatal fallback path into a hard failure.

Code

internal/cli/run.go[R2679-2680]

+	printer.StepStart(fmt.Sprintf("Fetching agent %s from %s/%s@%s", agentName, defaultAgentsRepoOwner, defaultAgentsRepoName, branchSHA[:12]))
+	localPath, dep, err := harness.FetchAgentHarness(ctx, pinnedURL, harness.ComposeOpts{

Relevance

⭐⭐ Medium

General defensive-coding accepted, but no direct precedent for guarding SHA truncation in logs.

PR-#1059
PR-#764
PR-#1238

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code slices branchSHA[:12] after only checking the error return, and the GitHub client returns
the decoded SHA string without validating non-emptiness, so a short string would cause a runtime
panic.

internal/cli/run.go[2657-2680]
internal/forge/github/github.go[1302-1316]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`branchSHA[:12]` is used directly in a log message. If `branchSHA` is shorter than 12 characters, Go will panic with a slice-bounds error.

### Issue Context
`GetBranchRef` returns a string SHA parsed from JSON; it is not locally validated as non-empty/non-short before being truncated for display.

### Fix Focus Areas
- internal/cli/run.go[2657-2681]
- internal/forge/github/github.go[1302-1316]

### Implementation notes
- Add a small helper:
 - If `len(branchSHA) >= 12`, use `branchSHA[:12]`.
 - Else use `branchSHA` as-is (or treat empty as an error and skip fallback).
- Consider also validating `branchSHA != ""` and returning fallthrough (`false`) with a warning if empty.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[fullsend-ai-review]

Review

The PR adds a runtime fallback to resolve known first-party agents from fullsend-ai/agents when not registered in config, along with a GetRef forge interface method, extensive tests, and comprehensive documentation (ADR annotation, architecture.md update, 781-line extraction plan). The three-tier resolution model (config → agents repo → disk scaffold) is correctly implemented with proper guard rails: known-agent gate, offline check, nil-client check, SHA validation via commitSHAPattern, allowlist enforcement, annotated tag dereferencing, and graceful error handling.

Prior review findings (SHA 3ce5eb73) were all low severity. The head SHA has changed to c9e24923 and four findings have been addressed:

• ✅ [regex-duplication] — Now uses commitSHAPattern directly instead of introducing a duplicate regex
• ✅ [silent-error] — FetchURL failure now logs printer.StepWarn before returning false
• ✅ [missing-readme-link] — Both plan documents now linked in README plans index
• ✅ [edge-case] — GetRef now dereferences annotated tags to their underlying commit SHA via the /git/tags/{sha} API

Findings

Low

• [integrity-bypass] internal/cli/run.go — The contentHash computed post-fetch serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, SSRF-hardened fetch (HTTPS-only, domain allowlist, DNS pre-resolution with IP validation, redirect blocking), and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources() (verified: allows only fullsend-ai/fullsend/ and fullsend-ai/agents/ prefixes). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the v0 tag SHA at runtime via GetRef, meaning the content fetched can change between deployments when the tag is moved. This is improved from the prior branch-based approach (tags require explicit force-push to move vs automatic branch HEAD advancement), but remains a weaker guarantee than pinned commit SHAs. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and tag protection.

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

Previous run

Review

The PR adds a runtime fallback to resolve known first-party agents from fullsend-ai/agents when not registered in config, along with a GetRef forge interface method, extensive tests, and comprehensive documentation (ADR annotation, architecture.md update, 781-line extraction plan). The three-tier resolution model (config → agents repo → disk scaffold) is correctly implemented with proper guard rails: known-agent gate, offline check, nil-client check, SHA validation, allowlist enforcement, and graceful error handling. The GetBranchRef → GetRef refactoring preserves backward compatibility with existing callers and tests.

Prior review findings (SHA 18253dcd) were all low severity. The head SHA has changed to 3ce5eb73 but the fundamental code structure and security properties are unchanged. All prior findings remain valid and are anchored at their prior severity.

Findings

Low

• [regex-duplication] internal/cli/run.go — validHexSHA (line 59 in diff) compiles ^[0-9a-f]{40}$, which is identical to commitSHAPattern in internal/cli/agent.go:21. Both are in package cli and directly accessible. Consolidate to avoid maintenance overhead.
  Remediation: Remove validHexSHA and use commitSHAPattern at the call site (line 960 equivalent).

• [silent-error] internal/cli/run.go — When fetch.FetchURL fails in tryAgentsRepoFallback, the function returns false without calling printer.StepWarn, unlike every other error path in the same function (GetRef failure, invalid SHA, allowlist miss, CachePut failure all log warnings). This makes debugging harder when the fallback fails silently and the user only sees "harness file not found" from the disk fallback.
  Remediation: Add printer.StepWarn(fmt.Sprintf("Failed to fetch agent %s from agents repo: %v", agentName, err)) before the return.

• [missing-readme-link] README.md — The new plan document docs/plans/agent-extraction-to-agents-repo.md is referenced from ADR 0058 and architecture.md but not listed in the README's plans index section (lines 46–59), where all other plan documents are linked.
  Remediation: Add a link entry in the plans section.

• [integrity-bypass] internal/cli/run.go — The contentHash computed post-fetch serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the v0 tag SHA at runtime via GetRef, meaning the content fetched can change between deployments when the tag is moved. This is improved from the prior branch-based approach (tags require explicit moves vs automatic branch HEAD advancement), but remains a weaker guarantee than pinned commit SHAs. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and tag protection.

• [edge-case] internal/forge/github/github.go — GetRef uses the GitHub /git/ref/{refPath} API which returns object.sha. For annotated tags, this returns the tag object SHA (not the commit SHA). Using a tag object SHA in a raw.githubusercontent.com URL would 404 because raw content URLs require commit-ish references. The fallback gracefully handles this (FetchURL error → return false → fall through to disk). Low impact since v0 is described as a floating tag (likely lightweight, not annotated).
  Remediation: Consider dereferencing the tag object to its commit SHA when object.type is "tag".

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

---

Labels: PR modifies agent resolution logic in CLI runner and forge interface, adds extraction plan documentation

Previous run

Review

The PR adds a runtime fallback to resolve known first-party agents from fullsend-ai/agents when not registered in config, along with a GetRef forge interface method, extensive tests, and comprehensive documentation (ADR annotation, architecture.md update, 781-line extraction plan). The three-tier resolution model (config → agents repo → disk scaffold) is correctly implemented with proper guard rails: known-agent gate, offline check, nil-client check, SHA validation, allowlist enforcement, and graceful error handling. The GetBranchRef → GetRef refactoring preserves backward compatibility with existing callers and tests.

Prior review findings (SHA 18253dcd) were all low severity. The head SHA has changed to 3ce5eb73 but the fundamental code structure and security properties are unchanged. All prior findings remain valid and are anchored at their prior severity.

Findings

Low

• [regex-duplication] internal/cli/run.go — validHexSHA (line 59 in diff) compiles ^[0-9a-f]{40}$, which is identical to commitSHAPattern in internal/cli/agent.go:21. Both are in package cli and directly accessible. Consolidate to avoid maintenance overhead.
  Remediation: Remove validHexSHA and use commitSHAPattern at the call site (line 960 equivalent).

• [silent-error] internal/cli/run.go — When fetch.FetchURL fails in tryAgentsRepoFallback, the function returns false without calling printer.StepWarn, unlike every other error path in the same function (GetRef failure, invalid SHA, allowlist miss, CachePut failure all log warnings). This makes debugging harder when the fallback fails silently and the user only sees "harness file not found" from the disk fallback.
  Remediation: Add printer.StepWarn(fmt.Sprintf("Failed to fetch agent %s from agents repo: %v", agentName, err)) before the return.

• [missing-readme-link] README.md — The new plan document docs/plans/agent-extraction-to-agents-repo.md is referenced from ADR 0058 and architecture.md but not listed in the README's plans index section (lines 46–59), where all other plan documents are linked.
  Remediation: Add a link entry in the plans section.

• [integrity-bypass] internal/cli/run.go — The contentHash computed post-fetch serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the v0 tag SHA at runtime via GetRef, meaning the content fetched can change between deployments when the tag is moved. This is improved from the prior branch-based approach (tags require explicit moves vs automatic branch HEAD advancement), but remains a weaker guarantee than pinned commit SHAs. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and tag protection.

• [edge-case] internal/forge/github/github.go — GetRef uses the GitHub /git/ref/{refPath} API which returns object.sha. For annotated tags, this returns the tag object SHA (not the commit SHA). Using a tag object SHA in a raw.githubusercontent.com URL would 404 because raw content URLs require commit-ish references. The fallback gracefully handles this (FetchURL error → return false → fall through to disk). Low impact since v0 is described as a floating tag (likely lightweight, not annotated).
  Remediation: Consider dereferencing the tag object to its commit SHA when object.type is "tag".

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

---

Labels: PR modifies agent resolution logic in CLI runner and forge interface, adds extraction plan documentation

Previous run (2)

Review

Findings

Low

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function computes a SHA256 hash post-fetch that serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the v0 tag SHA at runtime via GetRef, meaning the content fetched can change between deployments when the tag is moved. This is improved from the prior branch-based approach (tags require explicit moves vs automatic branch HEAD advancement), but remains a weaker guarantee than pinned commit SHAs. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and tag protection.

• [edge-case] internal/forge/github/github.go — GetRef uses the GitHub /git/ref/{refPath} API which returns object.sha. For annotated tags, this returns the tag object SHA (not the commit SHA). Using a tag object SHA in a raw.githubusercontent.com URL would 404 because raw content URLs require commit-ish references. The fallback gracefully handles this (FetchURL error → return false → fall through to disk). Low impact since v0 is described as a floating tag (likely lightweight, not annotated).
  Remediation: Consider dereferencing the tag object to its commit SHA when object.type is "tag" (GitHub returns both object.sha and object.type in the /git/ref response).

Previous run (3)

Review

Findings

High

• [test-integrity] internal/forge/fake.go — The refactored FakeClient.GetBranchRef no longer checks f.err("GetBranchRef") for error injection. The old implementation checked this before any map lookup; the new implementation checks BranchRefs first, then falls through to GetRef which only checks f.err("GetRef"). This silently breaks error injection for at least two existing tests not modified by this PR:
  • TestPinAgentURL_TransientErrorDoesNotFallback (agent_test.go:718) — sets Errors["GetBranchRef"] to "HTTP 500: internal server error" and asserts the error contains that message and does NOT contain "default branch". With the new code, the injected error is ignored, GetRef returns ErrNotFound, the fallback path fires, and all three assertions fail.
  • TestRunAgentUpdate_ForgeGetBranchRefError (agent_test.go:974) — sets Errors["GetBranchRef"] to "rate limited". The error injection is silently ignored; the test passes but for the wrong reason (ErrNotFound from GetRef, not the injected error).
    Remediation: Add f.err("GetBranchRef") check to the new GetBranchRef wrapper before falling through to GetRef. This preserves backward compatibility with existing test error injection.

Medium

• [documentation-code-mismatch] docs/plans/agent-registration.md:824 — Plan states "The fallback resolves the main branch HEAD SHA via the forge client" but the implementation resolves tags/v0 (via forgeClient.GetRef(ctx, owner, repo, "tags/" + config.DefaultUpstreamRef) where DefaultUpstreamRef = "v0"). This is a tag reference, not a branch reference — a deliberate improvement over the earlier branch-based approach, but the documentation does not reflect the actual mechanism.
  Remediation: Update to: "The fallback resolves the floating version tag (v0) SHA via the forge client".

Low

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function computes a SHA256 hash post-fetch that serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the v0 tag SHA at runtime via GetRef, meaning the content fetched can change between deployments when the tag is moved. This is improved from the prior branch-based approach (tags require explicit moves vs automatic branch HEAD advancement), but remains a weaker guarantee than pinned commit SHAs. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and tag protection.

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

---

Labels: PR modifies agent resolution logic in the CLI runner and forge interface with substantial documentation updates.

Previous run (4)

Review

Findings

Low

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function computes a SHA256 hash post-fetch that serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the v0 tag SHA at runtime via GetRef, meaning the content fetched can change between deployments when the tag is moved. This is improved from the prior branch-based approach (tags require explicit moves vs automatic branch HEAD advancement), but remains a weaker guarantee than pinned commit SHAs. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and tag protection.

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

Previous run (5)

Review

Findings

Low

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function computes a SHA256 hash post-fetch that serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the v0 tag SHA at runtime via GetRef, meaning the content fetched can change between deployments when the tag is moved. This is improved from the prior branch-based approach (tags require explicit moves vs automatic branch HEAD advancement), but remains a weaker guarantee than pinned commit SHAs. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and tag protection.

• [edge-case] internal/forge/github/github.go — GetRef uses the GitHub /git/ref/{refPath} API which returns object.sha. For annotated tags, this returns the tag object SHA (not the commit SHA). Using a tag object SHA in a raw.githubusercontent.com URL would 404 because raw content URLs require commit-ish references. The fallback gracefully handles this (FetchURL error → return false → fall through to disk). Low impact since v0 is described as a floating tag (likely lightweight, not annotated).
  Remediation: Consider dereferencing the tag object to its commit SHA when object.type is "tag" (GitHub returns both object.sha and object.type in the /git/ref response).

Previous run (6)

Review

Findings

High

• [test-integrity] internal/forge/fake.go — The refactored FakeClient.GetBranchRef no longer checks f.err("GetBranchRef") for error injection. The old implementation checked this before any map lookup; the new implementation checks BranchRefs first, then falls through to GetRef which only checks f.err("GetRef"). This silently breaks error injection for at least two existing tests not modified by this PR:
  • TestPinAgentURL_TransientErrorDoesNotFallback (agent_test.go:718) — sets Errors["GetBranchRef"] to "HTTP 500: internal server error" and asserts the error contains that message and does NOT contain "default branch". With the new code, the injected error is ignored, GetRef returns ErrNotFound, the fallback path fires, and all three assertions fail.
  • TestRunAgentUpdate_ForgeGetBranchRefError (agent_test.go:974) — sets Errors["GetBranchRef"] to "rate limited". The error injection is silently ignored; the test passes but for the wrong reason (ErrNotFound from GetRef, not the injected error).
    Remediation: Add f.err("GetBranchRef") check to the new GetBranchRef wrapper before falling through to GetRef. This preserves backward compatibility with existing test error injection.

Medium

• [documentation-code-mismatch] docs/plans/agent-registration.md:824 — Plan states "The fallback resolves the main branch HEAD SHA via the forge client" but the implementation resolves tags/v0 (via forgeClient.GetRef(ctx, owner, repo, "tags/" + config.DefaultUpstreamRef) where DefaultUpstreamRef = "v0"). This is a tag reference, not a branch reference — a deliberate improvement over the earlier branch-based approach, but the documentation does not reflect the actual mechanism.
  Remediation: Update to: "The fallback resolves the floating version tag (v0) SHA via the forge client".

Low

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function computes a SHA256 hash post-fetch that serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the v0 tag SHA at runtime via GetRef, meaning the content fetched can change between deployments when the tag is moved. This is improved from the prior branch-based approach (tags require explicit moves vs automatic branch HEAD advancement), but remains a weaker guarantee than pinned commit SHAs. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and tag protection.

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

---

Labels: PR modifies agent resolution logic in the CLI runner and forge interface with substantial documentation updates.

Previous run (7)

Review

Findings

Low

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function computes a SHA256 hash post-fetch that serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the main branch HEAD at runtime via GetBranchRef, meaning the content fetched can change between deployments without any binary update. A compromise of the fullsend-ai/agents repo's main branch could push malicious harness YAML automatically fetched by all installations using the fallback. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and branch protections.

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

Previous run (8)

Review

Findings

Low

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function computes a SHA256 hash post-fetch that serves only as a cache key and audit log entry, not for integrity verification. Unlike config-registered agents (where hashes are pinned at enrollment time), this fallback path has no known-good hash to compare against. Supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist. This is a documented design trade-off for the transitional mechanism.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go — The fallback resolves the main branch HEAD at runtime via GetBranchRef, meaning the content fetched can change between deployments without any binary update. A compromise of the fullsend-ai/agents repo's main branch could push malicious harness YAML automatically fetched by all installations using the fallback. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and branch protections.

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

Previous run (9)

Review

Findings

High

• [error-handling-gap] internal/cli/run.go:2717 — If CachePut fails on line 2717, the function logs a warning but continues to compute localPath via CachePath (line 2721) and returns it as the resolved harness path. CachePut is what writes the content file to disk at the CachePath-derived directory (confirmed by reading cache.go:101-123 — CachePut internally calls CachePath with the same content hash, then writes via atomicWrite). A failure means the file at localPath does not exist. The downstream caller will attempt to read this non-existent file and fail with an opaque error. The function should return false and fall through to disk-based lookup.
  Remediation: When CachePut fails, return ("", nil, false) to fall through to disk-based lookup, matching the function's documented contract that all errors are non-fatal.

Medium

• [integrity-bypass] internal/cli/run.go:2715 — The tryAgentsRepoFallback function fetches YAML content via fetch.FetchURL, computes its SHA256 post-fetch (line 2715), but this hash serves no integrity purpose — it is only used as a cache key and audit log entry. Unlike config-registered agents where the hash is pinned at enrollment time and verified against fetched content, this fallback path has no known-good hash to verify against. Supply-chain integrity relies solely on the commit-pinned URL, TLS transport, and the org allowlist. The code comment on lines 2711–2714 explicitly acknowledges this limitation.
  Remediation: Consider embedding known-good SHA256 hashes for each agent in defaultAgentsRepoKnownAgents, or pin a specific commit SHA in the binary rather than resolving main at runtime.

• [documentation-correctness] docs/plans/agent-registration.md:258 — The plan states the agents repo fallback "routes through FetchAgentHarness for integrity validation." This is factually incorrect. The implementation calls fetch.FetchURL directly (line 2706), manually caches content via fetch.CachePut (line 2717), and constructs the harness.Dependency struct itself (lines 2741–2748). It does not call FetchAgentHarness at all — that function is only used in the config-registered URL path (line 2637). This mismatch could mislead future reviewers into believing the fallback has the same integrity guarantees as config-registered agents.
  Remediation: Update the plan text to accurately describe the fallback's fetch mechanism: direct fetch.FetchURL with commit-pinned URL, manual caching, and no FetchAgentHarness call.

Low

• [fail-open] internal/cli/run.go:2679 — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). This means the agents-repo fallback activates for orgs without config. Mitigated: an explicitly empty allowlist ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [mutable-reference] internal/cli/run.go:2683 — The fallback resolves the main branch HEAD at runtime via GetBranchRef, meaning the content fetched can change between deployments without any binary update. A compromise of the fullsend-ai/agents repo's main branch could push malicious harness YAML automatically fetched by all installations using the fallback. Mitigated by defaultAgentsRepoKnownAgents gate (6 agents only), org allowlist, SHA validation, and branch protections.

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

• [scope-creep-beyond-authorization] internal/cli/run.go — The defaultAgentsRepoKnownAgents map hardcodes knowledge of the fullsend-ai/agents repository structure into the binary. ADR 0058's Context section identifies this pattern as the problem being solved. The code comments and ADR update now explicitly document this as a transitional mechanism with a planned deprecation path (Phase 5 / extraction plan Step 7).

---

Labels: PR adds agent resolution feature touching runner and harness components with substantial documentation updates.

Previous run (10)

Review

Findings

Medium

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function fetches YAML content from a commit-pinned URL, computes its SHA256, then passes a self-referential pinnedURL (rawURL + "#sha256=" + hash) to FetchAgentHarness. The integrity check in fetchBaseURL compares fetched content against the expectedHash extracted from the URL fragment — but that hash was derived from the same content moments earlier, making the verification tautological. For config-registered agents, the hash is pinned at enrollment time, providing a meaningful integrity anchor. This fallback path lacks that anchor. If the fullsend-ai/agents repository is compromised, malicious harness YAML would be accepted. The code comment acknowledges this limitation and notes that supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist rather than the hash.
  Remediation: Consider embedding known-good SHA256 hashes for each agent in the defaultAgentsRepoKnownAgents map (e.g., map[string]string with name → expected hash), or pin a specific commit SHA in the binary rather than resolving main at runtime.

Low

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). An org without config will still have agents resolved from the remote agents repo via this default allowlist. Mitigated: an explicitly empty AllowedRemoteResources ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

• [scope-creep-beyond-authorization] internal/cli/run.go — The defaultAgentsRepoKnownAgents map hardcodes knowledge of the fullsend-ai/agents repository structure into the binary. ADR 0058's Context section identifies this pattern ("which agents fullsend knows about is currently compiled into the binary") as the problem being solved. The code comments and ADR update now explicitly document this as a transitional mechanism with a planned deprecation path (ADR 0058 Phase 5 / extraction plan Step 7).

Previous run (11)

Review

Findings

Medium

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function fetches YAML content from a commit-pinned URL, computes its SHA256, then passes a self-referential pinnedURL (rawURL + "#sha256=" + hash) to FetchAgentHarness. The integrity check in fetchBaseURL compares fetched content against the expectedHash extracted from the URL fragment — but that hash was derived from the same content moments earlier, making the verification tautological. For config-registered agents, the hash is pinned at enrollment time, providing a meaningful integrity anchor. This fallback path lacks that anchor. If the fullsend-ai/agents repository is compromised, malicious harness YAML would be accepted. The code comment acknowledges this limitation and notes that supply-chain integrity relies on the commit-pinned URL, TLS transport, and the org allowlist rather than the hash.
  Remediation: Consider embedding known-good SHA256 hashes for each agent in the defaultAgentsRepoKnownAgents map (e.g., map[string]string with name → expected hash), or pin a specific commit SHA in the binary rather than resolving main at runtime.

Low

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). An org without config will still have agents resolved from the remote agents repo via this default allowlist. Mitigated: an explicitly empty AllowedRemoteResources ([]string{}) is not nil, so the default is NOT applied — this distinction is correctly tested in TestTryAgentsRepoFallback_ExplicitlyEmptyAllowlist.

• [missing-authorization] internal/cli/run.go — This PR adds a non-trivial feature with no linked GitHub issue. The work is documented via a new extraction plan (docs/plans/agent-extraction-to-agents-repo.md), updates to ADR 0058, and architecture.md — establishing context, but a tracking issue would provide clearer authorization lineage.

• [scope-creep-beyond-authorization] internal/cli/run.go — The defaultAgentsRepoKnownAgents map hardcodes knowledge of the fullsend-ai/agents repository structure into the binary. ADR 0058's Context section identifies this pattern ("which agents fullsend knows about is currently compiled into the binary") as the problem being solved. The code comments and ADR update now explicitly document this as a transitional mechanism with a planned deprecation path (ADR 0058 Phase 5 / extraction plan Step 7).

---

Labels: PR modifies agent harness resolution logic in the CLI runner and adds documentation for the agent extraction plan.

Previous run (12)

Review

Findings

High

• [stale-behavioral-description] docs/architecture.md — The Agent Registry section describes agent resolution as config-based with runtime URL/path loading, but does not document the new three-tier resolution model (config → agents repo fallback → disk harnesses) introduced by this PR. This is the primary architectural reference and is now materially inaccurate about a core mechanism.
  Remediation: Update the Agent Registry section to document the three-tier resolution model: (1) config entries from OrgConfig.Agents, (2) fallback to fullsend-ai/agents repo for known agents not in config, (3) fallback to disk harnesses.

Medium

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function fetches remote YAML content, computes its SHA256, then passes a self-referential pinnedURL (rawURL + "#sha256=" + hash) to FetchAgentHarness. The integrity verification inside fetchBaseURL is tautological — it verifies content against a hash derived from that same content. If the upstream agents repo is compromised, the fallback will accept the malicious harness with no integrity check against a known-good hash. Config-registered agents are protected because their hashes are pinned at enrollment time; this fallback path lacks equivalent protection.
  Remediation: Consider embedding known-good SHA256 hashes for each agent harness in the agentsRepoKnownAgents map, or pin a specific commit SHA in the binary rather than resolving main at runtime.

• [missing-authorization] internal/cli/run.go — This PR adds a runtime fallback mechanism to fetch agents from fullsend-ai/agents when not found in config, with no linked issue. The PR body references "step 7 of the agent extraction plan" but the documented agent-registration.md plan has 5 phases with no step 7.
  Remediation: Link the issue or plan document that authorizes this transitional fallback mechanism.

• [scope-design-mismatch] internal/cli/run.go — The diff introduces a third agent resolution layer (config → fullsend-ai/agents repo fallback → scaffold) not described in ADR-0058. ADR-0058 defines agent registration as a config-level concept with resolution from merged set of scaffold + config, then lookup by name. The agents repo fallback tier is not documented.
  Remediation: Update ADR-0058 or write a superseding ADR documenting this transitional fallback mechanism.

• [scope-creep-beyond-authorization] internal/cli/run.go — The new agentsRepoKnownAgents map hardcodes knowledge of the fullsend-ai/agents repository structure into the binary. ADR-0058's Context section explicitly identifies this pattern ("which agents fullsend knows about is currently compiled into the binary") as the problem being solved. If this is an intentional transitional aid, it must be documented as such with a planned deprecation path.
  Remediation: Document this as a transitional mechanism with a planned deprecation path.

• [missing-behavioral-change] docs/plans/agent-registration.md — Phase 3 implementation plan describes agent resolution as a two-tier model (config → disk scaffold fallback). The new tryAgentsRepoFallback and agents repo resolution tier are not documented in this plan.
  Remediation: Update Phase 3 sections to document the three-tier resolution.

Low

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). An org that deliberately removes the agents repo prefix from their AllowedRemoteResources would still see the fallback execute when orgCfg is nil, bypassing their intent to restrict remote resources.

• [input-validation] internal/cli/run.go — The branchSHA returned by forgeClient.GetBranchRef is interpolated directly into the URL without validation that it is a valid hex SHA. URL normalization in MatchingAllowedPrefixInList mitigates path traversal, but explicit validation (e.g., ^[0-9a-f]{40}$) would be cleaner defense-in-depth.

• [implementation-architecture-gap] internal/cli/run.go — The tryAgentsRepoFallback pre-fetches content and caches it before calling FetchAgentHarness, which does call the shared infrastructure. The duplication is limited to SHA resolution and content pre-fetch needed to construct the pinned URL.

• [redundant-network-call] internal/cli/run.go — tryAgentsRepoFallback fetches harness content via fetch.FetchURL, computes SHA256, caches with CachePut, then calls FetchAgentHarness which hits the cache. The double-fetch is mitigated by cache but there is still redundant parsing/processing.

• [naming-consistency] internal/cli/run.go — Inconsistent naming: defaultAgentsRepoOwner/Name/Branch use default prefix but agentsRepoURLPrefix and agentsRepoKnownAgents drop it.

Previous run (13)

Review

Findings

High

• [stale-behavioral-description] docs/architecture.md — The Agent Registry section describes agent resolution as config-based with runtime URL/path loading, but does not document the new three-tier resolution model (config → agents repo fallback → disk harnesses) introduced by this PR. This is the primary architectural reference and is now materially inaccurate about a core mechanism.
  Remediation: Update the Agent Registry section to document the three-tier resolution model: (1) config entries from OrgConfig.Agents, (2) fallback to fullsend-ai/agents repo for known agents not in config, (3) fallback to disk harnesses.

Medium

• [integrity-bypass] internal/cli/run.go — The tryAgentsRepoFallback function fetches remote YAML content, computes its SHA256, then passes a self-referential pinnedURL (rawURL + "#sha256=" + hash) to FetchAgentHarness. The integrity verification inside fetchBaseURL is tautological — it verifies content against a hash derived from that same content. If the upstream agents repo is compromised, the fallback will accept the malicious harness with no integrity check against a known-good hash. Config-registered agents are protected because their hashes are pinned at enrollment time; this fallback path lacks equivalent protection.
  Remediation: Consider embedding known-good SHA256 hashes for each agent harness in the agentsRepoKnownAgents map, or pin a specific commit SHA in the binary rather than resolving main at runtime.

• [missing-authorization] internal/cli/run.go — This PR adds a runtime fallback mechanism to fetch agents from fullsend-ai/agents when not found in config, with no linked issue. The PR body references "step 7 of the agent extraction plan" but the documented agent-registration.md plan has 5 phases with no step 7.
  Remediation: Link the issue or plan document that authorizes this transitional fallback mechanism.

• [scope-design-mismatch] internal/cli/run.go — The diff introduces a third agent resolution layer (config → fullsend-ai/agents repo fallback → scaffold) not described in ADR-0058. ADR-0058 defines agent registration as a config-level concept with resolution from merged set of scaffold + config, then lookup by name. The agents repo fallback tier is not documented.
  Remediation: Update ADR-0058 or write a superseding ADR documenting this transitional fallback mechanism.

• [scope-creep-beyond-authorization] internal/cli/run.go — The new agentsRepoKnownAgents map hardcodes knowledge of the fullsend-ai/agents repository structure into the binary. ADR-0058's Context section explicitly identifies this pattern ("which agents fullsend knows about is currently compiled into the binary") as the problem being solved. If this is an intentional transitional aid, it must be documented as such with a planned deprecation path.
  Remediation: Document this as a transitional mechanism with a planned deprecation path.

• [missing-behavioral-change] docs/plans/agent-registration.md — Phase 3 implementation plan describes agent resolution as a two-tier model (config → disk scaffold fallback). The new tryAgentsRepoFallback and agents repo resolution tier are not documented in this plan.
  Remediation: Update Phase 3 sections to document the three-tier resolution.

Low

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is nil (no org config loaded), tryAgentsRepoFallback falls back to config.DefaultAllowedRemoteResources(). An org that deliberately removes the agents repo prefix from their AllowedRemoteResources would still see the fallback execute when orgCfg is nil, bypassing their intent to restrict remote resources.

• [input-validation] internal/cli/run.go — The branchSHA returned by forgeClient.GetBranchRef is interpolated directly into the URL without validation that it is a valid hex SHA. URL normalization in MatchingAllowedPrefixInList mitigates path traversal, but explicit validation (e.g., ^[0-9a-f]{40}$) would be cleaner defense-in-depth.

• [implementation-architecture-gap] internal/cli/run.go — The tryAgentsRepoFallback pre-fetches content and caches it before calling FetchAgentHarness, which does call the shared infrastructure. The duplication is limited to SHA resolution and content pre-fetch needed to construct the pinned URL.

• [redundant-network-call] internal/cli/run.go — tryAgentsRepoFallback fetches harness content via fetch.FetchURL, computes SHA256, caches with CachePut, then calls FetchAgentHarness which hits the cache. The double-fetch is mitigated by cache but there is still redundant parsing/processing.

• [naming-consistency] internal/cli/run.go — Inconsistent naming: defaultAgentsRepoOwner/Name/Branch use default prefix but agentsRepoURLPrefix and agentsRepoKnownAgents drop it.

Previous run (14)

Review

Findings

Medium

• [logic-error] internal/cli/run.go — In tryAgentsRepoFallback, the agent name is normalized to lowercase for the known-agents check (strings.ToLower(agentName)) but used as-is when constructing the raw GitHub URL. GitHub raw content URLs are case-sensitive, so fullsend run Triage would pass the allowlist gate but construct .../harness/Triage.yaml instead of .../harness/triage.yaml, causing a silent 404 and falling through to disk. Use strings.ToLower(agentName) in the URL construction as well.

• [test-inadequate] internal/cli/run_test.go — TestResolveAgentSource_AgentsRepoFallback_DiskWins passes an empty git token, which causes tryAgentsRepoFallback to bail immediately (no-token guard). The test does not actually verify that disk "wins" over a successful agents-repo fetch — it only tests disk resolution when the fallback is skipped entirely. There is no test covering the interaction when both sources are available with a valid token.

• [scope-design-mismatch] internal/cli/run.go — tryAgentsRepoFallback adds a third resolution layer (config → agents repo → disk) not described in ADR-0058, which defines only config and scaffold/disk layers. Consider updating ADR-0058 or writing a superseding ADR to document this transitional fallback mechanism.

• [scope-creep-beyond-authorization] internal/cli/run.go — The new agentsRepoKnownAgents map and defaultAgentsRepo* constants hardcode knowledge of the fullsend-ai/agents repository structure into the binary. ADR-0058's Context section identifies this exact pattern ("which agents fullsend knows about is currently compiled into the binary") as the problem being solved. If this is an intentional transitional aid, document it as such with a planned deprecation path.

Low

• [redundant-network-call] internal/cli/run.go — The harness content is fetched via fetch.FetchURL to compute the SHA256 hash, then FetchAgentHarness fetches the same commit-SHA-pinned URL again. Consider caching the first fetch result or passing the content through to avoid the double round-trip.

• [fail-open] internal/cli/run.go — When composeOpts.OrgAllowlist is empty, the code substitutes config.DefaultAllowedRemoteResources(). An org that explicitly sets allowed_remote_resources: [] to prohibit remote fetches would have the default silently applied. Practical risk is limited (defaults only allow fullsend-ai-owned URLs).

• [edge-case] internal/cli/run.go — branchSHA[:12] in the StepStart message does not guard against an unexpectedly short SHA. In practice GetBranchRef always returns a 40-char hex string, but a defensive min(len(branchSHA), 12) truncation would prevent a theoretical panic.

• [comment-style] internal/cli/run.go — Const block comment and agentsRepoKnownAgents comment are slightly verbose compared to the established per-constant naming pattern (e.g., metricsFile is the filename...).

• [comment-accuracy] internal/cli/run.go — The resolveAgentSource comment could clarify that the agents-repo fallback only applies to unconfigured agents, not all agents.

• [error-handling-idiom] internal/cli/run.go — Minor: "Could not resolve" vs "Failed to" — both forms are used equally across the codebase, so this is a stylistic nit only.

---

Labels: PR modifies agent harness resolution logic in the CLI runner.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:36 PM UTC · Completed 5:53 PM UTC
Commit: 5965f00 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:01 PM UTC · Completed 6:17 PM UTC
Commit: 2d3468a · View workflow run →

Superseded by updated review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:27 PM UTC · Completed 6:41 PM UTC
Commit: 6626d23 · View workflow run →