Skip to content

Add external-agents-tests action#1

Draft
nodo wants to merge 2 commits intomainfrom
nodo/external-agens-test
Draft

Add external-agents-tests action#1
nodo wants to merge 2 commits intomainfrom
nodo/external-agens-test

Conversation

@nodo
Copy link

@nodo nodo commented Mar 24, 2026

Note

Medium Risk
Moderate risk because it changes CI and local test execution paths (removing protocol-level e2e tests and adjusting Makefile targets), which could hide regressions if the new compliance workflow or lifecycle filtering is misconfigured.

Overview
Shifts the external-agent testing model to black-box-first: protocol compliance is now validated via the shared external-agents-tests suite (and new .github/workflows/protocol-compliance.yml), while this repo’s e2e/ harness is explicitly narrowed to lifecycle integration scenarios.

Removes the repo-local protocol/subcommand test harness code (e2e/harness.go, e2e/testenv.go, fixtures, and the Kiro protocol test package) and updates lifecycle coverage to directly execute are-hooks-installed when asserting hook installation.

Updates the external-agent builder skill docs (SKILL.md, write-tests.md, implement.md), top-level docs (README.md, AGENTS.md, e2e/README.md), and the Makefile to reflect the new split and to run lifecycle tests by default (with optional AGENT filtering).

Written by Cursor Bugbot for commit 1fdb39c. Configure here.

@nodo nodo changed the title Add external-agents-tests Add external-agents-tests action Mar 24, 2026
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: CombinedOutput mixes stderr into JSON parsing
    • Replaced cmd.CombinedOutput() with cmd.Output() in hooksInstalled so JSON unmarshaling only consumes stdout while still surfacing stderr on failures.

Create PR

Or push these changes by commenting:

@cursor push cf50eb3fbf
Preview (cf50eb3fbf)
diff --git a/e2e/lifecycle_test.go b/e2e/lifecycle_test.go
--- a/e2e/lifecycle_test.go
+++ b/e2e/lifecycle_test.go
@@ -219,9 +219,12 @@
 		"LANG=en_US.UTF-8",
 	)
 
-	out, err := cmd.CombinedOutput()
+	out, err := cmd.Output()
 	if err != nil {
-		t.Fatalf("%s are-hooks-installed failed: %v\n%s", binPath, err, out)
+		if exitErr, ok := err.(*exec.ExitError); ok {
+			t.Fatalf("%s are-hooks-installed failed: %v\nstdout: %s\nstderr: %s", binPath, err, out, exitErr.Stderr)
+		}
+		t.Fatalf("%s are-hooks-installed failed: %v\nstdout: %s", binPath, err, out)
 	}
 
 	var resp struct {

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant