feat(resolve): add runtime fetch schema and ResolveSkillURL (Phase 4 PR 1) by ggallen · Pull Request #2104 · fullsend-ai/fullsend

ggallen · 2026-06-10T00:11:33Z

Summary

Add allow_runtime_fetch and max_runtime_fetches fields to the harness schema with validation (negative max rejected, max without allow rejected, backward-compatible defaults)
Export ResolveSkillURL in internal/resolve/ for single-URL runtime skill directory resolution — uses forge API (ListDirectoryContents/GetFileContentAtRef) to fetch skill directories, ComputeTreeHash for integrity verification, and CachePutDir/CacheGetDir for directory caching, consistent with the directory model from feat(resolve): model skills as directories instead of single files #2139
Update Phase 4 implementation plan to reflect actual PR decomposition and directory model

This is PR 1 of 3 for Phase 4 (Runtime Dependency Loading) of ADR-0038. PR 2 adds the in-sandbox fullsend-fetch-skill script; PR 3 wires the runner-side socket handler and CLI integration.

Changes from rebase on #2139

ResolveSkillURL was rewritten to use the directory-based skill model introduced in #2139:

Skills are directories fetched via forge API (not single files via HTTP)
Tree hash verification (ComputeTreeHash) instead of file hash (ComputeSHA256)
Directory cache (CacheGetDir/CachePutDir) instead of file cache
Non-forge HTTPS URLs are rejected (no HTTP directory listing standard)
Returns directory path (<hash>/tree/) instead of file path (<hash>/content)
All tests rewritten to use forge.FakeClient and registerSkillDir helpers

Test plan

go test ./... — all 29 packages pass
make lint — clean
5 new harness validation tests for AllowRuntimeFetch/MaxRuntimeFetches
11 new ResolveSkillURL tests using forge-based directory model:
- valid fetch, cache hit, hash mismatch, allowlist rejection, missing hash, non-HTTPS rejection, non-forge URL rejection, nil ForgeClient, audit entry, offline miss/hit
Verify backward compatibility: harness without new fields loads and validates identically

🤖 Generated with Claude Code

github-actions · 2026-06-10T00:12:59Z

Site preview

Preview: https://beeaada0-site.fullsend-ai.workers.dev

Commit: 413e2cb51df3081d78c20f92c1d4e466f09b0aeb

fullsend-ai-review · 2026-06-10T00:13:14Z

🤖 Finished Review · ✅ Success · Started 12:13 AM UTC · Completed 12:24 AM UTC
Commit: 4ed6da4 · View workflow run →

fullsend-ai-review · 2026-06-10T00:24:01Z

Review

Findings

Medium

[scope-tier-mismatch] PR metadata — PR title claims "feat(resolve): add runtime fetch schema and ResolveSkillURL (Phase 4 PR 1)" but includes 6 categories of changes unrelated to Phase 4: binary package deletion (~800 lines), COMMITS.md→CONTRIBUTING.md migration, 3 new scaffold composite actions, merge-queue script deletion, skill doc simplification, and hosted mint URL removal. Phase 4 changes are ~150 lines; non-Phase 4 changes are ~2350+ lines.
Remediation: Update the title to reflect the actual scope or split into separate PRs.
[code-duplication] internal/cli/run.go — Cross-compilation logic in run.go (crossCompileFullsend) and admin.go (vendorFullsendBinary) both implement module-root discovery via go env GOMOD and go build with GOOS/GOARCH. Binary download logic in run.go is nearly identical to the deleted internal/binary/download.go code.
Remediation: Extract common cross-compilation and download logic into a shared internal package.
[error-message-consistency] internal/resolve/resolve.go — ResolveSkillURL uses a hardcoded const field = "runtime_skill" rather than accepting field as a parameter, unlike resolveFileURL/resolveSkillDirURL which receive field as a parameter. See also: [defense-in-depth] finding at this location.
Remediation: Accept field as a parameter for consistency, or document why it is hardcoded.
[architectural-coherence] internal/binary/ — Deletion of entire internal/binary package (7 files, ~800 lines). Logic was inlined into run.go and admin.go. The cli-internals.md removed the "Binary acquisition" section but does not document where the logic moved.
Remediation: Document the architectural change in cli-internals.md or a commit message.

Low

[missing-validation] internal/harness/harness.go — Validate() accepts allow_runtime_fetch: true with an empty allowed_remote_resources list. While ResolveSkillURL rejects every URL at runtime via MatchingAllowedPrefix, a user enabling runtime fetch but forgetting to add allowed prefixes gets no feedback until an agent tries to fetch.
[defense-in-depth] internal/resolve/resolve.go — ResolveSkillURL does not check h.AllowRuntimeFetch itself. The docstring states callers must gate on it. The function is exported, so future callers that omit the gate would bypass the control. No callers exist yet (PR 3 will add them). See also: [error-message-consistency] finding at this location.
[test-coverage-gap] internal/cli/run.go — resolveLinuxBinary implements a 3-strategy fallback chain but has no dedicated test exercising the orchestration logic. Individual helpers (downloadReleaseBinary, crossCompileFullsend, extractFullsendFromTarGz) are tested but the fallback sequencing is not covered.
[scope-creep] internal/scaffold/fullsend-repo/.github/actions/ — Three new composite GitHub Actions (mint-token, setup-gcp, validate-enrollment) are added but not mentioned in Phase 4 plan and are unrelated to runtime dependency loading.
[doc-style] internal/resolve/resolve.go — Minor godoc pattern deviation for ResolveSkillURL compared to sibling functions.
[validation-logic] internal/harness/harness.go — Error message "max_runtime_fetches requires allow_runtime_fetch: true" could be clearer about the zero-value exception.

Info

[consumer-completeness] internal/resolve/resolve.go — DefaultMaxRuntimeFetches is exported but has no consumers in this PR. Per Phase 4 plan, PR 3 will wire the rate limiter.
[secrets-handling] internal/scaffold/fullsend-repo/.github/actions/mint-token/action.yml — Correctly masks all sensitive values (MINT_URL, OIDC_TOKEN, MINT_RESPONSE, TOKEN). No issues found.
[secrets-handling] internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml — Correctly masks credentials. No issues found.
[input-validation] internal/scaffold/fullsend-repo/.github/actions/validate-enrollment/action.yml — Validates inputs and prevents workflow command injection. No issues found.
[scope-disclosure] PR metadata — PR body transparently discloses 7 distinct change categories beyond Phase 4 scope.

Previous run

Review

Findings

High

[protected-path] skills/merge-queue/SKILL.md, skills/merge-queue/scripts/dequeue-reason.sh, skills/merge-queue/scripts/queue-status.sh — Protected files under skills/ are modified (SKILL.md updated, two scripts deleted). The PR has no linked issue and does not explain why these protected governance files are being changed. Human approval is always required for protected-path changes.
Remediation: Link an issue authorizing the merge-queue script deletions, or add justification in the PR body explaining why these scripts are being removed.

Medium

[missing-test-entry] internal/scaffold/scaffold_test.go:57 — The new composite action .github/actions/mint-token/action.yml is added to the scaffold directory but is not included in the TestFullsendRepoFilesExist expected file list. Only setup-gcp/action.yml and validate-enrollment/action.yml were added. A separate TestMintTokenActionContent test exists but does not guard against the file being missing from the scaffold embed.
Remediation: Add ".github/actions/mint-token/action.yml" to the expected slice in TestFullsendRepoFilesExist.
[removed-guidance-still-valid] CLAUDE.md:27 — The diff removes the "Dispatch workflows" paragraph that instructs contributors to keep internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml and .github/workflows/reusable-dispatch.yml in sync when changing routing logic. Both files still exist in the repository and share routing logic. Removing this guidance without relocating it increases the risk of the two files drifting out of sync.
Remediation: Either keep the paragraph in CLAUDE.md, or move it to a more appropriate location (e.g., a comment in the dispatch workflow files, or AGENTS.md).

Low

[test-coverage-gap] internal/cli/run.go — The resolveLinuxBinary function inlines the fallback chain from the deleted binary.ResolveForRun. The new run_test.go tests cover individual helpers (isReleasedVersion, extractFullsendFromTarGz, crossCompileFullsend, downloadReleaseBinary, downloadChecksumForAsset, validateLinuxBinary) but not the resolveLinuxBinary orchestration function itself.
[missing-validation] internal/harness/harness.go:292 — Validate() does not check whether allow_runtime_fetch: true is accompanied by a non-empty allowed_remote_resources list. ResolveSkillURL has a runtime guard via MatchingAllowedPrefix, so this is defense-in-depth — explicit validation at config load time would give a better error message.
[fail-open] internal/resolve/resolve.go — ResolveSkillURL does not check h.AllowRuntimeFetch before resolving. The doc comment says callers must gate, but no defense-in-depth check exists in the function itself. No callers exist yet (PR 3 adds the socket handler).
[error-message-consistency] internal/resolve/resolve.go — Error message prefix runtime: differs from existing field-based error prefixes in the same file. Existing errors use the field name (e.g., skills[%d]:) as the prefix.

Info

[consumer-completeness] internal/resolve/resolve.go — DefaultMaxRuntimeFetches is exported but has no consumers in this PR. Expected — PR 3 will wire the rate limiter.
[scope-disclosure] — PR includes 7 distinct categories of changes beyond stated Phase 4 scope. Disclosed in PR body with requires-manual-review label.
[secrets-handling] internal/scaffold/fullsend-repo/.github/actions/mint-token/action.yml — Mint-token action correctly masks sensitive values (MINT_URL, OIDC_TOKEN, TOKEN, MINT_RESPONSE). No issues found.
[secrets-handling] internal/scaffold/fullsend-repo/.github/actions/setup-gcp/action.yml — Setup-gcp action correctly masks credentials. No issues found.
[input-validation] internal/scaffold/fullsend-repo/.github/actions/validate-enrollment/action.yml — Validate-enrollment action correctly validates inputs and prevents workflow command injection. No issues found.
[code-relocation] internal/cli/run.go — Binary download/checksum logic relocated from internal/binary/ with identical security properties (SHA256 verification, path traversal protection, max size limits preserved).

Previous run (2)

Review

Findings

Medium

[missing-test-entry] internal/scaffold/scaffold_test.go:57 — The new composite action .github/actions/mint-token/action.yml is added to the scaffold directory but not included in the TestFullsendRepoFilesExist expected file list. The diff adds setup-gcp and validate-enrollment to this list but omits mint-token. While TestMintTokenActionContent would catch a truly missing file, the existence completeness check is inconsistent.
Remediation: Add ".github/actions/mint-token/action.yml" to the expected list in TestFullsendRepoFilesExist.

Low

[removed-safeguard] CLAUDE.md:25 — The diff removes the "Dispatch workflows" paragraph that warned contributors to keep internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml and .github/workflows/reusable-dispatch.yml in sync. Both files still exist and share routing logic for per-org vs per-repo installation modes. Removing this guard increases the risk of introducing a routing logic divergence.
Remediation: Restore the dispatch workflow sync warning in CLAUDE.md or AGENTS.md.
[test-coverage-gap] internal/cli/run.go — resolveLinuxBinary implements a fallback chain (release download → cross-compile → latest release) inlined from the deleted binary.ResolveForRun, but the deleted package's tests covering this chain (preference ordering, cross-compile fallback, latest-release fallback, all-strategies-fail, max-download-size) were not ported. maxDownloadSize changed from var to const, removing the ability to test the max-size path.
[fail-open] internal/resolve/resolve.go — ResolveSkillURL does not check h.AllowRuntimeFetch before resolving. The doc comment says callers must gate, but no defense-in-depth check exists in the function itself. No callers exist yet (PR 3 adds the socket handler), and the function already enforces integrity hash, allowlist, HTTPS, and forge-URL gates independently.
Remediation: Consider adding if !h.AllowRuntimeFetch { return error } as defense-in-depth when wiring the handler in PR 3.
[missing-validation] internal/harness/harness.go:292 — Validate() does not check whether allow_runtime_fetch: true is accompanied by a non-empty allowed_remote_resources list. The Phase 4 plan explicitly defers this validation to PR 3.
Remediation: Add this validation in PR 3 as planned.

Info

[scope-creep] — PR scope exceeds stated intent: title claims "runtime fetch schema and ResolveSkillURL" but includes 7 categories of changes (Phase 4 schema, binary package deletion, COMMITS.md migration, scaffold actions, merge-queue script deletion, skill doc simplification, vendor logic removal). The scope is disclosed in the PR body and the PR carries requires-manual-review.
[scope-justification-missing] internal/cli/admin.go — The internal/binary/ package deletion and --fullsend-binary flag removal from admin install/setup commands lacks standalone justification. The flag is preserved on the run command.
[feature-removal-undocumented] skills/merge-queue/SKILL.md — queue-status.sh and dequeue-reason.sh were deleted without explanation; only enqueue-pr.sh remains.
[naming-convention] internal/resolve/resolve.go — DefaultMaxRuntimeFetches is exported but has no current external consumers. It may be needed by PR 3's socket handler; consider making it unexported if not.

Previous run (3)

Review

Findings

Medium

[silently-ignored-flag] internal/cli/github.go:139 — The --vendor-fullsend-binary flag is accepted without error in per-repo GitHub setup mode but silently ignored since the vendoring logic was removed from this code path. Users passing this flag will get no feedback that it has no effect, which could lead to confusion when the binary is not vendored as expected.
Remediation: Either remove the flag from per-repo mode entirely or emit a warning/error when it is passed in a context where it has no effect.
[logic-error] internal/cli/admin.go:1263 — In runDryRun, the call to vendorFullsendBinary hardcodes vendorBinary=false, which means dry-run mode will never exercise the binary vendoring path regardless of user intent. If a user runs --dry-run to preview what admin setup would do, they will not see vendoring activity even when --vendor-fullsend-binary is set.
Remediation: Pass the actual vendorBinary flag value to vendorFullsendBinary in runDryRun so dry-run output accurately reflects what the real run would do.

Low

[error-handling-idiom] internal/cli/run.go:246 — lookPath errors are wrapped but the fallback to download proceeds silently. Consider logging a debug/info message when falling back from local binary to download so operators can trace resolution behavior.
[stale-reference] CLAUDE.md — References to COMMITS.md ("You must read and follow COMMITS.md") are now stale since COMMITS.md was deleted. The commit message conventions have moved to CONTRIBUTING.md.
Remediation: Update the CLAUDE.md reference to point to CONTRIBUTING.md.
[missing-test] internal/resolve/resolve.go — ResolveSkillURL has no unit tests. The function handles URL validation, scheme enforcement, and domain allowlisting — all worth covering with tests for edge cases (empty input, non-HTTPS schemes, disallowed domains).

Info

[scope-inferred] — No linked issue found. Authorization inferred from ADR-0038 (Phase 4 runtime fetch schema) which covers the allow_runtime_fetch and max_runtime_fetches additions. The scope includes both the Phase 4 schema work and cleanup of the deleted internal/binary/ package.
[naming-convention] internal/resolve/resolve.go — DefaultMaxRuntimeFetches is exported but only used internally. Consider making it unexported (defaultMaxRuntimeFetches) unless it needs to be part of the public API.
[backward-incompatible] internal/layers/vendor.go — VendorBinary signature changed (removed commitMsg parameter). All internal call sites are updated, but external consumers (if any) would break. Low risk given this is an internal package.

Previous run (4)

Review

Findings

High

[logic-error] internal/cli/admin.go:1042 — New vendorFullsendBinary function runs go build ./cmd/fullsend/ without setting buildCmd.Dir to the module root. Unlike the sibling function crossCompileFullsend (which discovers the module root via go env GOMOD and sets buildCmd.Dir), this function relies on the CLI's working directory being the module root. If the user runs fullsend admin install --vendor-fullsend-binary from any subdirectory, go build ./cmd/fullsend/ will fail because ./cmd/fullsend/ is relative to CWD, not the module root. The deleted TestVendorFromSubdirectory was specifically testing this scenario.
Remediation: Add module root discovery to vendorFullsendBinary (same go env GOMOD + filepath.Dir pattern used in the deleted CrossCompile function) and set buildCmd.Dir = modRoot.

Medium

[scope-creep] internal/binary/ — PR deletes the entire internal/binary/ package (7 files) implementing the binary resolution policy, inlining a subset of the logic into internal/cli/run.go. This is a significant architectural change not mentioned in the PR title ("Phase 4 PR 1"). The PR body mentions "simplifies binary resolution" but understates the scope.
[scope-creep] internal/cli/vendor.go — PR deletes vendor.go and vendor_test.go, removing the multi-strategy binary acquisition policy (release download fallback) and replacing it with cross-compile-only vendoring in the new vendorFullsendBinary function. The --fullsend-binary flag is removed from admin install and github setup commands, eliminating the release download fallback for vendoring.
[silently-ignored-flag] internal/cli/github.go:139 — --vendor-fullsend-binary is still registered on the github setup command, but runGitHubSetupPerRepo had its vendoring and stale-cleanup blocks removed. Passing --vendor-fullsend-binary to per-repo github setup now silently does nothing. The flag is only consumed in runGitHubSetupPerOrg.
Remediation: Either wire vendorFullsendBinary into runGitHubSetupPerRepo, remove the flag from per-repo mode, or error if set with a per-repo target.
[logic-error] internal/cli/admin.go:1244 — runDryRun now hardcodes vendorBinary=false and vendorFn=nil when building the layer stack, regardless of whether --vendor-fullsend-binary was passed. This causes admin install --vendor-fullsend-binary --dry-run to incorrectly show the vendor layer as disabled.
Remediation: Pass the actual vendorBinary value through to runDryRun and buildLayerStack, or explicitly document that dry-run no longer previews vendoring.

Low

