refactor(#5668): remove dead scaffold functions and make lock config-aware

[fullsend-ai-coder]

Summary

• Remove HarnessNames(), HarnessContentHash(), and HarnessBaseURLWithHash() from internal/scaffold/baseurl.go — no production callers remain after PR refactor(scaffold)!: delete agent files from scaffold embed #5588 removed the migrateModified code path
• Update fullsend lock to resolve agents from config.yaml when no local harness file exists in .fullsend/harness/, supporting both local-path and URL-sourced agent entries
• Update fullsend lock --all to discover agents registered in config, not just local harness files

Related Issue

Addresses the two remaining items from #5552:

• Item 5: Dead code removal (precondition met by PR refactor(scaffold)!: delete agent files from scaffold embed #5588)
• Item 6: Make fullsend lock config/agents-repo-aware

Changes

• internal/scaffold/baseurl.go: Remove three dead functions and their unused imports (crypto/sha256, encoding/hex, io/fs, sort, strings)
• internal/scaffold/baseurl_test.go: Remove corresponding test functions
• internal/cli/lock.go: Add resolveHarnessForLock() that tries local path first, then falls back to harness.ResolveRegisteredPath() for config-registered agents; restructure lockOneAgent() to load org config early; update runLockAll() to merge config agent names with local discoveries
• internal/cli/lock_all_test.go: Add 9 tests covering config fallback paths (local path preference, URL fallback, disabled agents, missing config, --all with config agents)

Testing

• go test ./internal/scaffold/... passes
• go test ./internal/cli/ -run "TestLock|TestRunLock|TestResolveHarnessPath|TestResolveFromLock|TestDiscoverHarness|TestResolveHarnessForLock" — all 50+ lock tests pass with -race
• Tests added for new resolveHarnessForLock function and config-aware lockOneAgent/runLockAll
• Secret scan passes

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)

---

Closes #5668

Post-script verification

• Branch is not main/master (agent/5668-lock-awareness-dead-code)
• Secret scan passed (gitleaks — b4fd4ae8b290e94e2a8c22918621146e28e92182..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:09 PM UTC · Completed 4:24 PM UTC
Commit: bd3e9fa · View workflow run →

[codecov]

Codecov Report

❌ Patch coverage is 90.78947% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/lock.go	90.78%	4 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Looks good to me

Previous run

Review

Findings

Medium

• [logic-error] internal/cli/lock.go — resolveFromLock checks h.MatchingAllowedPrefix(lockDep.URL) for every lock dependency, including the new agent_source entries. The agent_source URL (the config-registered URL from which the harness YAML was fetched) is typically not listed in the harness's own AllowedRemoteResources field — it's validated against the org-level allowlist during lock creation instead. This means resolveFromLock will reject agent_source dependencies with "is no longer in allowed_remote_resources", causing lock-file resolution to fail at runtime for config-URL agents. While run.go degrades gracefully (falls back to network fetch), this silently defeats the lock file's reproducibility guarantee for these agents.
  Remediation: Skip the h.MatchingAllowedPrefix check for agent_source dependencies (e.g., add an early continue before the allowlist check when lockDep.Field == "agent_source"), since the agent source URL is validated against the org-level allowlist during lock creation.

Low

• [error-handling] internal/cli/lock.go — resolveHarnessForLock distinguishes "file not found" from other errors by checking strings.Contains(localErr.Error(), "harness file not found"). This couples the fallback logic to the exact error message text in resolveHarnessPath. If the message is changed, the fallback silently stops working and config-only agents would fail with the raw error instead of falling back to config resolution.
  Remediation: Consider using a sentinel error (e.g., var errHarnessNotFound = errors.New(...)) returned by resolveHarnessPath and checked with errors.Is in resolveHarnessForLock.

Previous run (2)

Review

Findings

High

• [consumer-completeness] internal/cli/lock.go:860 — The new agent_source field type introduced by resolveHarnessForLock is not handled in the resolveFromLock mutation switch. When fullsend run uses a lock file containing an agent_source dependency, the default case fails the Sscanf for skills[%d] and appends the agent source's cache path as a transitive skill dependency to h.Skills. This corrupts the harness by injecting the harness YAML file itself as a skill.
  Remediation: Add a case to the mutation switch in resolveFromLock for m.field == "agent_source" that is a no-op (similar to the base case), since the agent source is informational and the harness is already loaded from it.

Medium

• [logic-error] internal/cli/lock.go:382 — When the harness is resolved from a config URL, harnessPath is a cache-internal path (e.g., <workspace>/.fullsend-cache/<sha>/content). The Source field in the lock result is computed as filepath.Join("harness", filepath.Base(harnessPath)), which evaluates to "harness/content" — a meaningless path that does not identify the actual agent.
  Remediation: Construct the Source value from the agent name (e.g., filepath.Join("harness", agentName+".yaml")) or from the config entry source URL.

Low

• [error-handling] internal/cli/lock.go:418 — In runLockAll, errors from harness.RegisteredAgents(orgCfg) are silently discarded. Config-only agents could be omitted from --all with no user feedback.
  Remediation: Log a warning via printer.StepWarn when regErr != nil.

• [incomplete-field-propagation] internal/cli/lock.go:601 — In resolveHarnessForLock, the Warning field from resolved.Dep is not propagated when building the agent_source dependency. While currently zero-valued in this code path, this is inconsistent with other Dependency construction sites.
  Remediation: Add Warning: resolved.Dep.Warning to the resolve.Dependency literal.

---

Labels: PR modifies harness resolution logic in the lock command and removes scaffold dead code

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

[ggallen]

/fs-fix - Rebase and fix all the review issues, both inline and in the sticky comments. Also, check the CI failures and see if any fix is needed there.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 4:36 PM UTC · Completed 4:47 PM UTC
Commit: bd3e9fa · View workflow run →

[ggallen]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:57 PM UTC · Completed 5:16 PM UTC
Commit: 28aec82 · View workflow run →

Superseded by updated review

[ggallen]

/fs-fix - Rebase and fix all the review issues, both inline and in the sticky comments.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 5:18 PM UTC · Completed 5:27 PM UTC
Commit: 28aec82 · View workflow run →

[ggallen]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:31 PM UTC · Completed 5:44 PM UTC
Commit: b8d1b89 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:01 PM UTC · Completed 7:17 PM UTC
Commit: b8d1b89 · View workflow run →

[fullsend-ai-retro]

Retro: PR #5682 — remove dead scaffold functions and make lock config-aware

Outcome: Merged in ~2h 51m (16:07–18:58 UTC). 3 commits across 3 review-fix cycles. 5 files changed (+414/−101).

Timeline

Time (UTC)	Event
11:36	Triage agent processes issue #5668, notes blocker on PR #5588
15:15	/fs-code triggered (Run 1) — agent correctly identifies blocker, exits cleanly
15:40	PR #5588 merges, unblocking #5668
15:47	/fs-code re-triggered (Run 2) — agent implements changes, opens PR at 16:07
16:08–16:24	Review Cycle 1: 1 HIGH, 1 MEDIUM, 2 LOW — all valid
16:24	Auto-triggered fix fails (gate check misclassifies app/fullsend-ai-coder as human author)
16:34	Human manually triggers /fs-fix
16:35–16:47	Fix Run 2 succeeds, pushes commit addressing all 4 findings
16:56	Human triggers /fs-review
16:57–17:16	Review Cycle 2: 1 MEDIUM, 1 LOW — both valid (caught gap in first fix)
17:17–17:27	Fix Run 3 succeeds, pushes commit addressing remaining findings
17:29–17:44	Review Cycle 3: "Looks good to me"
18:44	Human approves, no additional findings
18:58	Merged

Review Quality: Excellent

The review agent produced 6 findings across 2 cycles — all valid, zero false positives. The HIGH finding caught a genuine harness-corruption bug (new agent_source type not handled in resolveFromLock mutation switch). Review Cycle 2 was especially valuable: it caught that the fix agent's first pass was incomplete — it added the switch case but missed that the allowlist check earlier in the function would reject agent_source URLs.

Autonomy Readiness

The human reviewer (ggallen) approved without adding any findings beyond what the review agent identified. The review agent fully covered the human review, consistent with existing tracking in #4852 and #4795 for similar Go-focused agent PRs.

Friction Points (all tracked by existing issues)

1. Gate check bot classification (#1569, #5536): The fix workflow gate check uses a [bot]$ regex that doesn't match app/fullsend-ai-coder, causing the auto-triggered fix to fail and requiring human intervention via /fs-fix. The human also had to manually trigger /fs-review after each fix cycle, adding ~10 minutes of delay per cycle. Related issues #5463 and #5185 propose switching to performed_via_github_app.id or user.type detection. PR #5682 provides fresh evidence that this pattern continues to cause friction on every agent-authored PR.

2. Code agent missing switch case for new type (#853, #2694): The code agent added a new agent_source dependency type but didn't handle it in the resolveFromLock mutation switch — the exact pattern described in #853 (code agent should trace full lifecycle of new enum values) and #2694 (review agent should detect unhandled string-enum variants). Both issues reference the same file (internal/cli/lock.go) and the same discriminator-field pattern. PR #5682 is the third documented instance of this pattern recurring.

No Novel Proposals

All identified friction points are covered by existing open issues. The workflow functioned as designed — the review-fix loop caught real bugs iteratively, and the human reviewer confirmed the agent's judgment. The code agent's first-time output was architecturally sound; the issues were implementation-level gaps that the review agent correctly caught at appropriate severity levels.