From f4e19d57cf8d97b3fbb58185c1b36e0d821e8aaa Mon Sep 17 00:00:00 2001 From: Greg Allen Date: Tue, 16 Jun 2026 20:16:57 -0400 Subject: [PATCH] feat(harness): wire Lint() diagnostics into fullsend run and lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Call h.Lint() after harness loading in both `fullsend run` and `fullsend lock` commands to surface non-fatal warnings. Currently warns when the `role` field is missing from a harness file. This is Phase 3 PR 3 of ADR-0045. Lint diagnostics are informational only — commands still succeed regardless of warnings. For `fullsend lock`, diagnostics are deduplicated across forge variants and include the agent name for context. Severity-aware emission: warnings use StepWarn, errors use StepFail to ensure future SeverityError diagnostics are visually distinct. Signed-off-by: Greg Allen Signed-off-by: Claude Signed-off-by: Greg Allen --- internal/cli/lock.go | 10 ++++ internal/cli/lock_test.go | 58 +++++++++++++++++++ internal/cli/run.go | 29 ++++++++++ internal/cli/run_test.go | 117 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 214 insertions(+) diff --git a/internal/cli/lock.go b/internal/cli/lock.go index 0e8c0324aa..bdd850ac90 100644 --- a/internal/cli/lock.go +++ b/internal/cli/lock.go @@ -188,6 +188,7 @@ func lockOneAgent(ctx context.Context, agentName, absFullsendDir, forgeFlag stri var allDeps []resolve.Dependency seen := make(map[string]bool) + linted := make(map[string]bool) // track reported lint diagnostics to avoid duplicates across forge variants for _, platform := range forgePlatforms { h, baseDeps, loadErr := harness.LoadWithBase(ctx, harnessPath, harness.ComposeOpts{ @@ -202,6 +203,15 @@ func lockOneAgent(ctx context.Context, agentName, absFullsendDir, forgeFlag stri return nil, fmt.Errorf("loading harness for forge %q: %w", platform, loadErr) } + // Run lint diagnostics (non-fatal), deduplicating across forge variants + for _, diag := range h.Lint() { + key := diag.String() + if !linted[key] { + linted[key] = true + emitDiagnosticWithContext(printer, agentName, diag) + } + } + if err := h.ResolveRelativeTo(absFullsendDir); err != nil { printer.StepFail("Path validation failed") return nil, fmt.Errorf("resolving paths: %w", err) diff --git a/internal/cli/lock_test.go b/internal/cli/lock_test.go index 975e3726c6..c47ea7feaa 100644 --- a/internal/cli/lock_test.go +++ b/internal/cli/lock_test.go @@ -1197,3 +1197,61 @@ func TestRunLock_URLBaseAndURLRefsNoOrgConfig(t *testing.T) { // Should fail with a clear error about missing org config. assert.Contains(t, err.Error(), "config.yaml") } + +func TestRunLock_LintWarningOnMissingRole(t *testing.T) { + // Verifies that runLock emits a lint warning when harness has no role. + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "agents", "code.md"), + []byte("You are a coding agent."), + 0o644, + )) + // Harness without role field, no URL references (no lock needed) + require.NoError(t, os.WriteFile( + filepath.Join(dir, "harness", "code.yaml"), + []byte("agent: agents/code.md\n"), + 0o644, + )) + + var buf strings.Builder + printer := ui.New(&buf) + err := runLock(context.Background(), "code", dir, "", false, resolveFlags{}, printer) + require.NoError(t, err) + + // Verify lint warning was printed with agent name context + output := buf.String() + assert.Contains(t, output, "code") + assert.Contains(t, output, "role") + assert.Contains(t, output, "warning") +} + +func TestRunLock_NoLintWarningWithRole(t *testing.T) { + // Verifies that runLock does NOT emit a lint warning when harness has role set. + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "agents", "code.md"), + []byte("You are a coding agent."), + 0o644, + )) + // Harness with role field + require.NoError(t, os.WriteFile( + filepath.Join(dir, "harness", "code.yaml"), + []byte("agent: agents/code.md\nrole: coder\n"), + 0o644, + )) + + var buf strings.Builder + printer := ui.New(&buf) + err := runLock(context.Background(), "code", dir, "", false, resolveFlags{}, printer) + require.NoError(t, err) + + // Verify no lint warning about role + output := buf.String() + assert.NotContains(t, output, "role is not set") +} diff --git a/internal/cli/run.go b/internal/cli/run.go index ad9d6153f2..64ef55614c 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -341,6 +341,11 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep } printer.StepDone(fmt.Sprintf("Harness loaded (%.1fs)", time.Since(harnessStart).Seconds())) + // Run lint checks and report any diagnostics (non-fatal). + for _, diag := range h.Lint() { + emitDiagnostic(printer, diag) + } + // Print plan. printer.KeyValue("Agent", h.Agent) if h.Role != "" { @@ -1952,3 +1957,27 @@ func prHeadSHAFromEventPath(path string) string { } return payload.PullRequest.Head.SHA } + +// emitDiagnostic prints a harness lint diagnostic with severity-appropriate formatting. +// Warnings use StepWarn, errors use StepFail. This ensures future SeverityError +// diagnostics are visually distinct from warnings. +func emitDiagnostic(printer *ui.Printer, diag harness.Diagnostic) { + switch diag.Severity { + case harness.SeverityError: + printer.StepFail(diag.String()) + default: + printer.StepWarn(diag.String()) + } +} + +// emitDiagnosticWithContext prints a diagnostic with additional context (e.g., agent name). +// Used by lock --all where multiple harnesses are processed and context helps identify which. +func emitDiagnosticWithContext(printer *ui.Printer, context string, diag harness.Diagnostic) { + msg := fmt.Sprintf("%s: %s", context, diag.String()) + switch diag.Severity { + case harness.SeverityError: + printer.StepFail(msg) + default: + printer.StepWarn(msg) + } +} diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index e939c98508..7e53301712 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -1607,3 +1607,120 @@ func TestSetupStatusNotifier_PRHeadSHA(t *testing.T) { require.NoError(t, err) assert.NotNil(t, n) } + +func TestEmitDiagnostic_Warning(t *testing.T) { + var buf bytes.Buffer + printer := ui.New(&buf) + + diag := harness.Diagnostic{ + Severity: harness.SeverityWarning, + Field: "role", + Message: "test warning message", + } + emitDiagnostic(printer, diag) + + output := buf.String() + assert.Contains(t, output, "warning") + assert.Contains(t, output, "role") + assert.Contains(t, output, "test warning message") +} + +func TestEmitDiagnostic_Error(t *testing.T) { + var buf bytes.Buffer + printer := ui.New(&buf) + + diag := harness.Diagnostic{ + Severity: harness.SeverityError, + Field: "agent", + Message: "test error message", + } + emitDiagnostic(printer, diag) + + output := buf.String() + assert.Contains(t, output, "error") + assert.Contains(t, output, "agent") + assert.Contains(t, output, "test error message") +} + +func TestEmitDiagnosticWithContext(t *testing.T) { + var buf bytes.Buffer + printer := ui.New(&buf) + + diag := harness.Diagnostic{ + Severity: harness.SeverityWarning, + Field: "role", + Message: "role is not set", + } + emitDiagnosticWithContext(printer, "triage", diag) + + output := buf.String() + assert.Contains(t, output, "triage") + assert.Contains(t, output, "warning") + assert.Contains(t, output, "role") +} + +func TestRunAgent_LintWarningOnMissingRole(t *testing.T) { + // Verifies that runAgent emits a lint warning when harness has no role, + // but the command still proceeds (fails later at sandbox availability). + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "agents", "code.md"), + []byte("You are a coding agent."), + 0o644, + )) + // Harness without role field + require.NoError(t, os.WriteFile( + filepath.Join(dir, "harness", "code.yaml"), + []byte("agent: agents/code.md\n"), + 0o644, + )) + + var buf bytes.Buffer + rFlags := resolveFlags{maxDepth: 10, maxResources: 50} + printer := ui.New(&buf) + err := runAgent(context.Background(), "code", dir, "", "/tmp/repo", "", nil, false, "", "", rFlags, statusOpts{}, printer, false) + + // Command fails later (no openshell), but lint warning should be emitted + require.Error(t, err) + assert.Contains(t, err.Error(), "openshell") + + // Verify lint warning was printed + output := buf.String() + assert.Contains(t, output, "role") + assert.Contains(t, output, "warning") +} + +func TestRunAgent_NoLintWarningWithRole(t *testing.T) { + // Verifies that runAgent does NOT emit a lint warning when harness has role set. + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "agents"), 0o755)) + + require.NoError(t, os.WriteFile( + filepath.Join(dir, "agents", "code.md"), + []byte("You are a coding agent."), + 0o644, + )) + // Harness with role field + require.NoError(t, os.WriteFile( + filepath.Join(dir, "harness", "code.yaml"), + []byte("agent: agents/code.md\nrole: coder\n"), + 0o644, + )) + + var buf bytes.Buffer + rFlags := resolveFlags{maxDepth: 10, maxResources: 50} + printer := ui.New(&buf) + err := runAgent(context.Background(), "code", dir, "", "/tmp/repo", "", nil, false, "", "", rFlags, statusOpts{}, printer, false) + + // Command fails later (no openshell) + require.Error(t, err) + assert.Contains(t, err.Error(), "openshell") + + // Verify no lint warning about role + output := buf.String() + assert.NotContains(t, output, "role is not set") +}