[stale-reference] docs/guides/dev/local-dev.md:74 — The binary resolution priority table was reduced from 4 steps to 3 by removing "Download latest release" (step 4), but the new resolveLinuxBinary function in internal/cli/run.go still includes a downloadLatestReleaseBinary fallback. The documentation no longer matches the code's actual resolution strategy.
[scope-creep] CLAUDE.md — PR removes the "Dispatch workflows" paragraph documenting the requirement to keep dispatch.yml and reusable-dispatch.yml in sync. Both workflow files still exist. However, the most recent merge on main is PR docs(#2067): add dispatch workflow sync guidance to CLAUDE.md #2068 ("dispatch-sync-guidance"), which may have addressed this concern through other means.
[scope-creep] docs/guides/infrastructure/mint-administration.md — PR removes the "Hosted mint" section documenting the public hosted mint URL. The replacement text says work is in progress to offer a hosted service. This changes operational guidance for users currently relying on the hosted URL.
[function-organization] internal/cli/run.go:889 — sandboxArch() is called internally within bootstrapCommon() creating tight coupling. The prior validateLinuxBinary accepted an arch parameter. The new pattern reduces testability, though FULLSEND_SANDBOX_ARCH env var provides a test workaround.

Info

[content-scanning-gap] internal/resolve/resolve.go:308 — ResolveSkillURL does not run security.InputPipeline().Scan() on fetched content. Consistent with the static resolution path. Documented as future enhancement in Phase 4 plan.
[edge-case] docs/plans/universal-harness-access-phase4.md:634 — The fullsend-fetch-skill script pseudocode escapes via sed but does not handle newlines/tabs in URLs. Plan documentation only — adequate for well-formed HTTPS URLs.
[missing-authorization] — No linked issue. Changes are well-documented via ADR-0038 and follow the established multi-phase pattern.
[scope-alignment] docs/plans/universal-harness-access-phase3.md — PR adds the Phase 3 plan document (197 lines) even though the PR title is "Phase 4 PR 1". Bundling historical plan documentation with Phase 4 implementation increases review burden.

Previous run (5)

Review

Findings

Medium

[silently-ignored-flag] internal/cli/github.go — runGitHubSetupPerRepo had its vendoring and stale-binary cleanup logic removed (both dry-run and live paths), but --vendor-fullsend-binary is still registered on the github setup command. For per-repo github setup, passing --vendor-fullsend-binary now silently does nothing. Per-org setup still uses the flag correctly.
Remediation: Either restore vendoring logic for per-repo github setup (calling vendorFullsendBinary as admin install does), or remove the flag from the github setup command, or emit a warning when the flag is set in per-repo mode.
[silent-truncation-regression] internal/cli/run.go — downloadReleaseBinary uses io.LimitReader(resp.Body, maxDownloadSize) without the +1 overflow detection pattern from the deleted internal/binary/download.go. The old code read maxSize+1 bytes and checked buf.Len() > maxSize to produce a clear "exceeds maximum size" error. The new code silently truncates at exactly maxDownloadSize bytes, causing an oversized download to fail with a confusing checksum-mismatch error. Note: extractFullsendFromTarGz in the same PR correctly preserves the +1 pattern for maxBinarySize.
Remediation: Change to io.LimitReader(resp.Body, maxDownloadSize+1) and add if int64(buf.Len()) > maxDownloadSize { return fmt.Errorf(...) } before the checksum verification.

Low

[stale-guidance-removal] CLAUDE.md:27 — The "Dispatch workflows" paragraph documenting the requirement to keep dispatch.yml and reusable-dispatch.yml in sync was removed, but both files still exist and are unchanged in this PR. The sync requirement appears still valid.
[code-organization] internal/cli/run.go — ~260 lines of binary resolution logic inlined from the deleted internal/binary/ package. The validateLinuxBinary function now calls sandboxArch() internally instead of accepting an arch parameter, reducing testability. Tests can work around this via FULLSEND_SANDBOX_ARCH env var.
[function-signature-inconsistency] internal/layers/vendor.go:21 — VendorBinary signature changed to remove commitMsg parameter. The simplified call site hardcodes the commit message. Appropriate since vendoring now only uses cross-compilation (one source type), but loses metadata in git history.
[commit-message-simplification] internal/layers/vendorbinary.go:86 — Changed from RemoveStaleBinaryCommitMessage(path) to hardcoded "chore: remove vendored binary". The old function included path, reason, and re-run instructions in the commit body.
[documentation-coherence] docs/guides/getting-started/installation.md — Vendoring doc simplified from 4-tier fallback chain to single-line description. The new documentation correctly reflects the simplified behavior.

Info

[content-scanning-gap] internal/resolve/resolve.go — ResolveSkillURL does not run security.InputPipeline().Scan() on fetched content. Consistent with the static resolution path (resolveURL). Documented as a future enhancement in the Phase 4 plan.
[edge-case] docs/plans/universal-harness-access-phase4.md — The fullsend-fetch-skill script pseudocode escapes via sed but does not handle newlines/tabs. Plan documentation only — to be addressed when implementing the script in PR 2.
[missing-authorization] — No linked issue. Changes are well-documented via ADR-0038 and follow the established multi-phase pattern.
[scope-tier-alignment] — PR title says "Phase 4 PR 1" but includes binary package deletion, vendor simplification, dispatch workflows guidance removal, and hosted mint URL removal beyond the Phase 4 scope.

Previous run (6)

Review

Findings

High

[stale-doc] docs/guides/dev/local-dev.md:92 — The "Security scanning and cross-platform binary" section (lines 82-114) documents the --fullsend-binary CLI flag and the four-priority binary resolution strategy (ResolveForRun, ResolveForVendor, ResolveExplicit). This PR removes the --fullsend-binary flag from internal/cli/admin.go, deletes the entire internal/binary/ package, and inlines a simplified resolution strategy into internal/cli/run.go. The documentation will actively mislead users into using a non-existent flag.
Remediation: Update or remove the "Security scanning and cross-platform binary" section to reflect the new binary resolution approach (download → cross-compile → latest release, with no --fullsend-binary flag).

Medium

[stale-doc] docs/guides/user/running-agents-locally.md:306 — Troubleshooting tip suggests using --fullsend-binary to provide a Linux binary explicitly. This flag is removed in this PR.
Remediation: Update or remove the troubleshooting tip referencing --fullsend-binary.

Low

[rate-limiting-deferred] internal/resolve/resolve.go:311 — ResolveSkillURL does not check h.AllowRuntimeFetch before proceeding, relying entirely on the caller to gate access and enforce MaxRuntimeFetches. Acceptable for Phase 4 PR 1 where the enforcement layer comes in PR 3; consider adding a self-defense guard when wiring the handler.
[no-upper-bound] internal/harness/harness.go:276 — Validation rejects negative MaxRuntimeFetches but does not enforce an upper bound. A harness author could set max_runtime_fetches: 999999, effectively disabling rate limiting. Low risk since harness authors are trusted.
[architectural-coherence] internal/harness/harness.go:213 — AllowRuntimeFetch: true with MaxRuntimeFetches: 0 means "use default (10)" via zero-value-as-default. Standard Go pattern, documented in comment.
[TOCTOU-cache-path] internal/resolve/resolve.go:336 — ResolveSkillURL returns a file path rather than verified content bytes on cache hit. Pre-existing pattern from resolveURL.
[URL-normalization-edge-case] internal/harness/harness.go:676 — normalizeURLPath rejects backslash characters but does not reject other path separator characters. Low risk on Linux with url.PathUnescape decoding.

Info

[edge-case] docs/plans/universal-harness-access-phase4.md:449 — The fullsend-fetch-skill script pseudocode escapes via sed but does not handle newlines/tabs. Plan documentation only.
[content-scanning-gap] internal/resolve/resolve.go:194 — Remotely fetched skills are not scanned by security.InputPipeline().Scan() on the host side. Documented as a future enhancement.
[missing-authorization] — No linked issue. Changes are well-documented via ADR-0038 and follow the established multi-phase pattern.
[function-documentation] internal/resolve/resolve.go:971 — ResolveSkillURL doc comment uses imperative style ("Callers must gate on...") vs declarative style of ResolveHarness. The imperative form is appropriate for documenting caller preconditions.
[validation-error-messages] internal/harness/harness.go:281 — Error message max_runtime_fetches requires allow_runtime_fetch: true uses a different pattern from existing messages. Both forms are clear.

Previous run (7)

Review

Findings

Low

[rate-limiting-deferred] internal/resolve/resolve.go:311 — ResolveSkillURL does not check h.AllowRuntimeFetch before proceeding, relying entirely on the caller to gate access and enforce rate limits. The function has zero callers until PR 3 ships the handler, and the godoc explicitly documents the caller contract. Consider adding a self-defense guard (if !h.AllowRuntimeFetch { return error }) when wiring the handler in PR 3.
[stale-doc-missing-field] docs/ADRs/0045-forge-portable-harness-schema.md:267 — ADR-0045's platform-neutral fields table does not include the newly added allow_runtime_fetch and max_runtime_fetches fields. These are platform-neutral and should be listed alongside allowed_remote_resources.
[edge-case] docs/plans/universal-harness-access-phase4.md:449 — The fullsend-fetch-skill script pseudocode escapes backslashes and double-quotes via sed but does not handle newlines, tabs, or other control characters. URLs with these characters would produce malformed JSON. The runner safely rejects malformed requests, but the error message would be confusing. Address when implementing the actual script in PR 2.
[no-upper-bound] internal/harness/harness.go:276 — Validation rejects negative MaxRuntimeFetches but does not enforce an upper bound. A harness author could set max_runtime_fetches: 999999, effectively disabling rate limiting. The harness author is a trusted party, so this is low risk.
[architectural-coherence] internal/harness/harness.go:213 — AllowRuntimeFetch: true with MaxRuntimeFetches: 0 means "use default (10)." This zero-value-as-default pattern is standard in Go but the three-state semantics could surprise users. The Phase 4 plan documents this; consider adding a clarifying comment at point of use in PR 3.
[stale-doc] docs/ADRs/0038-universal-harness-access.md:176 — ADR-0038 describes runtime dependency loading mitigations in present tense as if fully implemented, but the end-to-end feature requires Phase 4 PR 3. Consider updating the ADR's Implementation Plan section to reference Phase 3 and Phase 4 plan documents.
[TOCTOU-cache-path] internal/resolve/resolve.go:336 — When CacheGet returns a cache hit, ResolveSkillURL returns a file path rather than verified content bytes. A TOCTOU window exists between verification and the caller's read. This is the same pre-existing pattern as resolveURL — not a regression.

Info

[missing-authorization] — This PR has no linked issue. The changes are well-documented via ADR-0038 and the Phase 4 implementation plan, following the established multi-phase pattern (Phases 1-3). Consider creating a tracking issue for Phase 4 implementation.
[function-documentation] internal/resolve/resolve.go:971 — The ResolveSkillURL doc comment uses imperative language ("Callers must gate on...") vs the declarative style of ResolveHarness. The imperative form is appropriate for documenting caller preconditions.
[validation-error-messages] internal/harness/harness.go:281 — Error message max_runtime_fetches requires allow_runtime_fetch: true uses a different pattern from existing messages (X is required when Y). Both forms are clear.
[audit-fail-closed] internal/resolve/resolve.go:366 — ResolveSkillURL correctly fails the entire fetch if audit logging fails, preventing silent audit trail loss. Positive finding.
[HTTPS-enforcement] internal/resolve/resolve.go:322 — HTTPS-only enforcement occurs before any network operation, preventing downgrade attacks. Positive finding.
[runtime-skills-skip-scanning] docs/plans/universal-harness-access-phase4.md:670 — Runtime-fetched skills bypass host-side security.InputPipeline().Scan(). Documented as a future enhancement with compensating controls (allowlist + integrity hash + SSRF hardening).

Previous run (8)

Review

Findings

Low

[algorithm-logic-consistency] docs/plans/universal-harness-access-phase4.md:222 — The fullsend-fetch-skill script pseudocode has a timeout logic error: ELAPSED increments by 1 each iteration but the loop sleeps for 0.2 seconds, so the loop runs 120 iterations × 0.2s = 24 seconds actual wall-clock timeout, not the documented 120 seconds. The error message also misleadingly prints ${MAX_WAIT_SECONDS}s. This is plan documentation (not shipped code) and should be corrected when implementing PR 2.
[edge-case] docs/plans/universal-harness-access-phase4.md:217 — The script pseudocode constructs JSON using printf with raw string interpolation (printf '{"url":"%s"}'). If a URL contains double-quote, backslash, or newline characters, the JSON would be malformed. Should be addressed when implementing the actual script in PR 2.
[design-coherence] internal/resolve/resolve.go — ResolveSkillURL places enforcement responsibility (rate limiting, AllowRuntimeFetch gating) on the caller rather than handling limits internally like ResolveHarness does with ResolveOpts. The doc comment explicitly states callers must gate on h.AllowRuntimeFetch and enforce MaxRuntimeFetches. This is an intentional design choice for a single-URL building block — PR 3's handler goroutine is the intended enforcement point.
[stale-doc] docs/ADRs/0038-universal-harness-access.md:176 — ADR-0038 describes runtime dependency loading mitigations in present tense as if fully implemented. The statements are partially true now (opt-in via allow_runtime_fetch, validation/caching via ResolveSkillURL) and will be fully true after PR 3. Consider clarifying implementation status.
[function-documentation] internal/resolve/resolve.go:964 — The ResolveSkillURL doc comment uses imperative language ("Callers must gate on...") which differs from the declarative style used by ResolveHarness. Consider rephrasing to: "Does not check h.AllowRuntimeFetch or enforce rate limits — the caller must handle these checks."
[validation-error-messages] internal/harness/harness.go:869 — Error message max_runtime_fetches requires allow_runtime_fetch: true uses a different pattern from existing validation messages (compare: validation_loop.script is required when validation_loop is set). The current message is clear and unambiguous; consider aligning phrasing for consistency.
[missing-authorization] N/A — This PR has no linked issue. The changes are well-documented via ADR-0038 and the Phase 4 implementation plan, and they follow the established multi-phase pattern (Phases 1-3). Consider creating a tracking issue for Phase 4 implementation.

Previous run (9)

Review

Findings

Medium

[stale-doc] docs/plans/universal-harness-access.md:335 — Documentation states that runtime dependency loading is "Not implemented in initial design" and describes Phase 4 as "future", but this PR begins Phase 4 implementation by adding AllowRuntimeFetch and MaxRuntimeFetches schema fields to internal/harness/harness.go and the ResolveSkillURL function to internal/resolve/resolve.go. The section at line 362 titled "Phase 2: Runtime fetch with policy (future)" also describes these fields as future features when they are now implemented with validation.
Remediation: Update status indicators and section headers in docs/plans/universal-harness-access.md to reflect that Phase 4 implementation has begun and the allow_runtime_fetch/max_runtime_fetches schema fields are now accepted.

Low

[missing-authorization] N/A — This PR has no linked issue. The changes are well-documented via ADR-0038 and the Phase 4 implementation plan added in this PR, and they follow the established multi-phase pattern (Phases 1-3). Consider creating a tracking issue for Phase 4 implementation.
[algorithm-logic-consistency] docs/plans/universal-harness-access-phase4.md:222 — The fullsend-fetch-skill script pseudocode has a timeout logic error: ELAPSED increments by 1 each iteration but the loop sleeps for 0.2 seconds, resulting in a 24-second actual timeout instead of the documented 120 seconds. This is in plan documentation (not shipped code) and should be corrected when implementing PR 2.
[edge-case] docs/plans/universal-harness-access-phase4.md:217 — The script pseudocode constructs JSON using printf with raw string interpolation. If a URL contains double-quote, backslash, or newline characters, the JSON would be malformed. Should be addressed when implementing the actual script in PR 2.
[pattern-inconsistency] internal/harness/harness.go — The validation for MaxRuntimeFetches=0 meaning "use default" (rather than "disabled") is non-obvious from the code alone, though documented in the Phase 4 plan. Consider adding a clarifying comment.
[authorization] internal/resolve/resolve.go:317 — ResolveSkillURL does not check h.AllowRuntimeFetch before proceeding. This follows the existing pattern where resolveURL also does not check feature flags. The doc comment now explicitly states callers must gate on AllowRuntimeFetch.
[denial-of-service] internal/resolve/resolve.go:317 — ResolveSkillURL has no resource-counting mechanism. Rate limiting is deferred to the runtimefetch.Run() handler goroutine in PR 3, consistent with how resolveURL delegates resource counting to resolveState.
[design-coherence] internal/resolve/resolve.go — ResolveSkillURL places enforcement responsibility on the caller rather than handling limits internally like ResolveHarness does with ResolveOpts. The difference is justified by the different use cases (single URL vs. full graph resolution).
[scope-mismatch] docs/plans/universal-harness-access-phase4.md — The PR title says "Phase 4 PR 1" but includes both Phase 3 and Phase 4 plan documents. The PR body already clarifies this, but consider updating the title.

Previous run (10)

Review

Findings

Low

[authorization] internal/resolve/resolve.go:308 — ResolveSkillURL does not check h.AllowRuntimeFetch before proceeding. This follows the existing pattern (resolveURL also does not check feature flags — the caller handles orchestration), and the function is in an internal package. Consider adding a doc comment noting that callers must gate on AllowRuntimeFetch.
[edge-case] internal/harness/harness.go:213 — When AllowRuntimeFetch is true and MaxRuntimeFetches is 0 (zero-value default), validation passes. The Phase 4 plan doc explicitly documents this as intentional: the default of 10 is applied at point of use in PR 3. Consider adding a DefaultMaxRuntimeFetches constant for clarity.
[denial-of-service] internal/resolve/resolve.go:308 — ResolveSkillURL has no resource-counting mechanism. Per the Phase 4 plan, rate limiting is enforced by the runtimefetch.Run() handler goroutine (PR 3), consistent with how resolveURL delegates resource counting to resolveState.

Info

[consumer-completeness] internal/harness/harness.go:213 — The new AllowRuntimeFetch and MaxRuntimeFetches fields are added with validation, and ResolveSkillURL is exported, but no caller invokes them yet. Expected for a schema-first PR (Phase 4 PR 1 of 3).

fullsend-ai-review · 2026-06-10T01:05:59Z

🤖 Finished Review · ✅ Success · Started 1:05 AM UTC · Completed 1:16 AM UTC
Commit: 4ed6da4 · View workflow run →

fullsend-ai-review · 2026-06-10T02:04:32Z

🤖 Finished Review · ✅ Success · Started 2:04 AM UTC · Completed 2:20 AM UTC
Commit: 4ed6da4 · View workflow run →

fullsend-ai-review · 2026-06-10T03:15:24Z

🤖 Finished Review · ✅ Success · Started 3:15 AM UTC · Completed 3:28 AM UTC
Commit: 4ed6da4 · View workflow run →

fullsend-ai-review · 2026-06-10T11:32:51Z

🤖 Finished Review · ✅ Success · Started 11:32 AM UTC · Completed 11:46 AM UTC
Commit: 4ed6da4 · View workflow run →

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review · 2026-06-11T03:07:56Z

🤖 Review · Started 3:07 AM UTC
Commit: 2f4c858 · View workflow run →

fullsend-ai-review · 2026-06-11T03:14:42Z

🤖 Finished Review · ✅ Success · Started 3:14 AM UTC · Completed 3:28 AM UTC
Commit: 820c2d4 · View workflow run →

When a GitHub Actions workflow is cancelled (cancel-in-progress), the status comment was left showing "Started" permanently. Two bugs caused this: 1. Go's default SIGINT handler calls os.Exit(2), which skips deferred cleanup. Fix: use signal.NotifyContext so SIGINT/SIGTERM cancel the context and defers run. 2. The root command used Execute() instead of ExecuteContext(), so cmd.Context() returned context.Background() — ctx.Err() was never non-nil and the "cancelled" status path was unreachable. Fix: thread the signal-aware context through ExecuteContext(). 3. The cancelled status path deleted the start comment instead of updating it. Fix: remove the special-case deletion so cancellation goes through the same update-in-place logic as success/failure, showing "⚠️ Cancelled" in the comment. Fixes fullsend-ai#2104 Signed-off-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>

rh-hemartin · 2026-06-11T06:40:04Z

Looks like this is including more changes than intended... is that right? It is re-adding files that were deleted for example. Please fix it.

github-actions · 2026-06-11T10:59:43Z

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

fullsend-ai-review · 2026-06-11T11:01:50Z

🤖 Finished Review · ✅ Success · Started 11:01 AM UTC · Completed 11:17 AM UTC
Commit: 9468768 · View workflow run →

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review · 2026-06-11T11:16:59Z

@@ -57,6 +57,8 @@ func TestFullsendRepoFilesExist(t *testing.T) {
 		".github/workflows/review.yml",


[medium] missing-test-entry

The new composite action .github/actions/mint-token/action.yml is added to the scaffold directory but is not included in the TestFullsendRepoFilesExist expected file list. Only setup-gcp/action.yml and validate-enrollment/action.yml were added.

Suggested fix: Add .github/actions/mint-token/action.yml to the expected slice in TestFullsendRepoFilesExist.

Fixed — added .github/actions/mint-token/action.yml to the expected file list in TestFullsendRepoFilesExist (413e2cb).

fullsend-ai-review · 2026-06-11T11:16:59Z


-**Dispatch workflows:** The scaffold `dispatch.yml` (at `internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml`) and the repo's `reusable-dispatch.yml` (at `.github/workflows/reusable-dispatch.yml`) share identical routing logic for different installation modes (per-org vs per-repo). When changing the jq payload construction, stage routing, or input/secret threading in one, apply the same change to the other.
-
 **Forge abstraction:** All git forge operations must go through the `forge.Client` interface in `internal/forge/forge.go`. Do not use `exec.Command("gh", ...)` or direct GitHub API calls outside `internal/forge/github/`. See [AGENTS.md](AGENTS.md#forge-abstraction) for details.


[medium] removed-guidance-still-valid

The diff removes the Dispatch workflows paragraph that instructs contributors to keep dispatch.yml and reusable-dispatch.yml in sync. Both files still exist and share routing logic. Removing this guidance without relocating it increases the risk of drift.

Suggested fix: Either keep the paragraph in CLAUDE.md, or move it to a more appropriate location.

Fixed — restored the dispatch workflows paragraph in CLAUDE.md. It was incorrectly removed during rebase conflict resolution (413e2cb).

fullsend-ai-review · 2026-06-11T11:16:59Z

@@ -290,6 +292,12 @@ func (h *Harness) Validate() error {
 	if h.ValidationLoop != nil && h.ValidationLoop.Script == "" {


[low] missing-validation

Validate() does not check whether allow_runtime_fetch: true is accompanied by a non-empty allowed_remote_resources list. ResolveSkillURL has a runtime guard but explicit validation at config load time would give a better error message.

Acknowledged — ResolveSkillURL checks the allowlist at runtime (line 489), which rejects URLs when allowed_remote_resources is empty. Adding a validation-time check would give a better error message but is lower priority since the runtime guard catches it. Can add in a follow-up if needed.

fullsend-ai-review · 2026-06-11T11:16:59Z

+// fetch. Like static skill resolution, it uses the forge API to list and fetch
+// the skill directory contents, verifies the tree hash, and caches the result.
+// The audit entry uses FetchType "runtime" to distinguish from static resolution.
+// No transitive resolution — runtime-fetched skills are leaf nodes.


[low] error-message-consistency

Error message prefix runtime: differs from existing field-based error prefixes in the same file.

Fixed — changed all error prefixes in ResolveSkillURL from ad-hoc "runtime: " to field-based "runtime_skill: ", consistent with the field parameter pattern used by resolveURL and resolveSkillDirURL (413e2cb).

…PR 1) Add allow_runtime_fetch and max_runtime_fetches harness fields with validation, and export ResolveSkillURL for single-URL runtime skill resolution with SSRF protection, integrity verification, caching, and "runtime" audit logging. Includes Phase 3/4 implementation plan docs and cross-references from Phase 1/2 plans. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Greg Allen <gallen@redhat.com>

fullsend-ai-review · 2026-06-11T11:28:43Z

🤖 Finished Review · ✅ Success · Started 11:28 AM UTC · Completed 11:48 AM UTC
Commit: 413e2cb · View workflow run →

ggallen · 2026-06-11T14:17:32Z

Looks like this is including more changes than intended... is that right? It is re-adding files that were deleted for example. Please fix it.

This is what I get for working late at night. I'm going to close this and open an entirely new PR. Thanks for catching it, @rh-hemartin.

ggallen · 2026-06-11T14:17:51Z

Needs a complete rewrite based on other changes. Closing.

fullsend-ai-retro · 2026-06-11T14:21:15Z

🤖 Finished Retro · ✅ Success · Started 2:21 PM UTC · Completed 2:26 PM UTC
Commit: 413e2cb · View workflow run →

fullsend-ai-retro · 2026-06-11T14:26:27Z

Retro: PR #2104 — closed without merge after 10 review cycles

What happened: PR #2104 (feat(resolve): add runtime fetch schema and ResolveSkillURL) was opened by ggallen on Jun 10 and closed without merge on Jun 11 after ~38 hours. The PR bundled ~150 lines of in-scope Phase 4 work with ~2350 lines of unrelated changes (binary package deletion, doc migration, scaffold actions, merge-queue cleanup). The review bot ran 10 times, flagging "scope-tier-mismatch" as a medium-severity finding but never blocking on it. A human reviewer (rh-hemartin) caught the real blocker: the PR was re-adding files that had been deleted in other merged PRs, indicating a stale branch. The author closed the PR and planned to reopen with a clean scope.

Token cost: 10 review dispatches on a PR that was ultimately discarded — significant wasted compute.

Existing issues that cover most improvement areas:

#1741 — detect stale-branch merge artifacts (would have caught the re-added deleted files)
#1013 — deduplicate findings across review iterations
#2111 — detect review-cycle churn and suggest approach reassessment

One new proposal below for escalating scope-tier-mismatch findings to blocking severity when the scope ratio is extreme.

Proposals filed

Escalate scope-tier-mismatch to CHANGES_REQUESTED when out-of-scope changes dominate (in fullsend-ai/fullsend)

ggallen · 2026-06-11T14:38:39Z

Superseded by #2173.

github-actions Bot deployed to site-preview June 10, 2026 00:12 View deployment

fullsend-ai-retro Bot mentioned this pull request Jun 10, 2026

feat(resolve): add runtime fetch schema and ResolveSkillURL (Phase 4 PR 1) #2103

Closed

4 tasks

fullsend-ai-review Bot approved these changes Jun 10, 2026

View reviewed changes

Comment thread internal/resolve/resolve.go

Comment thread internal/harness/harness.go Outdated

Comment thread internal/resolve/resolve.go

Comment thread internal/harness/harness.go Outdated

fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jun 10, 2026

ggallen force-pushed the feat/adr-0038-phase4-pr1 branch from dabf824 to e4ff7ad Compare June 10, 2026 01:04

github-actions Bot deployed to site-preview June 10, 2026 01:05 View deployment

fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed ready-for-merge All reviewers approved — ready to merge labels Jun 10, 2026

ggallen force-pushed the feat/adr-0038-phase4-pr1 branch from e4ff7ad to 0b29673 Compare June 10, 2026 01:58

github-actions Bot deployed to site-preview June 10, 2026 02:00 View deployment

fullsend-ai-review Bot approved these changes Jun 10, 2026

View reviewed changes

Comment thread docs/plans/universal-harness-access-phase4.md Outdated

Comment thread docs/plans/universal-harness-access-phase4.md Outdated

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Jun 10, 2026

ggallen force-pushed the feat/adr-0038-phase4-pr1 branch from 0b29673 to 752360c Compare June 10, 2026 03:13

github-actions Bot deployed to site-preview June 10, 2026 03:15 View deployment

fullsend-ai-review Bot approved these changes Jun 10, 2026

View reviewed changes

Comment thread internal/resolve/resolve.go Outdated

Comment thread docs/plans/universal-harness-access-phase4.md Outdated

fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Jun 10, 2026

ggallen force-pushed the feat/adr-0038-phase4-pr1 branch from 752360c to e0eac6a Compare June 10, 2026 11:31

github-actions Bot deployed to site-preview June 10, 2026 11:32 View deployment

fullsend-ai-review Bot suggested changes Jun 10, 2026

View reviewed changes

Comment thread internal/resolve/resolve.go Outdated

Comment thread internal/harness/harness.go

Comment thread internal/harness/harness.go Outdated

Comment thread internal/resolve/resolve.go

fullsend-ai-review Bot removed the ready-for-merge All reviewers approved — ready to merge label Jun 10, 2026

ggallen force-pushed the feat/adr-0038-phase4-pr1 branch from e0eac6a to 9666371 Compare June 10, 2026 12:02

ggallen force-pushed the feat/adr-0038-phase4-pr1 branch from 8ac8d39 to 2f4c858 Compare June 11, 2026 03:06

github-actions Bot deployed to site-preview June 11, 2026 03:07 View deployment

ggallen force-pushed the feat/adr-0038-phase4-pr1 branch from 2f4c858 to 820c2d4 Compare June 11, 2026 03:12

github-actions Bot deployed to site-preview June 11, 2026 03:14 View deployment

fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 11, 2026

ggallen mentioned this pull request Jun 11, 2026

fix: update status comment on cancellation instead of orphaning it #2146

Merged

3 tasks

ggallen force-pushed the feat/adr-0038-phase4-pr1 branch from 820c2d4 to 9468768 Compare June 11, 2026 10:59

github-actions Bot deployed to site-preview June 11, 2026 11:01 View deployment

fullsend-ai-review Bot suggested changes Jun 11, 2026

View reviewed changes

fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Jun 11, 2026

ggallen force-pushed the feat/adr-0038-phase4-pr1 branch from 9468768 to 413e2cb Compare June 11, 2026 11:26

github-actions Bot deployed to site-preview June 11, 2026 11:27 View deployment

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 11, 2026

ggallen closed this Jun 11, 2026

fullsend-ai-retro Bot mentioned this pull request Jun 11, 2026

Escalate scope-tier-mismatch to CHANGES_REQUESTED when out-of-scope changes dominate #2170

Open

github-actions Bot deleted the feat/adr-0038-phase4-pr1 branch July 12, 2026 05:47

		@@ -57,6 +57,8 @@ func TestFullsendRepoFilesExist(t *testing.T) {
		".github/workflows/review.yml",


		Dispatch workflows: The scaffold `dispatch.yml` (at `internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml`) and the repo's `reusable-dispatch.yml` (at `.github/workflows/reusable-dispatch.yml`) share identical routing logic for different installation modes (per-org vs per-repo). When changing the jq payload construction, stage routing, or input/secret threading in one, apply the same change to the other.

		Forge abstraction: All git forge operations must go through the `forge.Client` interface in `internal/forge/forge.go`. Do not use `exec.Command("gh", ...)` or direct GitHub API calls outside `internal/forge/github/`. See [AGENTS.md](AGENTS.md#forge-abstraction) for details.

		@@ -290,6 +292,12 @@ func (h *Harness) Validate() error {
		if h.ValidationLoop != nil && h.ValidationLoop.Script == "" {

Uh oh!

Conversation

ggallen commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes from rebase on #2139

Test plan

Uh oh!

github-actions Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

fullsend-ai-review Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Medium

Low

Info

Review

Findings

High

Medium

Low

Info

Review

Findings

Medium

Low

Info

Review

Findings

Medium

Low

Info

Review

Findings

High

Medium

Low

Info

Review

Findings

Medium

Low

Info

Review

Findings

High

Medium

Low

Info

Review

Findings

Low

Info

Review

Findings

Low

Review

Findings

Medium

Low

Review

Findings

Low

Info

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ggallen commented Jun 10, 2026 •

edited

Loading

github-actions Bot commented Jun 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 10, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 11, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 11, 2026 •

edited

Loading

fullsend-ai-review Bot commented Jun 11, 2026 •

edited

Loading

fullsend-ai-retro Bot commented Jun 11, 2026 •

edited

Loading