Repository navigation
Conversation
The scan in tests/runtime_purity covered 12 of about 100 core source files, missed several process-launch forms and never ran in CI, so it gave false confidence. Runtime purity stays an AGENTS.md rule; review now enforces it with the forms to search for and the location of the in-package writer-lock helper. A manual review confirms the core SDK is clean. Closes the SOW-0071 regression.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull Request Overview
The PR removes the only automated purity enforcement while relying on manual review. This can miss indirect, aliased, or newly reachable process and host-access paths across untouched core sources.
A key acceptance criterion is therefore not adequately addressed: purity enforcement is documented but not equivalently enforced or validated. The recommended whole-core review and audit scenarios are also absent from executable test or coverage evidence.
Test suggestions
- Verify the removed purity test cannot still be invoked or referenced by active workflows, scripts, skills, or validation plans.
- Review all non-test core Go and Rust SDK files for prohibited process launches and host access, allowing only the documented writer-lock helpers.
- Run the SOW audit and confirm the updated status and validation metadata are internally consistent.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify the removed purity test cannot still be invoked or referenced by active workflows, scripts, skills, or validation plans.
2. Review all non-test core Go and Rust SDK files for prohibited process launches and host access, allowing only the documented writer-lock helpers.
3. Run the SOW audit and confirm the updated status and validation metadata are internally consistent.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Architecture diagram
sequenceDiagram
participant Dev as Developer
participant Review as Code Review
participant CI as CI Pipeline
participant AGENTS as AGENTS.md Rules
participant Core as Core SDK Code
participant Lock as Writer-Lock Helper
Note over Dev,CI: Runtime Purity Enforcement Flow
Dev->>Core: Write/modify core SDK code
Core->>Core: Implement reader/writer/facade modules
Dev->>Review: Submit PR touching core SDK
alt Review process for core SDK changes
Review->>AGENTS: Read Runtime Purity Architecture rules
Review->>Review: Search diff for forbidden patterns
alt Forbidden patterns detected
Note over Review: Patterns: os/exec, os.StartProcess,<br/>syscall.ForkExec/Exec, std::process,<br/>host paths (/proc, /etc/machine-id), sysctl
Review-->>Dev: Request changes - remove impurity
else No forbidden patterns detected
Review->>Core: Validate only lock helpers use host access
Note over Core,Lock: Lock helper files:<br/>go/journal/lock*.go, file_lock*.go,<br/>rust/src/crates/journal-core/src/file/lock.rs
Core->>Lock: Allow writer-lock operations
Lock->>Lock: Read /proc, call sysctlbyname
Review-->>Dev: Approve - core code is pure
end
end
Note over CI,Lock: CI runs without purity scan test<br/>(tests/runtime_purity/ removed)
CI->>CI: Run functional tests only
CI->>Core: Execute runtime validation
CI-->>Dev: Report test results
Note over Core: Purity contract enforced by review,<br/>not automated scanning
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Adds exec.Command, exec.CommandContext and Command::new to the review search list, marks the scan acceptance criterion as superseded by the user decision, records the clean audit, and updates the stale paused-status note.
…-guard # Conflicts: # .agents/sow/SOW-status.md
…-guard # Conflicts: # .agents/sow/SOW-status.md
…-guard # Conflicts: # .agents/sow/SOW-status.md
Stacked on #14 (which is stacked on #13). Merge #13, then #14, then this. Completes the SOW-0071 regression (
.agents/sow/done/SOW-0071-20260530-runtime-purity-and-optional-platform-services.md).Why
tests/runtime_purity/test_core_runtime_purity.pywas a text scan meant to prove that core SDK code never launches programs or probes the host. In practice:go/journalalone has about 60);exec.Command(but missedexec.CommandContext,os.StartProcessandsyscall.ForkExec;It gave false confidence, and a code review finds these calls directly.
Changes
tests/runtime_purity/.AGENTS.md(Runtime Purity Architecture): review enforces the rule. It lists the process-launch and host-access forms to search for, and the in-package writer-lock helper files (go/journal/lock*.go,file_lock*.go,journal-core/src/file/lock.rs), which are the only core files allowed to use them.Validation
go/journaland the Rust SDK library crates found no core impurity. Only the opt-in lock helper reads/procor callssysctlbyname.AGENTS.md, the spec anddocs/Getting-Started.mdis unchanged..agents/sow/audit.sh: complete and clean.Summary by cubic
Removes the static runtime-purity scan and enforces the purity rule through code review instead.
The old test covered 12 of about 100 core source files, missed several process-launch variants, and never ran in CI, so it gave false confidence. Runtime purity stays an
AGENTS.mdrule; a manual review of the core SDK confirms it is clean, with only the writer-lock helpers reading/procor callingsysctlbyname.tests/runtime_purity/.AGENTS.mdnow directs reviewers to search diffs for process-launch and host-access forms (includingexec.Command,exec.CommandContext, andCommand::new), and names the in-package writer-lock helper files that are the only core code allowed to use them.Written for commit 31acf94. Summary will update on new commits.