Repository navigation
Enforce positive and negative evidence proofs - #12
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5e5eb300f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| reader.join(timeout=1) | ||
| process.stdout.close() | ||
| if reader.is_alive() or read_error: |
There was a problem hiding this comment.
When a proof shell exits after backgrounding a child that inherits stdout, process.wait() succeeds while the reader remains blocked on that child's open pipe. After the one-second join expires, process.stdout.close() can itself block waiting for the reader, so execution never reaches the reader.is_alive() error and the configured proof timeout no longer bounds the mutation; this can leave the workflow lock held indefinitely. Stop the process group or otherwise unblock the reader before closing the stream when it is still alive.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
The new proof runner’s timeout/overflow handling can hang indefinitely and can mask overflow errors, which is risky in an enforcement path invoked during completion/closeout.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR upgrades the Coordinator workflow state to schema v8 and enforces a new “proof contract” for non-legacy nodes: every node must carry descriptive evidence plus positive/negative proof commands, which are executed at node completion and again during workflow closeout to validate outcomes and persist positive output.
Changes:
- Bump workflow state schema to v8, introducing proof contracts (
evidence_*_proof_command), proof metadata, and legacy proof exemptions for pre-migration nodes. - Add bounded proof command runner and integrate proof execution into node completion, judge completion, runtime reconcile, and workflow closeout flows.
- Expand test coverage and update user-facing documentation to describe proof requirements, closeout behavior, and security considerations.
File summaries
| File | Description |
|---|---|
| tests/test_state.py | Updates state tests for schema v8 and adds extensive proof-enforcement scenarios. |
| tests/test_proofs.py | Adds unit tests for the proof command runner (cwd/env/output/timeout/limits). |
| tests/test_dynamic_runtime.py | Extends dynamic runtime tests to require proof bundles and validate proof propagation. |
| skill/SKILL.md | Documents the new proof contract requirements and lifecycle semantics. |
| skill/scripts/lib/coordinator/state/store.py | Implements schema v8, validates proof contracts, executes proofs on completion/closeout, and upgrades v6/v7 state. |
| skill/scripts/lib/coordinator/state/proofs.py | Introduces bounded shell execution for proof commands with output limits and timeouts. |
| skill/scripts/lib/coordinator/cli/state.py | Updates CLI surface to require proof plan input for reconcile and removes mandatory result/evidence for proof-enforced completion. |
| skill/references/workflow-protocol.md | Updates protocol docs for proof execution at completion and closeout. |
| skill/references/state-schema.md | Updates schema docs to v8, including proof fields, legacy exemptions, and closeout reruns. |
| skill/references/model-routing.md | Updates schema version references used by routing docs. |
| skill/references/dynamic-runtime-graphs.md | Documents proof bundles for runtime reconcile/expansion and proof-enforced loops. |
| skill/references/complexity-accounting.md | Updates digest and plan/split documentation to include proof contract fields. |
| skill/README.md | Highlights v8 proof requirements and closeout enforcement in the skill overview. |
| SECURITY.md | Adds security guidance for proof commands (inherited env, no secrets in output/commands). |
| README.md | Updates top-level README to describe proof-enforced closeout behavior for v8 nodes. |
| README.zh-CN.md | Mirrors proof-enforcement docs in the Chinese README. |
| README.pt-BR.md | Mirrors proof-enforcement docs in the Portuguese README. |
| README.es.md | Mirrors proof-enforcement docs in the Spanish README. |
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| except subprocess.TimeoutExpired as exc: | ||
| _stop_process(process) | ||
| process.wait() | ||
| reader.join(timeout=1) | ||
| process.stdout.close() | ||
| raise ProofExecutionError( | ||
| f"proof command exceeded the {timeout}-second timeout" | ||
| ) from exc |
| if reader.is_alive() or read_error: | ||
| raise ProofExecutionError("unable to capture proof command output") | ||
| if overflow.is_set(): | ||
| raise ProofExecutionError(f"proof command output exceeds {output_limit} bytes") |
| def proof_contract(node_id: str, *, succeeds: bool = True) -> dict[str, str]: | ||
| output = f"{node_id} positive proof" | ||
| positive_body = f"print({output!r})" + ("" if succeeds else "; raise SystemExit(1)") | ||
| negative_body = "raise SystemExit(1)" if succeeds else "print('negative proof reproduced')" | ||
| return { |
| def python_command(body: str) -> str: | ||
| arguments = [sys.executable, "-c", body] | ||
| return ( | ||
| subprocess.list2cmdline(arguments) | ||
| if os.name == "nt" | ||
| else shlex.join(arguments) | ||
| ) |
| def proof_contract(node_id: str, *, succeeds: bool = True) -> dict[str, str]: | ||
| output = f"{node_id} positive proof" | ||
| positive_body = f"print({output!r})" + ("" if succeeds else "; raise SystemExit(1)") | ||
| negative_body = "raise SystemExit(1)" if succeeds else "print('negative proof reproduced')" | ||
| return { |
Summary
Validation