feat(harness): wire Lint() diagnostics into fullsend run and lock

[ggallen]

Summary

• Wire h.Lint() into fullsend run after harness loading to surface non-fatal warnings
• Wire h.Lint() into fullsend lock with deduplication across forge variants
• Currently warns when role field is missing from harness files

This is Phase 3 PR 3 of ADR-0045 (Forge-Portable Harness Schema).

Context

ADR-0045 Phase 3 is the "Deprecate" milestone. This PR implements the first step: surfacing lint diagnostics in CLI commands. The Lint() method and Diagnostic type already exist in internal/harness/lint.go (shipped in Phase 1).

Lint diagnostics are informational only — commands still succeed regardless of warnings. This prepares users for Phase 4 where role will become required.

Test plan

• go build ./cmd/fullsend/ compiles
• go test ./internal/cli/... -run "TestRun|TestLock" passes
• make lint passes
• Manual: fullsend run on harness without role → warning printed, command succeeds
• Manual: fullsend lock --all on harnesses without role → warnings printed (deduplicated), command succeeds

Related

• ADR: docs/ADRs/0045-forge-portable-harness-schema.md
• Phase 3 plan: docs/plans/adr-0045-forge-portable-harness-phase3.md

🤖 Generated with Claude Code

[github-actions]

Site preview

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

Commit: f4e19d57cf8d97b3fbb58185c1b36e0d821e8aaa

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:07 PM UTC · Completed 8:17 PM UTC
Commit: 91943cf · View workflow run →

[codecov]

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/run.go	86.66%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Looks good to me

Previous run

Looks good to me

Previous run

Review

Findings

Medium

• [naming-and-conventions] internal/cli/lock.go:209 — Diagnostic message formatting is inconsistent between lock.go and run.go, and both differ from the canonical Diagnostic.String() method. lock.go uses fmt.Sprintf("%s: %s", diag.Field, diag.Message) (dropping severity), while run.go uses fmt.Sprintf("harness lint: %s", diag) (adding a redundant prefix). The Diagnostic type has a String() method that formats as "severity: field: message" — both call sites should use it.
  Remediation: Use printer.StepWarn(diag.String()) in both lock.go and run.go for consistent formatting via the canonical Diagnostic.String() method.

• [naming-and-conventions] internal/cli/run.go:344 — Same inconsistency as above from the run.go side: the "harness lint: " prefix is redundant when Diagnostic.String() already includes severity and field information.
  Remediation: Change to printer.StepWarn(diag.String()) to remove the redundant prefix.

Low

• [pattern-violation] internal/cli/lock.go:210 — The ADR phase 3 plan specifies that in --all mode, lock diagnostics should include the harness name as context. The implementation omits the harness/agent name. The lockOneAgent function receives agentName as a parameter but does not include it in the diagnostic output. The parent runLockAll prints a header per agent providing visual context, but the plan is explicit about including it in the diagnostic line.

• [pattern-violation] internal/cli/lock.go:210 — lock.go manually formats diag.Field and diag.Message, dropping the severity. If a SeverityError diagnostic is added in the future, lock.go would emit it via StepWarn with no textual indication of error severity, while run.go would at least include the severity string via diag.String().

Previous run

Looks good to me

Previous run (2)

Review

Findings

Medium

• [naming-and-conventions] internal/cli/lock.go:209 — Diagnostic message formatting is inconsistent between lock.go and run.go, and both differ from the canonical Diagnostic.String() method. lock.go uses fmt.Sprintf("%s: %s", diag.Field, diag.Message) (dropping severity), while run.go uses fmt.Sprintf("harness lint: %s", diag) (adding a redundant prefix). The Diagnostic type has a String() method that formats as "severity: field: message" — both call sites should use it.
  Remediation: Use printer.StepWarn(diag.String()) in both lock.go and run.go for consistent formatting via the canonical Diagnostic.String() method.

• [naming-and-conventions] internal/cli/run.go:344 — Same inconsistency as above from the run.go side: the "harness lint: " prefix is redundant when Diagnostic.String() already includes severity and field information.
  Remediation: Change to printer.StepWarn(diag.String()) to remove the redundant prefix.

Low

• [pattern-violation] internal/cli/lock.go:210 — The ADR phase 3 plan specifies that in --all mode, lock diagnostics should include the harness name as context. The implementation omits the harness/agent name. The lockOneAgent function receives agentName as a parameter but does not include it in the diagnostic output. The parent runLockAll prints a header per agent providing visual context, but the plan is explicit about including it in the diagnostic line.

• [pattern-violation] internal/cli/lock.go:210 — lock.go manually formats diag.Field and diag.Message, dropping the severity. If a SeverityError diagnostic is added in the future, lock.go would emit it via StepWarn with no textual indication of error severity, while run.go would at least include the severity string via diag.String().

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:42 PM UTC · Completed 8:54 PM UTC
Commit: eb89d2f · View workflow run →

[ralphbean]

LGTM!

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:20 AM UTC · Completed 12:32 AM UTC
Commit: f4e19d5 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 1:20 AM UTC · Completed 1:25 AM UTC
Commit: f4e19d5 · View workflow run →

[fullsend-ai-retro]

Retro: PR #2362 — Wire Lint() diagnostics into fullsend run and lock

Workflow quality: Good. This human-authored PR had a clean workflow with high-quality review feedback.

Timeline

1. PR opened by ggallen (2026-06-16 20:03 UTC)
2. First review run approved with "Looks good to me" (~20:17), noting prior findings had been addressed
3. Second review run approved (~20:54) but posted two valid low-severity findings:
   • [low] test-adequacy: Missing tests for lint warning behavior in run and lock commands
   • [low] severity-loss: Unconditional StepWarn() regardless of Diagnostic.Severity
4. Human reviewer (ralphbean) approved with "LGTM!" (20:59)
5. Author addressed both findings in a follow-up commit (00:16 UTC, June 17)
6. Third review run approved (00:32), PR merged (01:15)

Assessment

• Review quality: Both findings were valid and actionable — the author agreed and fixed them. No false positives.
• Rework: One round of rework, ~3.5 hours elapsed. Reasonable for a human-authored PR.
• Token cost: Three review runs is appropriate (initial, post-push, post-fix).

Existing coverage

The main pattern observed — review agent approving while posting low-severity findings that drive author rework — is already tracked in #1368 ("Review agent approval with residual findings drives unnecessary author rework") and #2029 ("Review agent: use COMMENT verdict for re-reviews with only low-severity findings").

No new proposals filed — this workflow executed well and the one systemic pattern is already tracked.