Skip to content

refactor(scaffold)!: delete agent files from scaffold embed - #5588

Merged
ggallen merged 1 commit into
fullsend-ai:mainfrom
ggallen:worktree-scaffold-cleanup
Jul 28, 2026
Merged

refactor(scaffold)!: delete agent files from scaffold embed#5588
ggallen merged 1 commit into
fullsend-ai:mainfrom
ggallen:worktree-scaffold-cleanup

Conversation

@ggallen

@ggallen ggallen commented Jul 25, 2026

Copy link
Copy Markdown
Member

Summary

  • Delete all scaffold agent files (agents/, harness/, policies/, schemas/, env/, skills/, plugins/, agent-specific scripts) — these are now served from fullsend-ai/agents at runtime via resolveAgentSource()
  • Retain scripts still invoked by reusable workflow YAML on the GitHub Actions runner:
    • pre-code.sh + test, pre-fix.sh — also duplicated in fullsend-ai/agents where they run via harness pre_script: inside the sandbox (two independent invocation paths per agent run)
    • pre-fetch-prior-review.sh + test — workflow-only, no counterpart in fullsend-ai/agents
    • setup-prioritize.sh — one-time setup script referenced by an error message in the scaffolded prioritize-scheduler.yml template
  • Remove dead migrateModified code path from migrate-customizations command (depended on deleted harness files)
  • Remove hack/lint-agent-docs linter (validated harness doc: fields that no longer exist)
  • Update executableFiles map, tests, Makefile script-test target, .pre-commit-config.yaml, agent docs, and broken symlinks
  • Make HarnessNames() return an empty list instead of erroring when the harness directory is absent (deferred removal per Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 — the removal precondition is now met by this PR's migrate.go changes, but kept as a deliberate scope choice for this already-large PR)
  • Fix stale documentation references across 9 files pointing to deleted scaffold paths

Note: fullsend lock has its own independent resolveHarnessPath in lock.go — updating it to be config/agents-repo-aware is tracked separately (see follow-up issues below).

Addresses #5552

Follow-up work

Test plan

  • go build ./... passes
  • go test ./internal/scaffold/... ./internal/cli/ ./internal/layers/ — all tests pass
  • Pre-commit hooks pass
  • TestFileModeMatchesFilesystem verifies executableFiles matches disk

🤖 Generated with Claude Code

@ggallen
ggallen requested a review from a team as a code owner July 25, 2026 01:14
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 25, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:15 AM UTC · Completed 1:35 AM UTC
Commit: a1c3c45 · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Remove embedded agent assets from scaffold; resolve from fullsend-ai/agents at runtime

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Remove embedded agent/harness/policy assets from the scaffold; resolve them from
 fullsend-ai/agents at runtime.
• Update scaffold tests, Makefile script-test target, and pre-commit hooks for the new
 source-of-truth.
• Make HarnessNames() return an empty list (not an error) when harnesses aren’t embedded.
Diagram

graph TD
  CLI["fullsend CLI"] --> LOCK["lock command"] --> SCAF["internal/scaffold"]
  SCAF --> EMB[("Embedded scaffold files")]
  SCAF --> RES["resolveAgentSource()"] --> AG{{"fullsend-ai/agents"}}
  subgraph Legend
    direction LR
    _mod["Module"] ~~~ _store[("Embedded files")] ~~~ _ext{{"External repo"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Vendor a pinned snapshot (git submodule / subtree)
  • ➕ Builds remain self-contained and offline-friendly
  • ➕ Reproducible content without runtime network access
  • ➖ Reintroduces duplication/maintenance burden
  • ➖ Submodule workflows can be brittle for contributors
2. Ship an asset bundle per release (tarball)
  • ➕ Keeps a single source-of-truth while remaining reproducible
  • ➕ Simplifies pinning content to a CLI version
  • ➖ Requires release artifact plumbing and signature/verification story
  • ➖ Still needs an update mechanism for org overrides
3. Embed only a minimal fallback harness set
  • ➕ Preserves basic functionality when external repo is unavailable
  • ➕ Reduces risk of “empty by default” behavior surprising users
  • ➖ Adds complexity deciding what is “minimal” and keeping it correct
  • ➖ Partial duplication still exists

Recommendation: The PR’s approach (runtime resolution from fullsend-ai/agents, with scaffold no longer embedding agent assets) is the cleanest long-term maintenance strategy and reduces drift. The key follow-up risk to watch is reproducibility/offline behavior; ensuring resolveAgentSource() is pinned (commit SHA) and cached where appropriate will mitigate most of the downsides compared to vendoring snapshots.

Files changed (15) +29 / -439

Bug fix (1) +2 / -1
baseurl.goMake HarnessNames resilient to missing embedded harness directory +2/-1

Make HarnessNames resilient to missing embedded harness directory

• Changes HarnessNames() to return an empty list when the embedded harness directory is absent, instead of returning an error. This allows callers (e.g., fullsend lock) to handle the scaffold cleanup gracefully.

internal/scaffold/baseurl.go

Refactor (1) +4 / -25
scaffold.goTrim executableFiles list after removing agent-related scaffold scripts +4/-25

Trim executableFiles list after removing agent-related scaffold scripts

• Removes now-deleted script entries from the executableFiles map and updates minor commentary to align with remaining embedded script behavior.

internal/scaffold/scaffold.go

Tests (3) +11 / -394
baseurl_test.goRemove tests that assume embedded harness content exists +2/-109

Remove tests that assume embedded harness content exists

• Deletes hash/content-based harness tests and updates HarnessNames expectations to assert no harness templates are embedded. Keeps validation tests for invalid inputs.

internal/scaffold/baseurl_test.go

scaffold_test.goPrune scaffold embed assertions for removed agent assets +6/-282

Prune scaffold embed assertions for removed agent assets

• Removes tests that verified embedded agents/harnesses/schemas/skills and script contents, and adjusts remaining expectations (e.g., managed header tests and YAML-count thresholds) to match the slimmer scaffold embed.

internal/scaffold/scaffold_test.go

vendormanifest_test.goUpdate vendoring assertions to reference remaining embedded assets +3/-3

Update vendoring assertions to reference remaining embedded assets

• Adjusts vendormanifest tests to assert presence/absence using a remaining embedded script path instead of an embedded agent prompt path that was removed.

internal/scaffold/vendormanifest_test.go

Documentation (8) +10 / -9
README.mdDocument agents source-of-truth as fullsend-ai/agents +3/-2

Document agents source-of-truth as fullsend-ai/agents

• Replaces references to internal scaffold harness YAMLs with guidance that default agents live in fullsend-ai/agents and are resolved at runtime.

docs/agents/README.md

code.mdUpdate agent source link to external agents repo +1/-1

Update agent source link to external agents repo

• Changes the “Source” reference from an embedded harness YAML path to the fullsend-ai/agents repository.

docs/agents/code.md

fix.mdUpdate agent source link to external agents repo +1/-1

Update agent source link to external agents repo

• Changes the “Source” reference from an embedded harness YAML path to the fullsend-ai/agents repository.

docs/agents/fix.md

prioritize.mdUpdate agent source link to external agents repo +1/-1

Update agent source link to external agents repo

• Changes the “Source” reference from an embedded harness YAML path to the fullsend-ai/agents repository.

docs/agents/prioritize.md

retro.mdUpdate agent source link to external agents repo +1/-1

Update agent source link to external agents repo

• Changes the “Source” reference from an embedded harness YAML path to the fullsend-ai/agents repository.

docs/agents/retro.md

review.mdUpdate agent source link to external agents repo +1/-1

Update agent source link to external agents repo

• Changes the “Source” reference from an embedded harness YAML path to the fullsend-ai/agents repository.

docs/agents/review.md

triage.mdUpdate agent source link to external agents repo +1/-1

Update agent source link to external agents repo

• Changes the “Source” reference from an embedded harness YAML path to the fullsend-ai/agents repository.

docs/agents/triage.md

review-autonomy-evidence.mdPoint protected-path discussion at new post-review location +1/-1

Point protected-path discussion at new post-review location

• Updates the document’s reference to post-review.sh to note it now lives in fullsend-ai/agents rather than the scaffold embed.

docs/problems/review-autonomy-evidence.md

Other (2) +2 / -10
.pre-commit-config.yamlScope lint-agent-docs hook to docs only +1/-1

Scope lint-agent-docs hook to docs only

• Updates the lint-agent-docs pre-commit hook to only watch docs/agents/, since harness YAMLs are no longer embedded under internal/scaffold/fullsend-repo/harness/.

.pre-commit-config.yaml

MakefileRemove scaffold script tests from script-test target +1/-9

Remove scaffold script tests from script-test target

• Drops script-test invocations for scaffold-embedded agent scripts that were removed, leaving only remaining repo-local script tests (e.g., reconcile-repos, topissues, gitlint-rules). Updates help text accordingly.

Makefile

@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown

Site preview

Preview: https://862fe0fc-site.fullsend-ai.workers.dev

Commit: 8b8c3bb1b41dfc329ea5d26cc8ce0a9699cf7c5d

@qodo-code-review

qodo-code-review Bot commented Jul 25, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 61 rules

Grey Divider


Action required

1. Stale vendored paths test ✓ Resolved 🐞 Bug ≡ Correctness
Description
TestManagedVendoredContentPathsFromEmbed still asserts a vendored path for an embedded agent file
(.defaults/internal/scaffold/fullsend-repo/agents/triage.md) that is no longer part of the
scaffold after this PR, so go test ./... will fail. The vendored-path expectations were updated
elsewhere to use remaining layered files (e.g. scripts/fullsend-check-output), but this test was
not updated.
Code

internal/scaffold/fullsend-repo/agents/triage.md[L1-4]

----
-name: triage
-description: Inspect a GitHub issue, assess information sufficiency, and produce a structured triage decision.
-skills:
Relevance

⭐⭐⭐ High

Updating stale test expectations after scaffold embed/path removals is typically accepted to keep go
test passing.

PR-#1039
PR-#1947

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflows layer test still asserts the old vendored agent path, while scaffold vendoring tests
now assert a different remaining layered file and scaffold tests no longer expect embedded agent
files.

internal/layers/workflows_test.go[619-626]
internal/scaffold/vendormanifest_test.go[163-169]
internal/scaffold/scaffold_test.go[50-66]

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/layers/workflows_test.go` asserts a vendored layered path that no longer exists after removing embedded agent files from the scaffold. This causes repository-wide tests to fail.

### Issue Context
The scaffold’s embedded layered content no longer includes `agents/triage.md`, and the vendoring enumeration/tests have been updated to validate remaining layered content instead.

### Fix Focus Areas
- internal/layers/workflows_test.go[619-626]
- internal/scaffold/vendormanifest_test.go[163-169]

### Suggested fix
- Replace the assertion for `.defaults/internal/scaffold/fullsend-repo/agents/triage.md` with an assertion for a layered path that is still embedded and vendored (for example `.defaults/internal/scaffold/fullsend-repo/scripts/fullsend-check-output`, matching `vendormanifest_test.go`).

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


2. Migrate misclassifies standard agents 🐞 Bug ≡ Correctness
Description
fullsend migrate and related tests treat scaffold.HarnessNames() as the source of truth for
“standard”/first-party harnesses, but this PR makes the embedded harness list empty, causing
modified first-party agents to be misclassified as migrateCustom (instead of migrateModified)
and breaking harness integration tests that assume embedded templates exist. Since migrateModified
still depends on embedded scaffold bytes (scaffold.HarnessContent and HarnessBaseURLWithHash)
for diffs/base URLs, fixing classification also requires switching migrateModified (and tests) to
an appropriate upstream/fixture source rather than the removed embed.
Code

internal/scaffold/baseurl.go[R82-86]

func HarnessNames() ([]string, error) {
	entries, err := fs.ReadDir(content, "fullsend-repo/harness")
	if err != nil {
-		return nil, fmt.Errorf("reading embedded harness directory: %w", err)
+		return nil, nil
	}
Relevance

⭐⭐ Medium

Potentially real migrate/scaffold behavior mismatch, but no close precedent; intent of PR is to
remove embeds.

PR-#2261

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The migrate path builds its standard-agent set from scaffold.HarnessNames(), but scaffold behavior
is changed such that the embedded harness directory is absent/empty, making HarnessNames() yield
an empty set; consequently, planMigrations can no longer recognize first-party agents as standard
and will route them away from migrateModified. At the same time, the migrateModified
implementation continues to load upstream harness bytes via scaffold.HarnessContent and computes
base URLs using scaffold.HarnessBaseURLWithHash, both of which rely on embedded scaffold content
that is now intentionally missing. Separately, internal/harness/scaffold_integration_test.go
iterates over scaffold.HarnessNames() and reads harness YAMLs from the extracted scaffold, while
scaffold tests now assert no harness templates are embedded, demonstrating an explicit mismatch that
will fail tests and any code paths depending on embedded templates.

internal/cli/migrate.go[101-110]
internal/cli/migrate.go[308-377]
internal/cli/migrate.go[444-456]
internal/scaffold/baseurl_test.go[92-96]
internal/scaffold/baseurl.go[36-49]
internal/harness/scaffold_integration_test.go[121-147]
internal/scaffold/baseurl.go[79-86]

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

## Issue description
`fullsend migrate` (and some integration tests) currently rely on embedded scaffold harness templates via `scaffold.HarnessNames()`/`HarnessContent`, but this PR makes the embedded harness directory absent/empty. This causes (1) modified first-party agents to be misclassified as `migrateCustom` instead of `migrateModified`, changing migration output semantics, and (2) harness integration tests that load/validate embedded harness YAMLs to fail. Because `migrateModified` still depends on embedded harness bytes and base-URL hashing, fixing classification must be paired with updating the upstream source used for diffs/base URL computation (or guarding/temporarily disabling `migrateModified` when no upstream source is available).

## Issue Context
- `runMigrateCustomizations` builds a `scaffoldSet` from `scaffold.HarnessNames()` and `planMigrations` uses membership in that set to select `migrateModified`; with embeds removed, this set becomes empty and first-party agents are treated as custom.
- The `migrateModified` path (`buildModifiedAgentFiles`) still loads upstream harness content from the scaffold embed (`scaffold.HarnessContent`) and builds base URLs via `scaffold.HarnessBaseURLWithHash`, which relies on embedded harness bytes.
- Scaffold behavior/tests now explicitly expect no embedded harness templates, but `internal/harness/scaffold_integration_test.go` still requires a non-empty harness list from `scaffold.HarnessNames()` and reads harness YAMLs from the extracted scaffold.
- `HarnessNames()` currently returns an empty list when the embedded harness directory is missing; error handling should avoid silently masking unexpected embed/fs failures.

## Fix Focus Areas
- internal/cli/migrate.go[101-110]
- internal/cli/migrate.go[308-377]
- internal/cli/migrate.go[417-457]
- internal/scaffold/baseurl.go[36-63]
- internal/scaffold/baseurl.go[79-98]
- internal/harness/scaffold_integration_test.go[17-260]
- internal/scaffold/baseurl_test.go[92-96]

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



Remediation recommended

3. Org-specific link in core 📘 Rule violation § Compliance
Description
A core problem doc (outside docs/problems/applied/) now includes an organization-specific
reference (fullsend-ai/agents). This violates the requirement to keep core problem docs generic
and move org-specific details into the applied hierarchy.
Code

docs/problems/review-autonomy-evidence.md[156]

+The existing protected-path downgrade in `post-review.sh` (now in [`fullsend-ai/agents`](https://github.com/fullsend-ai/agents)) prevents autonomous approval for PRs touching sensitive file paths. This is a path-based autonomy gate.
Relevance

⭐⭐⭐ High

They’ve previously accepted removing/generalizing org-specific references from core problem docs.

PR-#3064
PR-#12

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062038 requires that organization-specific identifiers not appear in core problem
documents outside docs/problems/applied/. The modified line includes a direct fullsend-ai/agents
link within a core problem doc, introducing an org-specific reference inline.

Rule 1062038: Separate generic and organization-specific problem documents
docs/problems/review-autonomy-evidence.md[154-160]

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

## Issue description
A core problem doc now contains an organization-specific identifier/link (`fullsend-ai/agents`) instead of keeping org-specific details under `docs/problems/applied/<org-name>/`.

## Issue Context
Core problem docs should remain generic; organization-specific references (org/repo names, internal workflows) should live in `docs/problems/applied/...` and be linked from the core doc.

## Fix Focus Areas
- docs/problems/review-autonomy-evidence.md[154-160]

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



Informational

4. Problem doc lacks multiple options 📘 Rule violation ⌂ Architecture
Description
docs/problems/review-autonomy-evidence.md is a modified problem document but it does not present
at least two distinct solution options with clearly labeled trade-offs, as required. This makes the
document non-compliant with the problem-doc format standard and reduces its usefulness for future
decision-making.
Code

docs/problems/review-autonomy-evidence.md[156]

+The existing protected-path downgrade in `post-review.sh` (now in [`fullsend-ai/agents`](https://github.com/fullsend-ai/agents)) prevents autonomous approval for PRs touching sensitive file paths. This is a path-based autonomy gate.
Relevance

⭐ Low

Team has rejected adding formal Options/trade-offs sections in problem docs despite the rule.

PR-#3063
PR-#2549

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062035 applies to modified files under docs/problems/ and requires at least two
options with trade-offs. The referenced problem document contains sections like ## The problem and
## Evidence corpus but no option/trade-off structure (and no "Option" sections), so it fails the
rule despite being modified in this PR.

Rule 1062035: Problem docs must present multiple options with trade-offs, not a single prescribed solution
docs/problems/review-autonomy-evidence.md[1-21]
docs/problems/review-autonomy-evidence.md[122-170]

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

## Issue description
`docs/problems/review-autonomy-evidence.md` is under `docs/problems/` and was modified in this PR, but it does not describe multiple solution options/approaches with explicit trade-offs.

## Issue Context
The compliance rule requires problem documents to document at least two distinct options and their trade-offs; a recommendation is allowed only if alternatives are still described.

## Fix Focus Areas
- docs/problems/review-autonomy-evidence.md[11-21]
- docs/problems/review-autonomy-evidence.md[154-170]

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


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread docs/problems/review-autonomy-evidence.md
Comment thread internal/scaffold/fullsend-repo/agents/triage.md
Comment thread internal/scaffold/baseurl.go
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review, skills/cutting-releases/pre-flight.md, images/code/scan-secrets — This PR modifies protected paths. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [dead-code] internal/scaffold/baseurl.go:26HarnessContentHash, HarnessBaseURLWithHash, and HarnessContent will always error since the embedded harness YAML files are deleted. HarnessNames now returns (nil, nil) instead of an error. All callers are removed in this PR, so these functions are dead code. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [error-handling] internal/scaffold/baseurl.go:83HarnessNames() returns (nil, nil) instead of a wrapped error when the harness directory is absent. This matches the established codebase idiom (ComparePathPresence uses the same pattern). Planned for removal per issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5.

  • [scope-authorization-mismatch] PR metadata — Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 ("Remove HarnessNames()/HarnessBaseURLWithHash() after migrate.go no longer needs them") is not completed in this PR, though the prerequisite (removing the migrate.go callers) is now met. Intentional phasing — reasonable scope management for a 113-file PR.

  • [documentation] internal/harness/harness.go:271 — The Doc field inline comment (// source-repo-only; not resolved at runtime, used by lint-agent-docs) was removed. The lint-agent-docs reference is stale (script deleted), but the field remains in the struct. Consider adding a comment documenting the field's current purpose if it is still used by docs/agents/ pages.

Previous run

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review, skills/cutting-releases/pre-flight.md, images/code/scan-secrets — This PR modifies protected paths. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [dead-code] internal/scaffold/baseurl.go:26HarnessContentHash, HarnessBaseURLWithHash, and HarnessContent will always error since the embedded harness YAML files are deleted. HarnessNames now returns (nil, nil) instead of an error. All callers are removed in this PR, so these functions are dead code. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [error-handling] internal/scaffold/baseurl.go:83HarnessNames() returns (nil, nil) instead of a wrapped error when the harness directory is absent. This matches the established codebase idiom (ComparePathPresence uses the same pattern). Planned for removal per issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5.

  • [scope-authorization-mismatch] PR metadata — Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 ("Remove HarnessNames()/HarnessBaseURLWithHash() after migrate.go no longer needs them") is not completed in this PR, though the prerequisite (removing the migrate.go callers) is now met. Intentional phasing — reasonable scope management for a 113-file PR.

Previous run (2)

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review, skills/cutting-releases/pre-flight.md — This PR modifies protected paths. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [dead-code] internal/scaffold/baseurl.go:26HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL will always return errors since the embedded harness directory is now empty. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [error-handling] internal/scaffold/baseurl.go:83HarnessNames() returns (nil, nil) instead of a wrapped error when the harness directory is absent. This matches the established codebase idiom (LoadProviderDefs, ComparePathPresence use the same pattern). Planned for removal per issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5.

  • [scope-authorization-mismatch] PR metadata — Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 ("Remove HarnessNames()/HarnessBaseURLWithHash() after migrate.go no longer needs them") is not completed in this PR, though the prerequisite (removing the migrate.go callers) is now met. Intentional phasing — reasonable scope management for a 118-file PR.

Previous run (3)

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review, skills/cutting-releases/pre-flight.md — This PR modifies protected paths. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [dead-code] internal/scaffold/baseurl.go:26HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL will always return errors since the embedded harness directory is now empty. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [error-handling] internal/scaffold/baseurl.go:83HarnessNames() returns (nil, nil) instead of a wrapped error when the harness directory is absent. This matches the established codebase idiom (LoadProviderDefs, ComparePathPresence use the same pattern). Planned for removal per issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5.

  • [scope-authorization-mismatch] PR metadata — Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 ("Remove HarnessNames()/HarnessBaseURLWithHash() after migrate.go no longer needs them") is not completed in this PR, though the prerequisite (removing the migrate.go callers) is now met. Intentional phasing — reasonable scope management for a 118-file PR.

  • [stale-doc] docs/plans/ (multiple files) — Plan documents, design specs, and ADRs reference deleted scaffold paths (internal/scaffold/fullsend-repo/harness/, DiffHarness, etc.). These are historical records that were correct at writing time; not actionable as they preserve useful context about what was changed.

Previous run (4)

Review

Findings

High

  • [missing-breaking-change-justification] PR metadata — PR title includes breaking change marker ! (refactor(scaffold)!:) but the PR body contains no BREAKING CHANGE: trailer. Per COMMITS.md, both the ! suffix and a BREAKING CHANGE: trailer explaining what breaks and how to migrate are required.
    Remediation: Add a BREAKING CHANGE: trailer to the PR body (or squash-merge commit body) explaining what breaks, who is affected, and migration steps.

Medium

  • [protected-path] .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review, skills/cutting-releases/pre-flight.md — This PR modifies protected paths. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [dead-code] internal/scaffold/baseurl.go:26HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL will always return errors since the embedded harness directory is now empty. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [stale-doc] docs/plans/adr-0045-forge-portable-harness-phase1.md, docs/plans/adr-0045-forge-portable-harness-phase2.md, docs/plans/agent-extraction-to-agents-repo.md, docs/plans/deprecate-customized-directory-overlay.md, docs/plans/universal-harness-access.md — Plan documents reference deleted scaffold paths (internal/scaffold/fullsend-repo/harness/, internal/layers/harnesswrappers.go, scaffold scripts). These are historical records and the references were correct at writing time; not actionable as they preserve useful context about what was changed.

  • [go-anti-pattern] internal/scaffold/baseurl.go:84HarnessNames() returns (nil, nil) instead of a wrapped error when the harness directory is absent. While (nil, nil) is valid Go for "no results, no error", the function is planned for removal per issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5.

  • [scope-authorization-mismatch] PR metadata — Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 ("Remove HarnessNames()/HarnessBaseURLWithHash() after migrate.go no longer needs them") is not completed in this PR, though the prerequisite (removing the migrate.go callers) is now met. Intentional phasing — reasonable scope management.

Previous run (5)

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review, skills/cutting-releases/pre-flight.md — This PR modifies protected paths. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [dead-code] internal/scaffold/baseurl.go:26HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL will always return errors since the embedded harness directory is now empty. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [stale-reference] docs/plans/agent-registration.md:466 — States that migrate-customizations "uses DiffHarness to compute minimal base: composition harnesses." DiffHarness and the migrateModified code path are removed by this PR. The plan document was not updated.
    Remediation: Update the paragraph to reflect that DiffHarness and the migrateModified path have been removed.

Previous run (6)

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review, skills/cutting-releases/pre-flight.md — This PR modifies protected paths. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

  • [stale-doc] docs/ADRs/0058-agent-registration.md:25 — ADR Context section states "Scaffold-embedded harnesses (internal/scaffold/fullsend-repo/harness/) define the complete agent set" and references HarnessNames() enumerating them. After this PR, both statements are incorrect.
    Remediation: Update the Context section to reflect that agents are now resolved from the fullsend-ai/agents repo at runtime.

Low

  • [dead-code] internal/scaffold/baseurl.go:26HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL will always return errors since the embedded harness directory is now empty. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [stale-doc] docs/ADRs/0045-forge-portable-harness-schema.md:378 — Example base URLs reference internal/scaffold/fullsend-repo/harness/triage.yaml (lines 378, 413) which no longer exists. Line 595 also references the deleted DiffHarness function.

  • [stale-doc] docs/ADRs/0032-safe-push-wrapper-for-sandboxed-agents.md:30 — References post-code.sh as a scaffold script, but this has been deleted from the scaffold (now in fullsend-ai/agents).

  • [stale-doc] docs/ADRs/0053-agent-driven-branch-targeting.md:24 — References post-code.sh in scaffold context (lines 24, 115). Decision section instructs changes to a script no longer in the scaffold.

  • [stale-doc] docs/superpowers/specs/2026-06-22-autonomy-readiness-skill-design.md:135 — States the skill file is at internal/scaffold/fullsend-repo/skills/autonomy-readiness/SKILL.md but the skills/ directory has been deleted from the scaffold.

  • [stale-doc] docs/superpowers/specs/2026-06-11-triage-prerequisites-design.md:135 — Design spec references deleted scaffold paths (internal/scaffold/fullsend-repo/schemas/triage-result.schema.json, agents/triage.md, scripts/post-triage.sh, scripts/pre-triage.sh).

  • [stale-doc] docs/superpowers/specs/2026-06-11-review-agent-contextual-labels-design.md:21 — Design spec references multiple deleted scaffold paths including skills/issue-labels/SKILL.md, harness/review.yaml, agents/review.md, schemas/review-result.schema.json, and scripts/post-review.sh.

  • [stale-doc] docs/ADRs/0047-vendored-installs-with-vendor-flag.md:90 — States layered agent content is copied from .defaults/internal/scaffold/fullsend-repo/ including directories that have been deleted from the scaffold.

Previous run (7)

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review, skills/cutting-releases/pre-flight.md — This PR modifies protected paths. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

  • [stale-doc] docs/ADRs/0058-agent-registration.md:25 — ADR Context section states "Scaffold-embedded harnesses (internal/scaffold/fullsend-repo/harness/) define the complete agent set" and references HarnessNames() enumerating them. After this PR, both statements are incorrect — the harness directory is empty and HarnessNames() returns an empty list.
    Remediation: Update the Context section to reflect that agents are now resolved from the fullsend-ai/agents repo at runtime.

Low

  • [dead-code] internal/scaffold/baseurl.go:26HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL will always return errors since the embedded harness directory is now empty. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [stale-doc] docs/ADRs/0045-forge-portable-harness-schema.md:378 — Example base URLs reference internal/scaffold/fullsend-repo/harness/triage.yaml (lines 378, 413) which no longer exists. Line 595 also references the deleted DiffHarness function.

  • [stale-doc] docs/ADRs/0032-safe-push-wrapper-for-sandboxed-agents.md:30 — References post-code.sh as a scaffold script, but this has been deleted from the scaffold (now in fullsend-ai/agents).

  • [stale-doc] docs/ADRs/0053-agent-driven-branch-targeting.md:24 — References post-code.sh in scaffold context (lines 24, 115). Decision section instructs changes to a script no longer in the scaffold.

  • [stale-doc] docs/superpowers/specs/2026-06-22-autonomy-readiness-skill-design.md:135 — States the skill file is at internal/scaffold/fullsend-repo/skills/autonomy-readiness/SKILL.md but the skills/ directory has been deleted from the scaffold.

  • [stale-doc] docs/superpowers/specs/2026-06-11-triage-prerequisites-design.md:135 — Design spec references deleted scaffold paths (internal/scaffold/fullsend-repo/schemas/triage-result.schema.json, agents/triage.md, scripts/post-triage.sh, scripts/pre-triage.sh).

  • [stale-doc] docs/superpowers/specs/2026-06-11-review-agent-contextual-labels-design.md:21 — Design spec references multiple deleted scaffold paths including skills/issue-labels/SKILL.md, harness/review.yaml, agents/review.md, schemas/review-result.schema.json, and scripts/post-review.sh.

  • [stale-doc] docs/ADRs/0047-vendored-installs-with-vendor-flag.md:90 — States layered agent content is copied from .defaults/internal/scaffold/fullsend-repo/ including directories that have been deleted from the scaffold.

Previous run (8)

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review, skills/cutting-releases/pre-flight.md — This PR modifies protected paths. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

  • [stale-reference] docs/guides/dev/cli-internals.md:168 — The migration-actions table still lists a "Modified" row referencing DiffHarness and the paragraph at line 170 describes the deleted diff engine (internal/harness/diff.go). Both migrateModified and diff.go are removed by this PR, but these documentation lines were not updated. The PR already updates other sections of the same file (line 607+ for executableFiles).
    Remediation: Remove the "Modified" row from the table and the paragraph describing the diff engine.

Low

  • [dead-code] internal/scaffold/baseurl.go:26HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL will always return errors since the embedded harness directory is now empty. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [stale-doc] docs/superpowers/specs/2026-06-11-triage-prerequisites-design.md:135 — Design spec references deleted scaffold paths (internal/scaffold/fullsend-repo/schemas/triage-result.schema.json, agents/triage.md, scripts/post-triage.sh, scripts/pre-triage.sh).

  • [stale-doc] docs/superpowers/specs/2026-06-11-review-agent-contextual-labels-design.md:21 — Design spec references multiple deleted scaffold paths including skills/issue-labels/SKILL.md, harness/review.yaml, agents/review.md, schemas/review-result.schema.json, and scripts/post-review.sh.

  • [stale-doc] docs/ADRs/0047-vendored-installs-with-vendor-flag.md:90 — States layered agent content is copied from .defaults/internal/scaffold/fullsend-repo/ including directories that have been deleted from the scaffold.

Previous run (9)

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml — This PR modifies protected paths: .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [stale-reference] internal/harness/harness.go:271 — The Doc field's inline comment says "used by lint-agent-docs", but hack/lint-agent-docs is deleted in this PR. The comment is the sole remaining stale reference to the deleted script.

  • [dead-code] internal/scaffold/baseurl.go:39HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL will always return errors since the embedded harness directory is now empty. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [dead-code] internal/harness/diff.go:32DiffHarness() was only called from buildModifiedAgentFiles() in migrate.go, which is deleted in this PR. DiffHarness and its DiffResult type are now dead code with no production callers.

  • [error-handling-idiom] internal/scaffold/baseurl.go:84HarnessNames() swallows all fs.ReadDir errors by returning (nil, nil). The function could distinguish the expected "directory not found" case from other errors via fs.ErrNotExist.

  • [stale-doc] skills/cutting-releases/pre-flight.md:69 — Section B lists agents/, harness/, scripts/, and skills/ as subdirectories to review under internal/scaffold/fullsend-repo/. These directories are deleted or substantially reduced in this PR.

  • [partial-issue-coverage] internal/scaffold/baseurl.go:53 — Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 (Remove HarnessNames()/HarnessBaseURLWithHash()) is not addressed. These exported functions remain as dead code. The deferral is intentional per the issue tracking.

Previous run (10)

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml — This PR modifies protected paths: .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [stale-reference] internal/harness/harness.go:271 — The Doc field's inline comment says "used by lint-agent-docs", but hack/lint-agent-docs is deleted in this PR. The comment is the sole remaining stale reference to the deleted script.

  • [dead-code] internal/scaffold/baseurl.go:39HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL will always return errors since the embedded harness directory is now empty. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 explicitly defers their removal.

  • [dead-code] internal/harness/diff.go:32DiffHarness() was only called from buildModifiedAgentFiles() in migrate.go, which is deleted in this PR. DiffHarness and its DiffResult type are now dead code with no production callers.

  • [error-handling-idiom] internal/scaffold/baseurl.go:84HarnessNames() swallows all fs.ReadDir errors by returning (nil, nil). The function could distinguish the expected "directory not found" case from other errors via fs.ErrNotExist.

  • [stale-doc] skills/cutting-releases/pre-flight.md:69 — Section B lists agents/, harness/, scripts/, and skills/ as subdirectories to review under internal/scaffold/fullsend-repo/. These directories are deleted or substantially reduced in this PR.

  • [partial-issue-coverage] internal/scaffold/baseurl.go:53 — Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 (Remove HarnessNames()/HarnessBaseURLWithHash()) is not addressed. These exported functions remain as dead code. The deferral is intentional per the issue tracking.

Previous run (11)

Review

Findings

High

  • [test-breakage] internal/harness/scaffold_integration_test.go:130 — Multiple tests in this file (not modified in the PR) will fail after harness YAML files are deleted from the scaffold embed. TestLoadWithOpts_ScaffoldTemplatesForgeResolution calls scaffold.HarnessNames() then require.NotEmpty(t, names) — will fail since HarnessNames() now returns nil. TestDiscoverAgents_ScaffoldDirectory asserts require.Len(t, agents, 6) — will fail since no harness YAMLs exist. TestResolveForge_ScaffoldRunnerEnvMerge hardcodes references to deleted harness files. TestLoadWithBase_WrapperMergesScaffold and TestLoadWithBase_WrapperOverridesBaseFields extract scaffold and load specific harness YAMLs by name — will fail.
    Remediation: Delete or update internal/harness/scaffold_integration_test.go. Tests asserting on scaffold harness content should be removed or converted to test against external agent fixtures.

  • [test-breakage] internal/cli/migrate_test.go:24 — customizedReviewHarness() calls scaffold.HarnessContent('review') which reads fullsend-repo/harness/review.yaml from the embed. This file is deleted, so HarnessContent returns an error and every test using this helper will fail: TestBuildModifiedAgentFiles_DiffAbort, TestBuildModifiedAgentFiles_DevCommitSHAError, TestMigrateCustomizations_ModifiedAgent_DryRun, TestMigrateCustomizations_ModifiedAgent_CreatesPR, TestBuildModifiedAgentFiles_WithAssociatedFiles.
    Remediation: Remove or update migrate_test.go to use fixture data instead of embedded scaffold content.

  • [logic-error] internal/cli/migrate.go:101 — migrate.go calls scaffold.HarnessNames() to build scaffoldSet, used in planMigrations to distinguish migrateModified (known scaffold agent with customizations) from migrateCustom. After this PR, HarnessNames() returns empty, so scaffoldSet is always empty and no agent is ever classified as migrateModified — all are treated as fully custom. This silently changes the migrate command's behavior. Additionally, HarnessNames() returns (nil, nil) instead of an error, masking that the harness directory is permanently absent.
    Remediation: Either remove the migrate command and its tests (if scaffold-based migration is no longer needed), or update migration logic to resolve agent content from fullsend-ai/agents.

  • [stale-doc] docs/guides/user/bring-your-own-agent.md:367 — User-facing documentation contains base: URL examples referencing deleted scaffold paths internal/scaffold/fullsend-repo/harness/code.yaml (lines 367, 410) and review.yaml (line 402). Users following these examples will construct URLs to files that no longer exist.
    Remediation: Update the base: URL examples to point to the new fullsend-ai/agents source.

  • [stale-doc] docs/guides/user/customizing-agents.md:214 — Stale curl URL references deleted path internal/scaffold/fullsend-repo/harness/code.yaml. The deprecated-overlay example directs users to download a harness file from a path that no longer exists — users will get a 404.
    Remediation: Update the curl URL to point to fullsend-ai/agents, or remove the deprecated example section.

  • [stale-doc] docs/guides/user/building-custom-agents.md:442 — User-facing workflow example uses sparse-checkout of internal/scaffold/fullsend-repo/ and copies layered dirs from that path. After this PR, those directories are empty — the workflow would copy nothing. Users following this example will build broken custom agents.
    Remediation: Update the workflow example to reference the new agent source location in fullsend-ai/agents.

Medium

  • [lint-guard-becomes-no-op] hack/lint-agent-docs:8 — The lint-agent-docs script hardcodes HARNESS_DIR to internal/scaffold/fullsend-repo/harness and iterates over $HARNESS_DIR/*.yaml. With nullglob enabled, all loops iterate zero times after harness YAML deletion, so the linter always reports OK regardless of actual doc quality. The pre-commit hook still fires on docs/agents/ changes, making it actively misleading.
    Remediation: Either remove the lint-agent-docs hook from .pre-commit-config.yaml and delete hack/lint-agent-docs, or update the script to source harness metadata from fullsend-ai/agents.

  • [premature-issue-closure] — The PR body says 'Closes Step 7: Remove scaffold agent fallback infrastructure #5552', but issue Step 7: Remove scaffold agent fallback infrastructure #5552 has remaining unchecked items: Item 5 (remove HarnessNames()/HarnessBaseURLWithHash() after migrate.go no longer needs them) and Item 6 (update fullsend lock to be config/agents-repo-aware). Using 'Closes' will auto-close the issue on merge, losing tracking of this remaining work.
    Remediation: Change 'Closes Step 7: Remove scaffold agent fallback infrastructure #5552' to 'Addresses Step 7: Remove scaffold agent fallback infrastructure #5552', or file follow-up issues for remaining Items 5 and 6 before merging.

  • [stale-doc] docs/contributing/bot-identities.md:3 — States GitHub App login is derived from the slug field in each harness file (internal/scaffold/fullsend-repo/harness/*.yaml). These harness files were deleted from the scaffold embed.
    Remediation: Update the reference to point to fullsend-ai/agents.

  • [stale-doc] docs/contributing/sandbox-topology.md:19 — States harness definitions that map agents to images live in internal/scaffold/fullsend-repo/harness/*.yaml. These harness files were deleted from the scaffold embed.
    Remediation: Update the reference to point to fullsend-ai/agents.

  • [protected-path] .pre-commit-config.yaml — This PR modifies protected paths: .pre-commit-config.yaml, skills/agent-scaffolding, skills/code-review, skills/finding-agent-runs, skills/pr-review. The PR links to issue Step 7: Remove scaffold agent fallback infrastructure #5552 and explains the rationale (deleting scaffold agent files now served from fullsend-ai/agents). Human approval is always required for protected-path changes, regardless of context.

Low

  • [dead-code] internal/scaffold/baseurl.go:36 — HarnessContentHash, HarnessBaseURLWithHash, HarnessContent, and HarnessBaseURL all read from fullsend-repo/harness/ which is empty after this PR. These functions will always return errors. Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 defers their removal to a follow-up — intentional phasing.

  • [stale-reference] internal/scaffold/baseurl.go:79 — HarnessNames() doc comment still says it returns names like e.g., ["code", "fix", "triage"]. After this PR it always returns an empty list. Function is planned for removal in follow-up.

  • [pre-commit-lint-coverage-gap] .pre-commit-config.yaml:182 — The lint-agent-docs hook file pattern was narrowed from ^(internal/scaffold/fullsend-repo/harness/|docs/agents/) to ^docs/agents/. The hook no longer triggers when harness definitions change (now in external agents repo). Consistent with the deletion.

  • [partial-issue-coverage] internal/scaffold/baseurl.go:83 — Issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5 (Remove HarnessNames()/HarnessBaseURLWithHash()) is not addressed in this PR. The functions are modified but not removed. Subsumed by the premature-issue-closure finding.

  • [error-handling-idiom] internal/scaffold/baseurl.go:84 — HarnessNames swallows all fs.ReadDir errors by returning (nil, nil). The established pattern in this package wraps errors with context. However, embed.FS cannot experience filesystem corruption — the only realistic error is directory-does-not-exist. Function is planned for removal per issue Step 7: Remove scaffold agent fallback infrastructure #5552 Item 5.

  • [stale-doc] docs/guides/dev/cli-internals.md:610 — The executableFiles code listing shows ~20+ entries including scripts/post-code.sh, scripts/scan-secrets. The map was trimmed to 6 entries. The snippet's purpose (explaining embed.FS permission tracking) remains correct but the example is outdated.

  • [stale-doc] docs/architecture.md:730 — References post-code.sh, scripts/pre-code.sh, scripts/post-code.sh which were deleted from the scaffold embed. The architecture doc describes the conceptual flow; these scripts now live in fullsend-ai/agents.

  • [stale-doc] docs/testing/functional-tests.md:66 — Documents FULLSEND_DIR default as internal/scaffold/fullsend-repo. The path still exists with non-agent scaffold content (workflows, templates), but no longer contains harness/agent content.

  • [stale-doc] skills/cutting-releases/pre-flight.md:72 — Pre-flight audit diff of internal/scaffold/fullsend-repo/ still works for remaining scaffold content but will not surface agent-content changes (now in fullsend-ai/agents).


Labels: PR removes harness/agent files from scaffold embed and updates installation infrastructure

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added component/harness Agent harness, config, and skills loading component/install CLI install and app setup go Pull requests that update go code labels Jul 25, 2026
@ggallen
ggallen force-pushed the worktree-scaffold-cleanup branch from a1c3c45 to 99bd1e7 Compare July 25, 2026 02:00
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 25, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 2:02 AM UTC · Ended 2:22 AM UTC
Commit: 99bd1e7 · View workflow run →

@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review July 25, 2026 02:22

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 25, 2026
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:02 AM UTC · Completed 2:22 AM UTC
Commit: 99bd1e7 · View workflow run →

@ggallen
ggallen force-pushed the worktree-scaffold-cleanup branch from 99bd1e7 to 3e2a7af Compare July 25, 2026 02:27
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 25, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 2:29 AM UTC · Ended 2:34 AM UTC
Commit: 3e2a7af · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 25, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:35 AM UTC · Completed 2:55 AM UTC
Commit: ce7595e · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen
ggallen force-pushed the worktree-scaffold-cleanup branch from ce7595e to 04402bf Compare July 25, 2026 02:59
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 25, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:01 AM UTC · Completed 3:22 AM UTC
Commit: 04402bf · View workflow run →

Merged via the queue into fullsend-ai:main with commit b4fd4ae Jul 28, 2026
22 of 23 checks passed
@ggallen
ggallen deleted the worktree-scaffold-cleanup branch July 28, 2026 15:40
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 28, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 3:43 PM UTC · Completed 3:59 PM UTC
Commit: 8b8c3bb · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5588refactor(scaffold)!: delete agent files from scaffold embed

What happened

PR #5588 was a large human-authored refactoring (113 files, +103/-16,469 lines) that deleted agent files from the scaffold embed, since they are now served from fullsend-ai/agents at runtime. The review agent (fullsend-ai-review) ran 11+ times across 12 commits and consistently reported 1 medium + 3 low findings (protected-path, dead-code, error-handling, scope-authorization-mismatch). It correctly applied the requires-manual-review label.

Human reviewer ascerra caught that companion files (process-fix-result.py, pre-fetch-prior-review.sh, setup-prioritize.sh) were not yet available in the agents repo, blocking the PR with CHANGES_REQUESTED. After the blocker was resolved, human reviewer waynesun09 used a 4-agent review squad (Claude, Grok, Gemini) and found 3 CRITICAL issues: pre-code.sh, pre-fix.sh, and pre-fetch-prior-review.sh were deleted from the scaffold but are still hard-invoked by reusable workflow YAML (reusable-code.yml, reusable-fix.yml, reusable-review.yml) with no existence guard. Merging without the fix would have broken code, fix, and review agent runs org-wide.

Review quality gap

The review agent's correctness sub-agent analyzed Go code callers of deleted files (correctly finding dead code in baseurl.go and migrate.go) but never checked whether workflow YAML files invoke the deleted scripts via bash commands — a completely separate runtime consumption path. The review orchestrator also skipped the cross-repo-contracts sub-agent, classifying the PR as "internal restructuring only," despite the PR title explicitly stating it deletes files shared with another repo.

The review agent DID find things humans didn't flag: stale Doc field comment referencing deleted lint-agent-docs, and a comprehensive sweep of stale doc references across ADRs and design specs.

Evidence for existing issues

  • #1525 (cross-file impact analysis gap): This retro provides strong evidence. The review orchestrator classified the PR as "internal restructuring" and skipped the cross-repo-contracts sub-agent on a PR whose most critical issues were cross-repo contract violations. The correctness sub-agent's scope was limited to Go code analysis and did not extend to workflow YAML runtime references.
  • #5194 (cross-location verification): pre-fetch-prior-review.sh had no counterpart in fullsend-ai/agents at all — the review agent accepted the PR's claim of content relocation without verification.
  • agents#149 (out-of-diff file analysis): The critical findings were all in files outside the PR diff (.github/workflows/reusable-*.yml).
  • agents#480 (inline comment 422 failure): Review run 30282837936 attempted 2 inline comments but got HTTP 422; the post-script retried without them.

Cost

The review agent ran 12+ times at ~$6.59 per dispatch run (run 2: $6.59, 282K cache_create + 1.67M cache_read + 27K output tokens). Total estimated review cost for this PR: $50-80. Each run repeated the same 4 findings while missing the 3 CRITICAL ones. The human-orchestrated review squad found the critical issues in a single pass.

Proposals filed

ggallen pushed a commit to ggallen/fullsend that referenced this pull request Jul 28, 2026
…ock config-aware

Remove HarnessNames(), HarnessContentHash(), and HarnessBaseURLWithHash()
from internal/scaffold/baseurl.go — these functions have no production
callers after PR fullsend-ai#5588 removed the migrateModified code path. Their
corresponding tests are also removed.

Update fullsend lock to resolve agents from config when no local harness
file exists. The new resolveHarnessForLock function tries the local
harness directory first, then falls back to config-driven resolution
via harness.ResolveRegisteredPath. This supports both local-path and
URL-sourced agent entries. The --all flag now also discovers agents
registered in config.yaml, not just local harness files.

Closes fullsend-ai#5668
ggallen added a commit to ggallen/agents that referenced this pull request Jul 28, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ggallen added a commit to ggallen/agents that referenced this pull request Jul 28, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ggallen added a commit to ggallen/agents that referenced this pull request Jul 28, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ggallen added a commit to ggallen/agents that referenced this pull request Jul 28, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ggallen added a commit to ggallen/agents that referenced this pull request Jul 29, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ggallen added a commit to ggallen/agents that referenced this pull request Jul 29, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ggallen added a commit to ggallen/agents that referenced this pull request Jul 29, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ggallen added a commit to ggallen/agents that referenced this pull request Jul 29, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ggallen added a commit to ggallen/agents that referenced this pull request Jul 29, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ggallen added a commit to ggallen/agents that referenced this pull request Jul 29, 2026
pull_request_target.synchronize fires when the fix agent pushes, but
its actor-identity authorization check is gated on the PR's original
author, which for agent-authored PRs is the code agent's bot account
regardless of who triggered this fix run. GitHub App bots have no
collaborator role, so that check always fails closed and review is
never re-dispatched after a fix-agent push (fullsend-ai/fullsend#5188).

post-fix.src.sh now calls a new shared library, relabel-retrigger.lib.sh,
which removes then re-adds the ready-for-review label after a successful
push, forcing a fresh labeled webhook event. That path has no
actor-authorization gate at all — label application itself already
requires write access, so it needs no separate identity check —
mirroring post-code.src.sh's existing handling of the PR-open case.
GitHub does not fire a new labeled event when a label already present
is simply re-added, hence the remove-then-add sequence. Verified live
on a disposable PR in a personal fork: remove-then-add fires a second,
genuinely distinct pull_request.labeled run; a plain re-add of an
already-present label does not.

Extracted as a library (retrigger_via_label) rather than inline script
logic, per review feedback that this kind of dispatch-retrigger
functionality should be reusable across agent post-scripts, not
duplicated per-agent. The GitHub token is taken as an explicit
argument and scoped to each gh invocation individually
(GH_TOKEN="${token}" gh ...) rather than relying on the caller having
exported GH_TOKEN into the shell environment at the right point in
script execution — the library's parameter-passing design makes that
class of ordering bug structurally impossible.

retrigger_via_label checks whether the label was actually present
before attempting removal, so a genuine --remove-label failure (while
the label was present) escalates to a warning instead of being masked
by the idempotent --add-label call silently no-op'ing as if the
retrigger had succeeded.

Added a bundled-script-has-relabel-retrigger presence check and a real
NO_PUSH=false integration test (a genuine feature-branch commit pushed
against a local bare repo standing in for GitHub) to post-fix-test.sh,
so a transposed argument, wrong token, flipped NO_PUSH guard, or a
deleted call site would fail CI instead of passing silently.

This fix was originally attempted in fullsend-ai/fullsend#5551 against
internal/scaffold/fullsend-repo/scripts/post-fix.sh, but that copy is
being deleted from that repo (fullsend-ai/fullsend#5588) since agent
scripts are now served from this repo via resolveAgentSource() — this
repo's post-fix.sh is the actual production script, and it still had
the original bug, unaffected by anything in #5551.

Signed-off-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
ifireball pushed a commit that referenced this pull request Jul 29, 2026
givenCustomHarness and givenDisabledCustomHarness commit the harness
YAML and register it in config.yaml but never create the agent (or
policy) MD file referenced by the harness's agent: field. After
scaffold agent files were removed (#5552 / #5588), clean pool repos
fail at harness validation with "no such file or directory" for
.fullsend/agents/triage.md.

Add commitLocalHarnessResources that parses the harness YAML for
relative agent and policy paths and commits minimal stub files under
.fullsend/ on the config repo. This mirrors commitRelativeResources
in url_dispatch.go (used by URL-sourced harnesses) but writes to
the config repo with the .fullsend/ prefix.

Closes #5715

Signed-off-by: Barak Korren <bkorren@redhat.com>
fullsend-ai-coder Bot added a commit that referenced this pull request Jul 30, 2026
givenCustomHarness and givenDisabledCustomHarness commit the harness
YAML and register it in config.yaml but never create the agent (or
policy) MD file referenced by the harness's agent: field. After
scaffold agent files were removed (#5552 / #5588), clean pool repos
fail at harness validation with "no such file or directory" for
.fullsend/agents/triage.md.

Add commitLocalHarnessResources that parses the harness YAML for
relative agent and policy paths and commits minimal stub files under
.fullsend/ on the config repo. This mirrors commitRelativeResources
in url_dispatch.go (used by URL-sourced harnesses) but writes to
the config repo with the .fullsend/ prefix.

Closes #5715

Signed-off-by: Barak Korren <bkorren@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/harness Agent harness, config, and skills loading component/install CLI install and app setup go Pull requests that update go code requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants