test: fail closed around unsafe or faulty modules

[kanushka]

Closes #6. Parent: #3. Follows #5, which built the module contract this
proves the boundaries of.

What this is

The shell already refused a module that was unsafe, incompatible,
corrupt, or faulty. Almost none of that was asserted through the seam a
user actually runs, and a rejection that holds in a unit test and not
through the real process boundary is not a rejection anyone can rely on.

So this is a test change. No shell behaviour needed altering: every
acceptance criterion on #6 was already implemented, and the work was
proving it from outside, against a built shell and an isolated store.

How the faults are produced

Two fixture executables under test/acceptance/testdata:

• faultymodule speaks the module contract by hand. The SDK exists
  to make a module conform, so a module built on it cannot lie about its
  identity, require a capability no shell has heard of, or put a damaged
  frame on the wire — which is exactly the set of failures the shell has
  to survive. One control file beside its executable selects one fault,
  because the shell passes a module no arguments and sanitizes its
  environment to nothing.
• launchcanary records being run, and does nothing else.

Proving what did not happen

"Rejected before launch" and "rejected before invocation" are claims
about absence, so they are not left to the shell's own account of the
failure. The installed executable records how far the exchange got,
beside itself, and the assertions read that. Control cases prove each
marker does appear when the exchange reaches that point, so an absent
one is evidence rather than an untested default.

Coverage

Each acceptance criterion of #6, black-box:

Criterion	Where
Receipt path escapes its version directory	lexical and symbolic-link cases, both proved before launch
Same-named executable on PATH or in the working directory	ignored while installed, and still ignored when the installation is gone
Modified executable no longer matching its receipt	pre-existing case
Incompatible shell, protocol, operating system, architecture	one table, all four proved before launch
Runtime identity differing from the receipt	with the invocation proved not to have been sent
Unknown message kinds and unknown required capabilities	paired with a test that an additive unknown field is still accepted
Truncated, partial, malformed, oversized frames	four distinct stable problems
Premature exit, panic, non-zero exit	the shell reports the module's crash text rather than crashing with it
Hanging module terminated after timeout and grace	the module exits on a closed input, proving the grace was offered, and the process is proved gone
Bounded diagnostics, uncontaminated JSON	512 KiB flood against the shell's own limit
No partial structured result reported as success	every failing case asserts empty standard output

Notes for review

• TestAModuleThatSpeaksAnotherProtocolFailsBeforeInvocation is
  removed. The incompatible-receipt table covers it and also proves the
  before-launch half its name claimed but never asserted; a comment at
  its old site points to the replacement.
• The fault names and marker names are repeated between the fixtures and
  the tests, because a main package cannot be imported. Every fault is
  selected by content and every marker asserted by name, so a value that
  drifts fails its test rather than passing quietly.
• Acceptance now runs about 90s, from two cases that must wait out the
  invocation deadline. The timing-sensitive tests were run -count=3
  without a flake.

Verification

go vet ./..., gofmt -l ., and go test ./... are clean, including
the build-boundary tests and the SDK under GOWORK=off.

[coderabbitai]

📝 Walkthrough

Summary

• Added black-box acceptance coverage for shell behavior when modules provide invalid, incompatible, malformed, or incomplete responses.
• Added fault-injecting and launch-tracking test fixtures to verify stable failures, bounded diagnostics, clean JSON output, and prevention of unintended execution.
• Expanded coverage for receipt validation, executable isolation, metadata and identity mismatches, protocol errors, process failures, timeouts, and partial results.
• Strengthened hanging-module tests to verify processes are fully terminated.
• Consolidated redundant protocol incompatibility coverage into the broader table-driven tests.
• Verified with go vet ./..., gofmt -l ., and go test ./....

Walkthrough

Adds black-box acceptance tests for fail-closed module execution. Coverage includes receipt path and metadata validation, executable resolution, runtime identity checks, malformed protocol frames, panic and hanging modules, diagnostic bounding, and structured-output isolation. New fault-injecting and launch-canary executables provide controlled protocol violations and launch markers. Existing status tests now record module PIDs, verify termination, and configure modules through explicit control-file contents.

Sequence Diagram(s)

sequenceDiagram
  participant AcceptanceTest
  participant Shell
  participant Module
  participant Output
  AcceptanceTest->>Shell: invoke installed module
  Shell->>Module: validate handshake and send invocation
  Module-->>Shell: return result or fault
  Shell->>Output: emit valid stdout and bounded stderr
  Shell-->>AcceptanceTest: return stable exit class

Loading

Suggested reviewers: hevayo

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: adding fail-closed acceptance tests for unsafe or faulty modules.
Description check	✅ Passed	The description is directly related to the test-only acceptance coverage added in this PR.
Linked Issues check	✅ Passed	The changes cover the listed black-box rejection and process-failure cases, including all acceptance criteria in #6.
Out of Scope Changes check	✅ Passed	The added fixtures and test updates stay within the stated goal and do not introduce unrelated scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

test/acceptance/failclosed_test.go (1)

477-500: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the restrictive ShellRange default in installLaunchCanary.

fixture.Install already applies a broad default when ShellRange is empty, so this ">=0.1.0 <1.0.0" override can make compatible reference modules reject shell versions outside that range instead of exercising the intended failure scenarios.

♻️ Proposed fix

-	if install.ShellRange == "" {
-		install.ShellRange = ">=0.1.0 <1.0.0"
-	}
 	if len(install.ProtocolVersions) == 0 {
 		install.ProtocolVersions = []int{testProtocolVersionNumber}
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/acceptance/failclosed_test.go` around lines 477 - 500, Remove the
explicit “>=0.1.0 <1.0.0” fallback from installLaunchCanary, leaving an empty
install.ShellRange for fixture.Install to default. Preserve the existing
namespace, version, source path, and protocol-version setup.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/acceptance/failclosed_test.go`:
- Around line 477-500: Remove the explicit “>=0.1.0 <1.0.0” fallback from
installLaunchCanary, leaving an empty install.ShellRange for fixture.Install to
default. Preserve the existing namespace, version, source path, and
protocol-version setup.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 553542c9-2e06-473a-b408-96b57efb5a6f

📥 Commits

Reviewing files that changed from the base of the PR and between e57aeee and 5f27d00.

📒 Files selected for processing (4)

• test/acceptance/failclosed_test.go
• test/acceptance/status_test.go
• test/acceptance/testdata/faultymodule/main.go
• test/acceptance/testdata/launchcanary/main.go