From 30571fad24fa3f5602de467d5c80dc8d67d4a880 Mon Sep 17 00:00:00 2001 From: Greg Allen Date: Mon, 22 Jun 2026 11:31:43 -0400 Subject: [PATCH] feat(harness): require role field in Validate() (ADR-0045 Phase 4 PR 1) Promote the missing-role check from a Lint() warning to a hard Validate() error. Every scaffold harness already sets role:, so this is non-breaking for existing users while enforcing the contract going forward. - Validate() now returns "role field is required" when Role is empty - Lint() no longer emits the role-is-not-set diagnostic - Role struct tag keeps omitempty (Validate() is the enforcement) - ADR-0045 struct example consistent with code - Phase 4 plan updated to mark PR 1 as in-review - Removed trivially-passing NoLintWarningWithRole tests - All test fixtures updated to include role: where needed Signed-off-by: Greg Allen Co-Authored-By: Claude Opus 4.6 Signed-off-by: Greg Allen --- .../adr-0045-forge-portable-harness-phase4.md | 2 +- internal/cli/lock_all_test.go | 39 +++++++---- internal/cli/lock_test.go | 70 ++++++------------- internal/cli/run_test.go | 62 +++------------- internal/harness/compose_test.go | 43 ++++++++++++ internal/harness/forge_test.go | 11 +++ internal/harness/harness.go | 15 ++-- internal/harness/harness_test.go | 70 ++++++++++++++----- internal/harness/integration_test.go | 6 +- internal/harness/lint.go | 12 +--- internal/harness/lint_test.go | 12 +--- 11 files changed, 180 insertions(+), 162 deletions(-) diff --git a/docs/plans/adr-0045-forge-portable-harness-phase4.md b/docs/plans/adr-0045-forge-portable-harness-phase4.md index 352796c0c..3f1ff69b5 100644 --- a/docs/plans/adr-0045-forge-portable-harness-phase4.md +++ b/docs/plans/adr-0045-forge-portable-harness-phase4.md @@ -94,7 +94,7 @@ Every consumer of the removed code, and the action taken: ## PR Dependency Graph ``` -PR 1 (require role in Validate) [independent] +PR 1 (require role in Validate) [independent] 🔄 In Review (#2446) PR 2 (remove agents from NewOrgConfig + ConfigRepoLayer) ──> PR 4 (remove OrgConfig.Agents field) │ diff --git a/internal/cli/lock_all_test.go b/internal/cli/lock_all_test.go index 438772fc7..3f737ec06 100644 --- a/internal/cli/lock_all_test.go +++ b/internal/cli/lock_all_test.go @@ -60,6 +60,7 @@ func TestLockAll_MultipleHarnesses(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) codeHarness := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: coder policy: "%s/policies/sandbox.yaml#sha256=%s" allowed_remote_resources: - "%s/" @@ -71,6 +72,7 @@ allowed_remote_resources: )) triageHarness := fmt.Sprintf(`agent: "%s/agents/triage.md#sha256=%s" +role: triage allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -118,6 +120,7 @@ func TestLockAll_MixedURLAndLocalHarnesses(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) urlHarness := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -127,7 +130,7 @@ allowed_remote_resources: 0o644, )) - localHarness := "agent: agents/local.md\n" + localHarness := "agent: agents/local.md\nrole: test\n" require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "local.yaml"), []byte(localHarness), @@ -163,7 +166,7 @@ func TestLockAll_ParseFailure(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "good.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) @@ -183,7 +186,7 @@ func TestLockAll_YMLExtension(t *testing.T) { dir := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) - localHarness := "agent: agents/code.md\n" + localHarness := "agent: agents/code.md\nrole: test\n" require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "review.yml"), []byte(localHarness), @@ -306,6 +309,7 @@ func TestLockAll_PartialProgressOnFailure(t *testing.T) { // First harness resolves successfully. goodHarness := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -349,7 +353,7 @@ func TestLockAll_InvalidForgeFlag(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) @@ -364,7 +368,7 @@ func TestRunLock_InvalidForgeFlag(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) @@ -381,7 +385,7 @@ func TestLockOneAgent_YMLFallback(t *testing.T) { // Only .yml extension, no .yaml. require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "review.yml"), - []byte("agent: agents/review.md\n"), + []byte("agent: agents/review.md\nrole: test\n"), 0o644, )) @@ -396,7 +400,7 @@ func TestLockOneAgent_StalenessCheck(t *testing.T) { dir := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) - harnessContent := []byte("agent: agents/code.md\n") + harnessContent := []byte("agent: agents/code.md\nrole: test\n") require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), harnessContent, @@ -431,12 +435,12 @@ func TestLockOneAgent_DualExtensionWarning(t *testing.T) { // Create both .yaml and .yml for the same stem. require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) @@ -454,7 +458,7 @@ func TestLockAll_CorruptLockFile(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) @@ -477,7 +481,7 @@ func TestLockAll_CobraDispatch(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) @@ -493,7 +497,7 @@ func TestLockCmd_SingleAgentCobraDispatch(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) @@ -515,6 +519,7 @@ func TestLockOneAgent_AllowlistViolation(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessYAML := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -578,6 +583,7 @@ func TestRunLock_SaveError(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessYAML := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -614,6 +620,7 @@ func TestLockAll_SaveError(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessYAML := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -650,6 +657,7 @@ func TestLockAll_WithUpdateFlag(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessYAML := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -696,6 +704,7 @@ func TestLockAll_AllUpToDateMessage(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessYAML := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -734,6 +743,7 @@ func TestLockAll_PrunesStaleEntry(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessYAML := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -760,7 +770,7 @@ allowed_remote_resources: // Replace the harness with a local-only version (no remote deps). require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/local.md\n"), + []byte("agent: agents/local.md\nrole: test\n"), 0o644, )) @@ -787,6 +797,7 @@ func TestLockAll_PrunesRemovedHarness(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessYAML := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test allowed_remote_resources: - "%s/" `, srv.URL, agentHash, srv.URL) @@ -816,7 +827,7 @@ allowed_remote_resources: // Add a different local-only harness so --all has something to iterate. require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "local.yaml"), - []byte("agent: agents/local.md\n"), + []byte("agent: agents/local.md\nrole: test\n"), 0o644, )) diff --git a/internal/cli/lock_test.go b/internal/cli/lock_test.go index c47ea7fea..45227b308 100644 --- a/internal/cli/lock_test.go +++ b/internal/cli/lock_test.go @@ -49,6 +49,7 @@ func setupLockTestDir(t *testing.T, srv *httptest.Server, agentHash, policyHash require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessContent := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test policy: "%s/policies/sandbox.yaml#sha256=%s" allowed_remote_resources: - "%s/" @@ -133,6 +134,7 @@ func TestRunLock_SkillDirectoryType(t *testing.T) { skillURL := fmt.Sprintf("https://github.com/test-org/test-repo/tree/main/skills/test#sha256=%s", treeHash) harnessContent := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test skills: - "%s" allowed_remote_resources: @@ -220,6 +222,7 @@ func TestRunLock_SkillDirectoryRoundTrip(t *testing.T) { skillURL := fmt.Sprintf("https://github.com/test-org/test-repo/tree/main/skills/test#sha256=%s", treeHash) harnessContent := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test skills: - "%s" allowed_remote_resources: @@ -303,6 +306,7 @@ func TestRunLock_NoURLReferences(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessContent := `agent: agents/code.md +role: test skills: - skills/rust ` @@ -401,6 +405,7 @@ func TestRunLock_MultiForgeLockAllVariants(t *testing.T) { // Forge overrides use local skills (no URL validation needed) and the // agent/policy URLs are shared. Each variant adds a different pre_script. harnessContent := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test policy: "%s/policies/sandbox.yaml#sha256=%s" allowed_remote_resources: - "%s/" @@ -459,6 +464,7 @@ func TestRunLock_ForgeSelectsSingleVariant(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessContent := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test policy: "%s/policies/sandbox.yaml#sha256=%s" allowed_remote_resources: - "%s/" @@ -514,6 +520,7 @@ func TestRunLock_ForgeDeduplicatesAcrossVariants(t *testing.T) { // adds a different local pre_script. The lock should deduplicate the // shared URLs across variants. harnessContent := fmt.Sprintf(`agent: "%s/agents/code.md#sha256=%s" +role: test policy: "%s/policies/sandbox.yaml#sha256=%s" allowed_remote_resources: - "%s/" @@ -837,6 +844,7 @@ func TestRunLock_WithLocalBase(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) baseContent := `agent: agents/shared.md +role: test skills: - skills/common ` @@ -871,7 +879,7 @@ func TestResolveFromLock_BaseFieldNoOp(t *testing.T) { // because LoadWithBase already resolved the base composition. agentContent := []byte("You are a coding agent.") agentHash := fetch.ComputeSHA256(agentContent) - baseContent := []byte("agent: agents/shared.md\nskills:\n - skills/common\n") + baseContent := []byte("agent: agents/shared.md\nrole: test\nskills:\n - skills/common\n") baseHash := fetch.ComputeSHA256(baseContent) skillContent := []byte("# Skill A") skillHash := fetch.ComputeSHA256(skillContent) @@ -926,7 +934,7 @@ func TestRunLock_URLBaseOnlyDeps(t *testing.T) { // A child harness with a URL base and no other URL references. // The baseDeps conversion loop runs and the base-only-deps path is taken // (skip ResolveHarness, still record deps in lock file). - baseContent := []byte("agent: agents/shared.md\nskills:\n - skills/common\n") + baseContent := []byte("agent: agents/shared.md\nrole: test\nskills:\n - skills/common\n") baseHash := fetch.ComputeSHA256(baseContent) srv, policy := newLockTestServer(t, map[string][]byte{ @@ -971,7 +979,7 @@ func TestRunLock_URLBaseOnlyDeps(t *testing.T) { func TestRunLock_URLBaseOnlyDepsWithPlatform(t *testing.T) { // Same as above but with a forge platform set, exercising the platform != "" branch // in the base-only-deps logging path. - baseContent := []byte("agent: agents/shared.md\nskills:\n - skills/common\n") + baseContent := []byte("agent: agents/shared.md\nrole: test\nskills:\n - skills/common\n") baseHash := fetch.ComputeSHA256(baseContent) srv, policy := newLockTestServer(t, map[string][]byte{ @@ -1022,7 +1030,7 @@ func TestRunLock_URLRefsNoOrgConfigError(t *testing.T) { dir := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) - harnessContent := fmt.Sprintf("agent: \"%s/agents/code.md#sha256=%s\"\n", srv.URL, agentHash) + harnessContent := fmt.Sprintf("agent: \"%s/agents/code.md#sha256=%s\"\nrole: test\n", srv.URL, agentHash) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "noconfig.yaml"), []byte(harnessContent), @@ -1049,7 +1057,7 @@ func TestRunLock_MalformedOrgConfig(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "simple.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) require.NoError(t, os.WriteFile( @@ -1076,7 +1084,7 @@ func TestRunLock_MalformedOrgConfigWithURLRefs(t *testing.T) { dir := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) - harnessContent := fmt.Sprintf("agent: \"%s/agents/code.md#sha256=%s\"\n", srv.URL, agentHash) + harnessContent := fmt.Sprintf("agent: \"%s/agents/code.md#sha256=%s\"\nrole: test\n", srv.URL, agentHash) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "badcfg.yaml"), []byte(harnessContent), @@ -1104,6 +1112,7 @@ func TestRunLock_NoOrgConfigNoURLRefs(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) harnessContent := `agent: agents/code.md +role: test ` require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "simple.yaml"), @@ -1138,7 +1147,7 @@ func TestRunLock_OrgAllowlistSyncedAfterReAttempt(t *testing.T) { // Harness with URL agent refs — exercises the re-attempt path when // config.yaml is initially malformed. - harnessContent := fmt.Sprintf("agent: \"%s/agents/code.md#sha256=%s\"\n", srv.URL, agentHash) + harnessContent := fmt.Sprintf("agent: \"%s/agents/code.md#sha256=%s\"\nrole: test\n", srv.URL, agentHash) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "urlrefs.yaml"), []byte(harnessContent), @@ -1165,7 +1174,7 @@ func TestRunLock_OrgAllowlistSyncedAfterReAttempt(t *testing.T) { func TestRunLock_URLBaseAndURLRefsNoOrgConfig(t *testing.T) { // Harness with both a URL base and other URL references but no config.yaml. // LoadWithBase should fail at the URL base fetch (not at HasURLReferences). - baseContent := []byte("agent: agents/shared.md\n") + baseContent := []byte("agent: agents/shared.md\nrole: test\n") baseHash := fetch.ComputeSHA256(baseContent) agentContent := []byte("You are a coding agent.") agentHash := fetch.ComputeSHA256(agentContent) @@ -1178,7 +1187,7 @@ func TestRunLock_URLBaseAndURLRefsNoOrgConfig(t *testing.T) { dir := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) - harnessContent := fmt.Sprintf("base: \"%s/base.yaml#sha256=%s\"\nagent: \"%s/agents/code.md#sha256=%s\"\n", + harnessContent := fmt.Sprintf("base: \"%s/base.yaml#sha256=%s\"\nrole: test\nagent: \"%s/agents/code.md#sha256=%s\"\n", srv.URL, baseHash, srv.URL, agentHash) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "combo.yaml"), @@ -1198,8 +1207,8 @@ func TestRunLock_URLBaseAndURLRefsNoOrgConfig(t *testing.T) { 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. +func TestRunLock_ErrorOnMissingRole(t *testing.T) { + // Verifies that runLock fails with a hard error 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)) @@ -1209,7 +1218,7 @@ func TestRunLock_LintWarningOnMissingRole(t *testing.T) { []byte("You are a coding agent."), 0o644, )) - // Harness without role field, no URL references (no lock needed) + // Harness without role field require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), []byte("agent: agents/code.md\n"), @@ -1219,39 +1228,6 @@ func TestRunLock_LintWarningOnMissingRole(t *testing.T) { 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") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid harness: role field is required") } diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index c74ba4be2..dbc9d3ae3 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -154,7 +154,7 @@ func TestRunAgent_HarnessLoadPipeline(t *testing.T) { )) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) @@ -178,7 +178,7 @@ func TestRunAgent_YMLFallback(t *testing.T) { )) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) @@ -216,7 +216,7 @@ func TestRunAgent_HarnessLoadWithOrgConfig(t *testing.T) { )) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) require.NoError(t, os.WriteFile( @@ -247,7 +247,7 @@ func TestRunAgent_MalformedOrgConfig(t *testing.T) { )) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte("agent: agents/code.md\n"), + []byte("agent: agents/code.md\nrole: test\n"), 0o644, )) require.NoError(t, os.WriteFile( @@ -273,7 +273,7 @@ func TestRunAgent_MalformedOrgConfigWithURLRefs(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte(fmt.Sprintf("agent: \"https://example.com/agents/code.md#sha256=%s\"\n", agentHash)), + []byte(fmt.Sprintf("agent: \"https://example.com/agents/code.md#sha256=%s\"\nrole: test\n", agentHash)), 0o644, )) require.NoError(t, os.WriteFile( @@ -299,7 +299,7 @@ func TestRunAgent_URLRefsNoOrgConfig(t *testing.T) { agentHash := fetch.ComputeSHA256([]byte("agent content")) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte(fmt.Sprintf("agent: \"https://example.com/agents/code.md#sha256=%s\"\n", agentHash)), + []byte(fmt.Sprintf("agent: \"https://example.com/agents/code.md#sha256=%s\"\nrole: test\n", agentHash)), 0o644, )) @@ -313,7 +313,7 @@ func TestRunAgent_URLRefsNoOrgConfig(t *testing.T) { func TestRunAgent_WithURLBase(t *testing.T) { // Harness with a URL base — exercises the baseDeps logging loop. - baseContent := []byte("agent: agents/shared.md\n") + baseContent := []byte("agent: agents/shared.md\nrole: test\n") baseHash := fetch.ComputeSHA256(baseContent) srv, policy := newLockTestServer(t, map[string][]byte{ @@ -331,7 +331,7 @@ func TestRunAgent_WithURLBase(t *testing.T) { )) require.NoError(t, os.WriteFile( filepath.Join(dir, "harness", "code.yaml"), - []byte(fmt.Sprintf("base: \"%s/base.yaml#sha256=%s\"\n", srv.URL, baseHash)), + []byte(fmt.Sprintf("base: \"%s/base.yaml#sha256=%s\"\nrole: test\n", srv.URL, baseHash)), 0o644, )) require.NoError(t, os.WriteFile( @@ -1796,9 +1796,8 @@ func TestEmitDiagnosticWithContext(t *testing.T) { 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). +func TestRunAgent_ErrorOnMissingRole(t *testing.T) { + // Verifies that runAgent fails with a hard error 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)) @@ -1821,45 +1820,6 @@ func TestRunAgent_LintWarningOnMissingRole(t *testing.T) { repoDir := t.TempDir() err := runAgent(context.Background(), "code", dir, "", repoDir, "", 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) - repoDir := t.TempDir() - err := runAgent(context.Background(), "code", dir, "", repoDir, "", 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") + assert.Contains(t, err.Error(), "invalid harness: role field is required") } diff --git a/internal/harness/compose_test.go b/internal/harness/compose_test.go index fff4e871b..b020a1b01 100644 --- a/internal/harness/compose_test.go +++ b/internal/harness/compose_test.go @@ -33,6 +33,7 @@ func TestLoadWithBase_NoBase(t *testing.T) { dir := t.TempDir() path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/test.md +role: test model: opus `) @@ -49,6 +50,7 @@ func TestLoadWithBase_LocalBase_ScalarOverride(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test model: sonnet image: base-image timeout_minutes: 30 @@ -57,6 +59,7 @@ timeout_minutes: 30 path := writeTestHarness(t, dir, "child.yaml", ` base: base.yaml agent: agents/child.md +role: test model: opus `) @@ -80,6 +83,7 @@ func TestLoadWithBase_LocalBase_SkillsConcat(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/test.md +role: test skills: - skill-a - skill-b @@ -103,6 +107,7 @@ func TestLoadWithBase_LocalBase_RunnerEnvMerge(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/test.md +role: test runner_env: KEY1: base-value1 KEY2: base-value2 @@ -131,6 +136,7 @@ func TestLoadWithBase_LocalBase_HostFilesDedup(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/test.md +role: test host_files: - src: base-src1 dest: /dest1 @@ -165,6 +171,7 @@ func TestLoadWithBase_LocalBase_ValidationLoopReplace(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/test.md +role: test validation_loop: script: base-script.sh max_iterations: 5 @@ -191,6 +198,7 @@ func TestLoadWithBase_LocalBase_ValidationLoopInherit(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/test.md +role: test validation_loop: script: base-script.sh max_iterations: 5 @@ -216,6 +224,7 @@ func TestLoadWithBase_ChainedBases(t *testing.T) { // A → B → C: C is the root, B extends C, A extends B writeTestHarness(t, dir, "c.yaml", ` agent: agents/c.md +role: test model: c-model image: c-image skills: @@ -232,6 +241,7 @@ skills: path := writeTestHarness(t, dir, "a.yaml", ` base: b.yaml agent: agents/a.md +role: test skills: - skill-a `) @@ -255,11 +265,13 @@ func TestLoadWithBase_CycleDetection(t *testing.T) { // A → B → A (cycle) writeTestHarness(t, dir, "a.yaml", ` agent: agents/a.md +role: test base: b.yaml `) writeTestHarness(t, dir, "b.yaml", ` agent: agents/b.md +role: test base: a.yaml `) @@ -275,6 +287,7 @@ func TestLoadWithBase_SelfReference(t *testing.T) { // A → A (self-reference) path := writeTestHarness(t, dir, "a.yaml", ` agent: agents/a.md +role: test base: a.yaml `) @@ -291,6 +304,7 @@ func TestLoadWithBase_LocalBase_PathTraversal(t *testing.T) { // Child in subdir tries to reference base outside workspace root via ../ path := writeTestHarness(t, subdir, "child.yaml", ` agent: agents/child.md +role: test base: ../../../etc/passwd `) @@ -310,6 +324,7 @@ func TestLoadWithBase_LocalBase_PathTraversal_NoWorkspaceRoot(t *testing.T) { // Child in subdir tries to reference base outside via ../ path := writeTestHarness(t, subdir, "child.yaml", ` agent: agents/child.md +role: test base: ../outside.yaml `) @@ -345,6 +360,7 @@ func TestLoadWithBase_ForgeBlockMerge(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/test.md +role: test forge: github: pre_script: base-pre.sh @@ -389,6 +405,7 @@ func TestLoadWithBase_ForgeInheritPlatform(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/test.md +role: test forge: github: pre_script: gh-pre.sh @@ -413,6 +430,7 @@ model: opus func TestLoadWithBase_URLBase(t *testing.T) { baseContent := []byte(` agent: agents/remote.md +role: test model: sonnet skills: - remote-skill @@ -432,6 +450,7 @@ skills: path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: `+baseURL+` allowed_remote_resources: - `+server.URL+`/ @@ -467,12 +486,14 @@ func TestLoadWithBase_ChainedURLBases(t *testing.T) { // Test URL base whose own base is also a URL grandparentContent := []byte(` agent: agents/grandparent.md +role: test model: opus `) grandparentHash := computeHash(grandparentContent) parentContent := []byte(` agent: agents/parent.md +role: test skills: - parent-skill `) @@ -494,6 +515,7 @@ skills: // Now create parent content with base pointing to grandparent parentContentWithBase := []byte(fmt.Sprintf(` agent: agents/parent.md +role: test base: %s/grandparent.yaml#sha256=%s skills: - parent-skill @@ -520,6 +542,7 @@ skills: path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: `+parentURL+` skills: - child-skill @@ -567,6 +590,7 @@ func TestLoadWithBase_URLBase_HashMismatch(t *testing.T) { path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: `+baseURL+` allowed_remote_resources: - `+server.URL+`/ @@ -604,6 +628,7 @@ func TestLoadWithBase_URLBase_NotInAllowlist(t *testing.T) { path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: `+baseURL+` allowed_remote_resources: - https://other.example.com/ @@ -630,6 +655,7 @@ func TestLoadWithBase_URLBase_NoOrgAllowlist(t *testing.T) { path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: https://example.com/base.yaml#sha256=0000000000000000000000000000000000000000000000000000000000000000 `) @@ -644,6 +670,7 @@ func TestLoadWithBase_URLBase_MissingHash(t *testing.T) { path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: https://example.com/base.yaml allowed_remote_resources: - https://example.com/ @@ -662,6 +689,7 @@ func TestLoadWithBase_URLBase_OfflineMode_CacheMiss(t *testing.T) { path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: https://example.com/base.yaml#sha256=0000000000000000000000000000000000000000000000000000000000000000 allowed_remote_resources: - https://example.com/ @@ -681,6 +709,7 @@ allowed_remote_resources: func TestLoadWithBase_URLBase_OfflineMode_CacheHit(t *testing.T) { baseContent := []byte(` agent: agents/remote.md +role: test model: sonnet `) hash := computeHash(baseContent) @@ -693,6 +722,7 @@ model: sonnet path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: https://example.com/base.yaml#sha256=`+hash+` allowed_remote_resources: - https://example.com/ @@ -743,6 +773,7 @@ func TestLoadWithBase_AllowedRemoteResourcesNotMerged(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test allowed_remote_resources: - https://example.com/base/ `) @@ -881,6 +912,7 @@ func TestLoadWithBase_InvalidForgeAfterMerge(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test forge: invalid_platform: pre_script: test.sh @@ -901,6 +933,7 @@ func TestLoadWithBase_ValidationErrorAfterMerge(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test `) // Child clears the agent field (empty string doesn't override) @@ -921,6 +954,7 @@ func TestLoadWithBase_BaseFileNotFound(t *testing.T) { path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: nonexistent.yaml `) @@ -934,6 +968,7 @@ func TestLoadWithBase_URLBase_NonHTTPS(t *testing.T) { path := writeTestHarness(t, dir, "child.yaml", ` agent: agents/child.md +role: test base: http://example.com/base.yaml#sha256=0000000000000000000000000000000000000000000000000000000000000000 allowed_remote_resources: - http://example.com/ @@ -951,6 +986,7 @@ func TestLoadWithBase_SecurityInheritance(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test security: fail_mode: closed `) @@ -972,6 +1008,7 @@ func TestLoadWithBase_SecurityChildOverrides(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test security: fail_mode: closed `) @@ -994,6 +1031,7 @@ func TestLoadWithBase_APIServersConcat(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test api_servers: - name: base-api script: base-api.sh @@ -1021,6 +1059,7 @@ func TestLoadWithBase_PluginsConcat(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test plugins: - plugin-a `) @@ -1042,6 +1081,7 @@ func TestLoadWithBase_ProvidersConcat(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test providers: - provider-a `) @@ -1063,6 +1103,7 @@ func TestLoadWithBase_TimeoutInheritance(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test timeout_minutes: 30 sandbox_timeout_seconds: 600 `) @@ -1084,6 +1125,7 @@ func TestLoadWithBase_RunnerEnvNilBase(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/base.md +role: test `) path := writeTestHarness(t, dir, "child.yaml", ` @@ -1103,6 +1145,7 @@ func TestLoadWithBase_RuntimeFetchFieldsNotInherited(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/test.md +role: test allowed_remote_resources: - https://example.com/ allow_runtime_fetch: true diff --git a/internal/harness/forge_test.go b/internal/harness/forge_test.go index 67c88e85b..4bac21ec9 100644 --- a/internal/harness/forge_test.go +++ b/internal/harness/forge_test.go @@ -245,6 +245,7 @@ func TestResolveForge_ForgeConsumed(t *testing.T) { func TestValidate_ForgeUnrecognizedKey(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Forge: map[string]*ForgeConfig{ "gihub": {PreScript: "scripts/gh.sh"}, }, @@ -261,6 +262,7 @@ func TestValidate_ForgeScriptURL(t *testing.T) { t.Run("pre_script URL", func(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Forge: map[string]*ForgeConfig{ "github": {PreScript: "https://example.com/scripts/pre.sh"}, }, @@ -273,6 +275,7 @@ func TestValidate_ForgeScriptURL(t *testing.T) { t.Run("post_script URL", func(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Forge: map[string]*ForgeConfig{ "gitlab": {PostScript: "https://example.com/scripts/post.sh"}, }, @@ -285,6 +288,7 @@ func TestValidate_ForgeScriptURL(t *testing.T) { t.Run("validation_loop.script URL", func(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Forge: map[string]*ForgeConfig{ "github": { ValidationLoop: &ValidationLoop{ @@ -302,6 +306,7 @@ func TestValidate_ForgeScriptURL(t *testing.T) { t.Run("validation_loop missing script", func(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Forge: map[string]*ForgeConfig{ "github": { ValidationLoop: &ValidationLoop{ @@ -319,6 +324,7 @@ func TestValidate_ForgeScriptURL(t *testing.T) { func TestValidate_ForgeValidConfig(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Forge: map[string]*ForgeConfig{ "github": { PreScript: "scripts/pre-gh.sh", @@ -343,6 +349,7 @@ func TestValidate_ForgeValidConfig(t *testing.T) { func TestValidate_ForgeNilConfig(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Forge: map[string]*ForgeConfig{ "github": nil, }, @@ -353,6 +360,7 @@ func TestValidate_ForgeNilConfig(t *testing.T) { func TestValidate_ForgeSkillURLWithoutHash(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Forge: map[string]*ForgeConfig{ "github": { Skills: []string{"https://example.com/skills/summarize.md"}, @@ -368,6 +376,7 @@ func TestValidate_ForgeSkillURLWithoutHash(t *testing.T) { func TestValidate_ForgeSkillURLWithHash(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Forge: map[string]*ForgeConfig{ "github": { Skills: []string{"https://example.com/skills/summarize.md#sha256=abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"}, @@ -380,6 +389,7 @@ func TestValidate_ForgeSkillURLWithHash(t *testing.T) { func TestLoad_WithForgeSection(t *testing.T) { content := ` agent: agents/test.md +role: test pre_script: scripts/pre-common.sh skills: - skills/common @@ -415,6 +425,7 @@ forge: func TestLoad_WithoutForgeSection(t *testing.T) { content := ` agent: agents/test.md +role: test pre_script: scripts/pre.sh ` dir := t.TempDir() diff --git a/internal/harness/harness.go b/internal/harness/harness.go index 9c7630bdd..21c99b022 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -319,13 +319,14 @@ func (h *Harness) Validate() error { if h.Model != "" && !validModelName.MatchString(h.Model) { return fmt.Errorf("model %q contains invalid characters (allowed: a-z, A-Z, 0-9, _, -, ., @)", h.Model) } - if h.Role != "" { - if !validRoleName.MatchString(h.Role) { - return fmt.Errorf("role %q contains invalid characters (allowed: a-z, 0-9, _, -; must start with a lowercase letter)", h.Role) - } - if strings.Contains(h.Role, "--") { - return fmt.Errorf("role %q must not contain double hyphens", h.Role) - } + if h.Role == "" { + return fmt.Errorf("role field is required") + } + if !validRoleName.MatchString(h.Role) { + return fmt.Errorf("role %q contains invalid characters (allowed: a-z, 0-9, _, -; must start with a lowercase letter)", h.Role) + } + if strings.Contains(h.Role, "--") { + return fmt.Errorf("role %q must not contain double hyphens", h.Role) } if h.Slug != "" && !validSlugName.MatchString(h.Slug) { return fmt.Errorf("slug %q contains invalid characters (allowed: a-z, A-Z, 0-9, _, -; must start with a letter or digit)", h.Slug) diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index 76b862dfb..110e9b692 100644 --- a/internal/harness/harness_test.go +++ b/internal/harness/harness_test.go @@ -12,6 +12,7 @@ import ( func TestLoad_ValidHarness(t *testing.T) { content := ` agent: agents/hello-world.md +role: triage image: registry.example.com/sandbox:v1 skills: - skills/hello-world-summary @@ -68,6 +69,7 @@ skills: func TestLoad_ValidationLoopMissingScript(t *testing.T) { content := ` agent: agents/test.md +role: test validation_loop: max_iterations: 3 ` @@ -83,6 +85,7 @@ validation_loop: func TestLoad_HostFiles(t *testing.T) { content := ` agent: agents/test.md +role: test host_files: - src: ${GOOGLE_APPLICATION_CREDENTIALS} dest: /sandbox/workspace/.gcp-credentials.json @@ -114,6 +117,7 @@ host_files: func TestValidate_HostFileMissingSrc(t *testing.T) { content := ` agent: agents/test.md +role: test host_files: - dest: /sandbox/workspace/.gcp-credentials.json ` @@ -129,6 +133,7 @@ host_files: func TestValidate_HostFileMissingDest(t *testing.T) { content := ` agent: agents/test.md +role: test host_files: - src: ${GOOGLE_APPLICATION_CREDENTIALS} ` @@ -284,6 +289,7 @@ func TestFailModeClosed_Open(t *testing.T) { func TestLoad_SecurityConfig(t *testing.T) { content := ` agent: agents/test.md +role: test security: fail_mode: open host_scanners: @@ -335,7 +341,7 @@ security: } func TestValidate_SecurityInvalidFailMode(t *testing.T) { - h := &Harness{Agent: "test.md", Security: &SecurityConfig{FailMode: "invalid"}} + h := &Harness{Agent: "test.md", Role: "test", Security: &SecurityConfig{FailMode: "invalid"}} err := h.Validate() require.Error(t, err) assert.Contains(t, err.Error(), "fail_mode") @@ -344,6 +350,7 @@ func TestValidate_SecurityInvalidFailMode(t *testing.T) { func TestValidate_SecurityInvalidLLMGuardThreshold(t *testing.T) { h := &Harness{ Agent: "test.md", + Role: "test", Security: &SecurityConfig{ HostScanners: &HostScanners{ LLMGuard: &LLMGuardConfig{Threshold: 1.5}, @@ -358,6 +365,7 @@ func TestValidate_SecurityInvalidLLMGuardThreshold(t *testing.T) { func TestValidate_SecurityInvalidLLMGuardMatchType(t *testing.T) { h := &Harness{ Agent: "test.md", + Role: "test", Security: &SecurityConfig{ HostScanners: &HostScanners{ LLMGuard: &LLMGuardConfig{MatchType: "word"}, @@ -372,6 +380,7 @@ func TestValidate_SecurityInvalidLLMGuardMatchType(t *testing.T) { func TestValidate_SecurityInvalidTirithFailOn(t *testing.T) { h := &Harness{ Agent: "test.md", + Role: "test", Security: &SecurityConfig{ SandboxHooks: &SandboxHooks{ Tirith: &TirithConfig{FailOn: "low"}, @@ -386,6 +395,7 @@ func TestValidate_SecurityInvalidTirithFailOn(t *testing.T) { func TestValidate_SecurityInvalidEscalation(t *testing.T) { h := &Harness{ Agent: "test.md", + Role: "test", Security: &SecurityConfig{ Escalation: &EscalationConfig{OnCritical: "ignore"}, }, @@ -398,6 +408,7 @@ func TestValidate_SecurityInvalidEscalation(t *testing.T) { func TestValidate_SecurityValidConfig(t *testing.T) { h := &Harness{ Agent: "test.md", + Role: "test", Security: &SecurityConfig{ FailMode: "open", HostScanners: &HostScanners{ @@ -442,7 +453,7 @@ func TestValidate_AgentNameInvalid(t *testing.T) { } func TestValidate_AgentNameValid(t *testing.T) { - h := &Harness{Agent: "agents/hello-world_v2.md"} + h := &Harness{Agent: "agents/hello-world_v2.md", Role: "test"} require.NoError(t, h.Validate()) } @@ -461,19 +472,20 @@ func TestValidate_ModelValid(t *testing.T) { "claude-sonnet-4-6@20250514", "claude-opus-4-1@20250805", } { - h := &Harness{Agent: "agents/test.md", Model: model} + h := &Harness{Agent: "agents/test.md", Role: "test", Model: model} require.NoError(t, h.Validate(), "model %q should be valid", model) } } func TestValidate_PostScriptWithoutValidationLoop(t *testing.T) { - h := &Harness{Agent: "agents/test.md", PostScript: "scripts/post.sh"} + h := &Harness{Agent: "agents/test.md", Role: "test", PostScript: "scripts/post.sh"} require.NoError(t, h.Validate()) } func TestValidate_PostScriptWithValidationLoop(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", PostScript: "scripts/post.sh", ValidationLoop: &ValidationLoop{ Script: "scripts/validate.sh", @@ -484,56 +496,57 @@ func TestValidate_PostScriptWithValidationLoop(t *testing.T) { } func TestValidate_NegativeTimeout(t *testing.T) { - h := &Harness{Agent: "agents/test.md", TimeoutMinutes: -1} + h := &Harness{Agent: "agents/test.md", Role: "test", TimeoutMinutes: -1} err := h.Validate() require.Error(t, err) assert.Contains(t, err.Error(), "timeout_minutes must be non-negative") } func TestValidate_NegativeSandboxTimeout(t *testing.T) { - h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: -1} + h := &Harness{Agent: "agents/test.md", Role: "test", SandboxTimeoutSeconds: -1} err := h.Validate() require.Error(t, err) assert.Contains(t, err.Error(), "sandbox_timeout_seconds must be 0 (default) or between 30 and 600") } func TestValidate_SandboxTimeoutTooSmall(t *testing.T) { - h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 10} + h := &Harness{Agent: "agents/test.md", Role: "test", SandboxTimeoutSeconds: 10} err := h.Validate() require.Error(t, err) assert.Contains(t, err.Error(), "sandbox_timeout_seconds must be 0 (default) or between 30 and 600") } func TestValidate_SandboxTimeoutTooLarge(t *testing.T) { - h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 601} + h := &Harness{Agent: "agents/test.md", Role: "test", SandboxTimeoutSeconds: 601} err := h.Validate() require.Error(t, err) assert.Contains(t, err.Error(), "sandbox_timeout_seconds must be 0 (default) or between 30 and 600") } func TestValidate_SandboxTimeoutAtMin(t *testing.T) { - h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 30} + h := &Harness{Agent: "agents/test.md", Role: "test", SandboxTimeoutSeconds: 30} require.NoError(t, h.Validate()) } func TestValidate_SandboxTimeoutAtMax(t *testing.T) { - h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 600} + h := &Harness{Agent: "agents/test.md", Role: "test", SandboxTimeoutSeconds: 600} require.NoError(t, h.Validate()) } func TestValidate_ZeroSandboxTimeout(t *testing.T) { - h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 0} + h := &Harness{Agent: "agents/test.md", Role: "test", SandboxTimeoutSeconds: 0} require.NoError(t, h.Validate()) } func TestValidate_PositiveSandboxTimeout(t *testing.T) { - h := &Harness{Agent: "agents/test.md", SandboxTimeoutSeconds: 180} + h := &Harness{Agent: "agents/test.md", Role: "test", SandboxTimeoutSeconds: 180} require.NoError(t, h.Validate()) } func TestLoad_SandboxTimeoutField(t *testing.T) { content := ` agent: agents/test.md +role: test sandbox_timeout_seconds: 180 ` dir := t.TempDir() @@ -548,6 +561,7 @@ sandbox_timeout_seconds: 180 func TestLoad_ModelField(t *testing.T) { content := ` agent: agents/test.md +role: test model: sonnet ` dir := t.TempDir() @@ -598,6 +612,7 @@ func TestValidateFilesExist_SkipsVarPaths(t *testing.T) { func TestValidate_PluginNameValid(t *testing.T) { h := &Harness{ Agent: "agents/test.md", + Role: "test", Plugins: []string{"plugins/gopls-lsp", "plugins/my_plugin-2"}, } require.NoError(t, h.Validate()) @@ -607,6 +622,7 @@ func TestValidate_PluginNameInvalid(t *testing.T) { for _, name := range []string{"my plugin", "foo;bar", "bad@name"} { h := &Harness{ Agent: "agents/test.md", + Role: "test", Plugins: []string{"plugins/" + name}, } err := h.Validate() @@ -686,6 +702,7 @@ func TestHarness_AllowedRemoteResources_Parse(t *testing.T) { t.Run("with allowed_remote_resources", func(t *testing.T) { content := ` agent: agents/test.md +role: test allowed_remote_resources: - https://example.com/skills/ - https://cdn.example.com/policies/ @@ -702,6 +719,7 @@ allowed_remote_resources: t.Run("without allowed_remote_resources", func(t *testing.T) { content := ` agent: agents/test.md +role: test ` dir := t.TempDir() path := filepath.Join(dir, "test.yaml") @@ -1092,7 +1110,7 @@ slug: fullsend-ai-triage assert.Equal(t, "fullsend-ai-triage", h.Slug) } -func TestLoad_RoleAndSlugAbsent(t *testing.T) { +func TestLoad_RoleMissing(t *testing.T) { content := ` agent: agents/test.md ` @@ -1100,10 +1118,9 @@ agent: agents/test.md path := filepath.Join(dir, "test.yaml") require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) - h, err := Load(path) - require.NoError(t, err) - assert.Empty(t, h.Role) - assert.Empty(t, h.Slug) + _, err := Load(path) + require.Error(t, err) + assert.Contains(t, err.Error(), "role field is required") } func TestValidate_RoleValid(t *testing.T) { @@ -1131,14 +1148,14 @@ func TestValidate_RoleDoubleHyphen(t *testing.T) { func TestValidate_SlugValid(t *testing.T) { for _, slug := range []string{"fullsend-ai-triage", "Custom_App", "a1"} { - h := &Harness{Agent: "agents/test.md", Slug: slug} + h := &Harness{Agent: "agents/test.md", Role: "test", Slug: slug} require.NoError(t, h.Validate(), "slug %q should be valid", slug) } } func TestValidate_SlugInvalid(t *testing.T) { for _, slug := range []string{"-slug", "slug!name", "my slug"} { - h := &Harness{Agent: "agents/test.md", Slug: slug} + h := &Harness{Agent: "agents/test.md", Role: "test", Slug: slug} err := h.Validate() require.Error(t, err, "slug %q should be invalid", slug) assert.Contains(t, err.Error(), "slug") @@ -1193,6 +1210,7 @@ func TestLoadRaw_FileNotFound(t *testing.T) { func TestLoadWithOpts_AppliesForgeResolution(t *testing.T) { content := ` agent: agents/test.md +role: test pre_script: scripts/pre-common.sh skills: - skills/common @@ -1216,6 +1234,7 @@ forge: func TestLoadWithOpts_NoForge_SameAsLoad(t *testing.T) { content := ` agent: agents/test.md +role: test pre_script: scripts/pre.sh ` dir := t.TempDir() @@ -1230,6 +1249,7 @@ pre_script: scripts/pre.sh func TestLoadWithOpts_EmptyPlatform_PreservesForge(t *testing.T) { content := ` agent: agents/test.md +role: test forge: github: pre_script: scripts/pre-gh.sh @@ -1246,6 +1266,7 @@ forge: func TestLoadWithOpts_InvalidPlatform(t *testing.T) { content := ` agent: agents/test.md +role: test forge: github: pre_script: scripts/pre-gh.sh @@ -1264,6 +1285,7 @@ func TestLoadWithOpts_ValidationAfterForge(t *testing.T) { // The validation_loop in the forge block replaces the top-level one. content := ` agent: agents/test.md +role: test forge: github: validation_loop: @@ -1283,6 +1305,7 @@ forge: func TestLoadWithOpts_PlatformNotConfigured(t *testing.T) { content := ` agent: agents/test.md +role: test forge: github: pre_script: scripts/pre-gh.sh @@ -1301,6 +1324,7 @@ forge: func TestValidate_AllowRuntimeFetchWithoutAllowedResources(t *testing.T) { h := &Harness{ Agent: "agents/code.md", + Role: "test", AllowRuntimeFetch: true, } err := h.Validate() @@ -1312,6 +1336,7 @@ func TestValidate_MaxRuntimeFetchesWithoutAllowRuntimeFetch(t *testing.T) { v := 5 h := &Harness{ Agent: "agents/code.md", + Role: "test", MaxRuntimeFetches: &v, } err := h.Validate() @@ -1323,6 +1348,7 @@ func TestValidate_MaxRuntimeFetchesNegative(t *testing.T) { v := -1 h := &Harness{ Agent: "agents/code.md", + Role: "test", AllowRuntimeFetch: true, AllowedRemoteResources: []string{"https://github.com/fullsend-ai/library/"}, MaxRuntimeFetches: &v, @@ -1336,6 +1362,7 @@ func TestValidate_MaxRuntimeFetchesExceedsUpperBound(t *testing.T) { v := 1001 h := &Harness{ Agent: "agents/code.md", + Role: "test", AllowRuntimeFetch: true, AllowedRemoteResources: []string{"https://github.com/fullsend-ai/library/"}, MaxRuntimeFetches: &v, @@ -1349,6 +1376,7 @@ func TestValidate_AllowRuntimeFetchValid(t *testing.T) { v := 5 h := &Harness{ Agent: "agents/code.md", + Role: "test", AllowRuntimeFetch: true, MaxRuntimeFetches: &v, AllowedRemoteResources: []string{"https://github.com/fullsend-ai/library/"}, @@ -1360,6 +1388,7 @@ func TestValidate_AllowRuntimeFetchValid(t *testing.T) { func TestValidate_AllowRuntimeFetchDefaultMaxFetches(t *testing.T) { h := &Harness{ Agent: "agents/code.md", + Role: "test", AllowRuntimeFetch: true, AllowedRemoteResources: []string{"https://github.com/fullsend-ai/library/"}, } @@ -1373,6 +1402,7 @@ func TestValidate_MaxRuntimeFetchesExplicitZero(t *testing.T) { v := 0 h := &Harness{ Agent: "agents/code.md", + Role: "test", AllowRuntimeFetch: true, AllowedRemoteResources: []string{"https://github.com/fullsend-ai/library/"}, MaxRuntimeFetches: &v, @@ -1385,6 +1415,7 @@ func TestValidate_MaxRuntimeFetchesExplicitZero(t *testing.T) { func TestLoad_RuntimeFetchFields(t *testing.T) { content := ` agent: agents/code.md +role: test allowed_remote_resources: - https://github.com/fullsend-ai/library/ allow_runtime_fetch: true @@ -1406,6 +1437,7 @@ max_runtime_fetches: 15 func TestLoad_RuntimeFetchFieldsOmitted(t *testing.T) { content := ` agent: agents/code.md +role: test ` dir := t.TempDir() path := filepath.Join(dir, "minimal.yaml") diff --git a/internal/harness/integration_test.go b/internal/harness/integration_test.go index 0ccf90b1e..b4ae9668c 100644 --- a/internal/harness/integration_test.go +++ b/internal/harness/integration_test.go @@ -22,6 +22,7 @@ func TestLoadWithBase_BackwardCompat(t *testing.T) { path := writeTestHarness(t, dir, "simple.yaml", ` agent: agents/test.md +role: test timeout_minutes: 5 `) @@ -32,7 +33,7 @@ timeout_minutes: 5 assert.Equal(t, 5, h.TimeoutMinutes) assert.Empty(t, h.Base, "base field should be empty (no base)") assert.Nil(t, deps, "baseDeps should be nil when no base is used") - assert.Empty(t, h.Role) + assert.Equal(t, "test", h.Role) assert.Empty(t, h.Slug) assert.Nil(t, h.Forge) } @@ -45,6 +46,7 @@ func TestLoadWithBase_BaseWithForgeAndIdentity(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/shared.md +role: test model: sonnet skills: - base-skill-1 @@ -118,6 +120,7 @@ func TestLoadWithBase_BaseWithForgeSkillsConcatenation(t *testing.T) { writeTestHarness(t, dir, "base.yaml", ` agent: agents/test.md +role: test skills: - base-s1 forge: @@ -161,6 +164,7 @@ func TestLoadWithBase_NoBaseIdenticalToLoadWithOpts(t *testing.T) { content := ` agent: agents/test.md +role: test model: opus skills: - skill-a diff --git a/internal/harness/lint.go b/internal/harness/lint.go index 85a3f0aef..4dcfbfffe 100644 --- a/internal/harness/lint.go +++ b/internal/harness/lint.go @@ -38,15 +38,5 @@ func (d Diagnostic) String() string { // results are meaningless on an invalid harness. // Returns nil when no diagnostics are found. func (h *Harness) Lint() []Diagnostic { - var diags []Diagnostic - - if h.Role == "" { - diags = append(diags, Diagnostic{ - Severity: SeverityWarning, - Field: "role", - Message: "role is not set; it will be required in a future version", - }) - } - - return diags + return nil } diff --git a/internal/harness/lint_test.go b/internal/harness/lint_test.go index 14680b2bd..1a1653d9f 100644 --- a/internal/harness/lint_test.go +++ b/internal/harness/lint_test.go @@ -7,21 +7,11 @@ import ( ) func TestLint(t *testing.T) { - t.Run("role set", func(t *testing.T) { + t.Run("valid harness returns nil", func(t *testing.T) { h := &Harness{Role: "triage"} assert.Nil(t, h.Lint()) }) - t.Run("role empty", func(t *testing.T) { - h := &Harness{} - diags := h.Lint() - assert.NotNil(t, diags) - assert.Len(t, diags, 1) - assert.Equal(t, SeverityWarning, diags[0].Severity) - assert.Equal(t, "role", diags[0].Field) - assert.Contains(t, diags[0].Message, "required in a future version") - }) - t.Run("role and slug set", func(t *testing.T) { h := &Harness{Role: "triage", Slug: "my-slug"} assert.Nil(t, h.Lint())