feat(harness): add remote harness agent discovery via forge API (ADR-0045 Phase 3 PR 2)

[ggallen]

Summary

• Adds DiscoverRemoteAgents() function that discovers agent identity (role, slug) from harness files in a remote config repo via the forge API — the remote counterpart of DiscoverAgents() which reads from the local filesystem
• Extracts ParseRaw(data []byte) (*Harness, error) from LoadRaw() so callers with raw YAML bytes (e.g. from forge API responses) can parse without filesystem I/O
• LoadRaw() refactored to call ParseRaw() internally — no behavior change for existing callers

Details

New files:

• internal/harness/discover_remote.go — DiscoverRemoteAgents function using forge.Client.ListDirectoryContents + GetFileContentAtRef
• internal/harness/discover_remote_test.go — 14 subtests covering sorted output, missing dir, files without role/slug, malformed YAML, per-file API errors, empty dir, yml extension, subdirectory skipping, non-YAML filtering, sort stability, path field semantics, and error propagation

Modified files:

• internal/harness/harness.go — extracted ParseRaw() from LoadRaw()

No production callers added yet — this is pure library code. Subsequent PRs (Phase 3 PRs 4 and 5) will wire DiscoverRemoteAgents into loadKnownSlugs and the uninstall flows.

100% code coverage on discover_remote.go and ParseRaw. Overall harness package at 86.1%.

Test plan

• go test -v -run TestDiscoverRemoteAgents ./internal/harness/ — all 14 subtests pass
• go test -coverprofile=cover.out ./internal/harness/ && go tool cover -func=cover.out | grep discover_remote.go — 100% coverage
• make go-test — all existing tests pass
• make lint — passes
• make go-vet — passes

🤖 Generated with Claude Code

[github-actions]

Site preview

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

Commit: 22be06dc5eebebc7723033f200a6860baaae7f0e

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:21 PM UTC · Completed 12:32 PM UTC
Commit: a723e87 · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Low

• [missing-plan] docs/plans/ — PR title references "ADR-0045 Phase 3 PR 2" but no Phase 3 implementation plan exists. Only phase1.md and phase2.md are present. Consider creating docs/plans/adr-0045-forge-portable-harness-phase3.md or clarifying the PR title.

• [edge-case] internal/harness/discover_remote.go:39 — path.Base(e.Path) correctly strips parent components, but the subsequent "harness/"+name construction is a latent contract dependency on ListDirectoryContents returning paths relative to the queried directory. The test validates this, and path.Base is the correct package choice for non-filesystem paths. Not a live bug.

• [api-surface] internal/harness/discover_remote.go:23 — DiscoverRemoteAgents is exported but has no callers yet. The Phase 2 plan established precedent for adding discovery functions without immediate consumers (DiscoverAgents had no callers when added in Phase 2 PR 1).

Info

• [documentation-completeness] docs/ADRs/0045-forge-portable-harness-schema.md:620 — ADR-0045 mentions agent discovery scanning harness/*.yaml files but only describes local filesystem scanning. The new DiscoverRemoteAgents extends this to remote repos but is not reflected in the ADR.

Previous run

Review

Findings

Medium

• [scope-coherence] internal/harness/harness.go:276 — ParseRaw() is exported but both callers (LoadRaw and DiscoverRemoteAgents) are in the same package. If subsequent PRs wire this into flows in other packages, the export is justified — but if callers remain package-internal, this should be unexported (parseRaw) to keep the public API surface minimal. Worth deciding now before external consumers appear.

Low

• [edge-case] internal/harness/discover_remote.go:39 — path.Base(e.Path) correctly strips parent components, but the subsequent "harness/"+name construction is a latent contract dependency on ListDirectoryContents returning paths relative to the queried directory. The test at line 191 validates this, and path.Base is the correct package choice for non-filesystem paths. Not a live bug.

• [documentation-style] internal/harness/discover_remote.go:13 — Function comment uses "discovers" while the local counterpart DiscoverAgents uses "scans...and returns". Minor verb style inconsistency.

Info

• [phase-sequencing] The PR title references "ADR-0045 Phase 3 PR 2" but no Phase 3 implementation plan exists in docs/plans/ (Phase 1 and Phase 2 have plan documents). Consider adding a phase 3 plan to document the consumer migration sequence.

Previous run (2)

Looks good to me

Low

• [edge-case] internal/harness/discover_remote.go:39 — The name variable is set from e.Path, which should be a bare filename when ListDirectoryContents is called with recursive: false. If a forge implementation returned paths including parent components (e.g., harness/triage.yaml), then "harness/"+name would produce a doubled path. The GitHub implementation strips prefixes, so this is a latent contract dependency rather than a live bug.

• [refactoring-prerequisite] internal/harness/harness.go — ParseRaw() is extracted from LoadRaw() as a prerequisite for DiscoverRemoteAgents, which needs to parse raw bytes without filesystem I/O. LoadRaw is refactored to delegate to ParseRaw with no behavior change.

Info

• [field-population-pattern] internal/harness/discover_remote.go:59 — AgentInfo.Path is intentionally left empty for remote agents (no local path). This behavior is validated by the "path field is empty for remote agents" subtest.

• [naming-inconsistency] internal/harness/discover_remote.go:22 — DiscoverRemoteAgents adds a Remote qualifier while the local counterpart remains DiscoverAgents. This follows the common Go pattern of keeping the original name unqualified and suffixing the new variant.

• [phase-labeling-confusion] The PR title references "ADR-0045 Phase 3 PR 2". Consider verifying the phase label matches the current ADR-0045 phase plan to avoid confusion in the commit history.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:42 PM UTC · Completed 12:53 PM UTC
Commit: 541b45b · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:59 PM UTC · Completed 1:10 PM UTC
Commit: 22be06d · View workflow run →

[ralphbean]

LGTM.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:23 PM UTC · Completed 7:29 PM UTC
Commit: 22be06d · View workflow run →

[fullsend-ai-retro]

Retro Analysis: PR #2327

Verdict: Workflow went well — no new proposals.

This was a clean, human-authored feature PR (ADR-0045 Phase 3 PR 2) that added remote harness agent discovery via the forge API. The workflow executed effectively:

• Review quality was good. The review bot caught a real issue (exported ParseRaw that should be unexported) at medium severity in Run 2. The author addressed it by the final commit, and Run 3 confirmed only low-severity findings remained.
• Rework was minimal and appropriate. Two force-pushes refined the code, with the review bot correctly re-evaluating each revision.
• Human reviewer (ralphbean) approved with "LGTM" after the bot had already approved.
• Test coverage was 100% on new code with 14 subtests.
• Time to merge: ~7 hours (12:17 → 19:17 UTC), including human review turnaround.

Potential improvements already tracked

I identified minor inefficiencies but all are covered by existing open issues:

• 3 review runs for a 1-commit-delta PR: Already tracked by #1372 (cancel-and-skip redundant re-reviews), #963 (skip when SHA already reviewed), and #1452 (deduplicate review dispatches for same SHA).
• Bot review approval triggering additional fullsend.yaml runs: Already tracked by #1271 (filter bot-triggered pull_request_review events).
• Retro running on a clean human-authored PR with no actionable findings: Already tracked by #1675 (extend retro skip guard to human-authored PRs with no-finding reviews).