Skip to content

Remove the static runtime-purity scan; enforce purity in review - #15

Open
ktsaou wants to merge 10 commits into
sow-0045/reader-positionsfrom
sow-0071/runtime-purity-guard
Open

ktsaou wants to merge 10 commits into
sow-0045/reader-positionsfrom
sow-0071/runtime-purity-guard

Conversation

@ktsaou

@ktsaou ktsaou commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

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.py was a text scan meant to prove that core SDK code never launches programs or probes the host. In practice:

  • it scanned 12 of about 100 core source files (go/journal alone has about 60);
  • it caught exec.Command( but missed exec.CommandContext, os.StartProcess and syscall.ForkExec;
  • no CI workflow ran it.

It gave false confidence, and a code review finds these calls directly.

Changes

  • Deletes 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.
  • SOW-0098's validation plan no longer cites the removed test.
  • SOW-0071 is closed; both status indexes are updated.

Validation

  • A manual review of every non-test file in go/journal and the Rust SDK library crates found no core impurity. Only the opt-in lock helper reads /proc or calls sysctlbyname.
  • No runtime code, public API or docs change. The purity contract in AGENTS.md, the spec and docs/Getting-Started.md is 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.md rule; a manual review of the core SDK confirms it is clean, with only the writer-lock helpers reading /proc or calling sysctlbyname.

  • Deletes tests/runtime_purity/.
  • AGENTS.md now directs reviewers to search diffs for process-launch and host-access forms (including exec.Command, exec.CommandContext, and Command::new), and names the in-package writer-lock helper files that are the only core code allowed to use them.
  • Updates SOW-0071 to completed, marks the scan acceptance criterion as superseded, and adjusts SOW-0098's validation plan accordingly. Both SOW status indexes record the completion.

Written for commit 31acf94. Summary will update on new commits.

View guided diff Turn on auto-fix

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.
@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 841f325e-0057-4ad6-83b2-be18474404dd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@codacy-production

Copy link
Copy Markdown
Contributor

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

⚠️ Resolve issues before merging

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

Comment thread AGENTS.md Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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
Loading

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread AGENTS.md Resolved
Comment thread .agents/sow/SOW-status.md Resolved
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.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant