Skip to content

Keep invalid Studio proof checkout failures in closed CLI output - #8686

Merged
sfmskywalker merged 2 commits into
codex/elsa-integration-programfrom
codex/elsa-integration-studio-npm-8679
Oct 9, 2026
Merged

sfmskywalker merged 2 commits into
codex/elsa-integration-programfrom
codex/elsa-integration-studio-npm-8679

Conversation

@sfmskywalker

@sfmskywalker sfmskywalker commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Malformed checkout and output paths could escape the Studio npm proof CLI with raw Git diagnostics or a traceback instead of closed failure JSON. Normalize expected Git/OS source failures to source-status-unavailable, capture Git stderr, and close other ordinary exceptions in the existing CLI try with unexpected-failure. Failures exit 1 without exposing exception text; rejected paths are not adopted for receipt creation.

Refs #8679, #8685 and #8194. This corrective slice targets codex/elsa-integration-program; the existing final main PR #8685 will adopt it after normal reviewed integration. #8679 remains open until actual main acceptance.

Exact source head: a221888910e96711d07e66f86f6e698edde6524b, tree 0407d373f981c9b29e491b739051e200214682ea; program base is 88ca815cf4d973b681449734ec9c5e088ed7629b. Only the proof controller and owning tests change. Producer, workflow, npm manifests, lifecycle helper and dependencies are unchanged.

Actual subprocess regressions reproduce the original failures and cover missing/non-Git/file roots, a blocked output parent using a real clean Git fixture, and root/output path cycles. They require closed JSON, empty stderr and unchanged source/sentinels with no proof output. Specific ProofError codes remain first; parser/SystemExit/KeyboardInterrupt stay outside the fallback. Valid checkout/commit mismatch still rejects as source-checkout; post-admission receipts and source-drift finalization are preserved.

Separate lead and independent Luna runs each passed 26 normal + 26 optimized contracts with zero skips; syntax, full corrective diff and exact file hashes passed. Independent APPROVE + HIGH at this head is bound by report SHA-256 f610b8f5f77878169336d45c1a3420caf2622c7db9384dc1fb45278ff9705fea; the lead read the report and verified its hash. Lead source QA receipt SHA-256: 9885ef8f6f7b658d6c27c52198ae2fb8d69d1d27e15a9951a6f168a3c729cc0e.

Fresh exact-a221 hosted pair run 37911724081 is terminal SUCCESS; original-artifact readback is accepted by the lead and independent reviewer with APPROVE + HIGH. ZIP, original tarballs, complete inventories, source bindings, paired integrity, lock entries, ownership ledger and WASM bytes passed; consumer execution remains provider/receipt-backed because consumer output directories were not retained. Current-head Greptile is 5/5 and CodeRabbit completed actual a221 coverage with no actionable code comments. Broader exact-head CI and normal shared/main integration remain pending. The successful 902 hosted artifact readback remains attributed to source 902/run 37909801810, not this final correction. Four superseded 902 CI runs were cancelled/requested cancelled; those are not passing evidence. No task acceptance, publication or authority activation is claimed.

Summary by CodeRabbit

  • Bug Fixes
    • The proof command now returns clear JSON errors when source access or other unexpected failures prevent completion.
    • Invalid source roots and problematic output paths are rejected without creating output or changing existing files.
  • Tests
    • Added command-line checks for failure responses and protection against unintended file changes.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5088c294-bca1-40f1-bb97-0e938e1618fd

📥 Commits

Reviewing files that changed from the base of the PR and between 90286d0 and a221888.


📒 Files selected for processing (2)
  • scripts/integration-program/prove_studio_npm_pair.py
  • scripts/integration-program/test_prove_studio_npm_pair.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.



📝 Walkthrough

Walkthrough

The Git helper now maps operating-system and Git command failures to source-status-unavailable. The CLI returns JSON with unexpected-failure for otherwise-unhandled exceptions. New CLI tests cover invalid source roots, output paths, and symlink cycles.

Changes

Proof CLI failure handling

Layer / File(s) Summary
Normalize Git and unexpected failures
scripts/integration-program/prove_studio_npm_pair.py
The Git helper maps operating-system and Git command failures to source-status-unavailable. The CLI returns a JSON failure with unexpected-failure for otherwise-unhandled exceptions.
Verify CLI failure responses
scripts/integration-program/test_prove_studio_npm_pair.py
CLI tests check failure exit codes, JSON responses, empty stderr, and filesystem effects for missing, non-Git, and file-valued roots, output paths with a regular-file parent, and symlink cycles.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix


Merge Risk

Merge Risk: ⚪ Minimal · up to a2218

The proof CLI’s failure responses and invalid-path tests show no identified merge-blocking issue. Normal checks can proceed.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to a2218

The CLI rejects invalid inputs with predictable failures without exposing ordinary exception text. The reviewed change preserves source validation, output ownership checks and execution authority. No introduced or worsened security concern was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed behavior affects diagnostics visible to the CLI caller for supplied checkout and output paths. It does not expand admission, add a production caller, or introduce publishing authority; the existing receipt continues to record publisher_authority=false.

Trust Boundaries and Controls

  • observed — Path and identity inputs still cross the same admission checks before build execution. Runner retains its restricted child-process environment and private logs. The Git helper still inherits the invoking process environment; this PR changes diagnostic capture, not Git privileges or credential isolation.

Resilience and Maintainability Implications

  • observed — Post-admission failure containment remains intact: receipts start unsuccessful, execution failures retain closed codes, and source-drift or status-check failure revokes success. The existing receipt regression checks unsuccessful, unpublished output and retained source-drift information.



🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main change: keeping invalid Studio proof checkout failures in closed CLI output.
Description check Passed The description is detailed, on-topic, and covers the purpose, problem, solution, scope, verification, test results, and known pending checks. It does not follow every template heading or checklist it…


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


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

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

Copy link
Copy Markdown
Member Author

Elsa 3 Code Review — independent source review relay

APPROVE + HIGH @ 90286d0

Independent Luna 6 Extra High reviewed the complete two-file correction, exact parent/tree/hashes and actual CLI failure boundaries. Missing/non-Git/file roots return closed source-status-unavailable, exit 1, empty stderr, no output and no fixture mutation. A real valid checkout with a mismatched commit still returns source-checkout. The post-admission receipt and source-drift finalization gates remain intact. Separate lead and independent runs each passed 24 normal + 24 optimized contracts with zero skips; no material source finding remains.

Lead read the full independent report and verified SHA-256 25bb56c6a5805514603a8c068f097a3f43734b0536848824aa515a7cc951d590; lead source QA receipt SHA-256 c14febf333f37995b5c2ec0eb411341cd5c0268ebfd87b4f340a80feb4c15944. This relays independent approval, not author self-approval. Current-head hosted/archive proof, CI/Greptile and actual shared/main integration remain pending; no task acceptance or publication is claimed.

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@greptile-apps

greptile-apps Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] No blocking issues were identified in this corrective slice. This assessment does not establish completion of the separate hosted proof or main-branch integration gates.

Summary

This PR keeps invalid checkout and output paths from leaking Git errors or Python tracebacks.

  • Studio proof failures now return closed JSON from the CLI.

No actionable issues were identified. This assessment is based on code inspection; no additional tests were executed for this review.

Reviews (2) · Last reviewed commit: "fix: close Studio npm proof CLI filesyst..." · Reviewed by Greptile

Copy link
Copy Markdown
Member Author

Lead follow-up at 90286d0 found another instance of the same closed CLI boundary: an output path beneath a regular file exits 1 with empty stdout and a traceback, while preserving the sentinel and creating no proof output. Sanitized actual reproduction receipt SHA-256: 4fb71d9be8aa778f8d7509c67f3e5e7480354f77885875121f965a35b90e8f65. The narrow Git-root correction passes its three regressions, but main's fallback must also normalize filesystem/path admission failures without exposing exception text. A minimal correction and actual CLI regressions are in progress within this PR; no new Task or producer/helper/dependency changes. Source approval/CI will be refreshed after the final correction. The successful 902 hosted artifact proof remains truthfully attributed to that commit and is not final-head acceptance.

Copy link
Copy Markdown
Member Author

Original hosted artifact readback accepted for source 90286d0 only.

Run 37909801810, attempt 1/job 113751949346, completed successfully. Original artifact 11606650203 is 94,761,773 bytes and matches provider SHA-256 a61104f736a44af70a607c981dac35b46b5a2eb6356935b56f4c0db3a1761336. Lead and independent Luna readers verified all five ZIP files, both original tarballs (1,059/1,057 members), exact source/tree and proof version 0.0.0-proof.37909801810.1, embedded paired SHA-512 integrity, the reviewed lifecycle helper, all 1,050 ownership asset hashes and 67 unchanged third-party lock records. SDK/runtime binaries and both wrapper copies were checked; all 20 recorded commands succeeded, source was unchanged and publication/authority were false.

Lead readback SHA-256 c04b2bc24f8f425cdeb5d3c03fc35d95b7fc1e707c2fa9b89a2de1a6f6fd6578; independent report 3826811701b8e8c93e6bda42e25cc09c4eecc0d276769131aff43eecbf4885f1; independent machine receipt f5ba1a4b0b1f6ee9fcefb3420a52e10901bf14785574e07d2862a82e5533c638. The lead read the full independent report/JSON, verified their hashes and agreement with the lead readback.

Consumer node_modules/public/Vite directories were not retained or independently re-executed; those lifecycle/module/Vite outcomes remain provider-step, reviewed-controller and receipt-backed. This is package/static-consumer evidence, not browser/runtime/publication certification. The additional pre-admission output-path defect in the preceding comment still requires the bounded correction; this artifact acceptance does not accept #8679 or a future source head.

Copy link
Copy Markdown
Member Author

Elsa 3 Code Review — independent final CLI-boundary review relay

APPROVE + HIGH @ a221888

Independent Luna 6 Extra High reviewed the full corrective delta from program base 88ca: only the Git diagnostic mapping and three-line outer CLI fallback plus meaningful actual subprocess contracts. Both lead and independent runs passed 26 normal + 26 optimized tests with zero skips. Missing/non-Git/file roots, a regular-file output parent with a real clean Git source, and root/output cycles reject with closed JSON and empty stderr without changing source/sentinels or creating proof output. Specific ProofError codes remain first, parser/SystemExit/KeyboardInterrupt stay outside, and successful-source admission plus post-admission receipt/source-finalization gates are preserved.

Lead read the complete independent report and verified SHA-256 f610b8f5f77878169336d45c1a3420caf2622c7db9384dc1fb45278ff9705fea; lead complete-delta source QA receipt SHA-256 9885ef8f6f7b658d6c27c52198ae2fb8d69d1d27e15a9951a6f168a3c729cc0e. The lead removed only the review-created bytecode files and verified the exact writer clean before push. This relays independent approval, not author self-approval. Fresh a221 hosted/archive/CI/Greptile and actual shared/main integration remain pending; prior 902 artifact evidence retains its actual source/run.

Copy link
Copy Markdown
Member Author

@greptileai

Pushed final CLI-boundary correction a221888. This completes the filesystem/path admission fallback in addition to the Git-root fix; actual CLI controls and separate lead/independent 26 normal + 26 optimized contracts pass. Review the current head; prior 902 review/proof remains attributed to 902.

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Member Author

Accepted original paired-package artifact readback for exact source a221888910e96711d07e66f86f6e698edde6524b, tree 0407d373f981c9b29e491b739051e200214682ea only.

Hosted run 37911724081, attempt 1, job 113758208693 is terminal SUCCESS. Original artifact 11607565131 is 94,760,475 bytes, SHA-256 3f05853f6fad6fbac7389d9cf12825bc0914e457feb5dd79f57db7914a1d96fe. The lead and independent reviewer checked original ZIP CRC, safe five-file extraction, both original tarballs, complete member inventories, embedded source/version manifests, paired npm SHA-512 integrity, and exact source lifecycle helper bytes.

The WASM archive has 1,059 members; the React archive has 1,057. Both use proof version 0.0.0-proof.37911724081.1. The 1,050 ownership-ledger assets match both archives; the reconstructed producer lock and all 67 consumer third-party records match exact source. Actual runtime/host WASM headers, exports and identical copies were read back. All 20 producer/consumer commands are successful in the receipt; published and publisher_authority are false.

Independent artifact verdict: APPROVE + HIGH. The lead fully read its report, verified report SHA-256 c99ad5a26fa5a6b6130df1c2bf615b7b0bd50be902c4138588ed293119ada5e4 and machine evidence SHA-256 5aeeb236ae629c6d450630cfc05c2aced92b732834ebbb0deafeebfb923745a6, and compared source/tree, ZIP/receipt digests, integrity/member counts, ledger and dependency counts with its own readback (SHA-256 b1d9d47e626a32b28fdfbb87b96ebb4240553a167ca82349e6af10406eda38b0): all agreed.

Limits: retained consumer output directories were unavailable, so install/lifecycle/ESM/CommonJS/Vite, stale-file removal and unmanaged-file preservation remain exact-head provider/receipt-backed, not independently replayed. No browser/runtime certification, publication, publisher authority, later merge-head execution or broader task acceptance is claimed.

Independent source APPROVE + HIGH, Greptile 5/5 and completed CodeRabbit coverage are bound to this exact a221 head. CodeRabbit identified no actionable code comments; its generic docstring-coverage advisory does not represent a repository merge gate. Remaining exact-head CI and normal program/main integration still must pass. #8679 remains the sole active Task; #8683 remains queued.

Copy link
Copy Markdown
Member Author

Final slice merge gates pass at exact head a221888910e96711d07e66f86f6e698edde6524b, tree 0407d373f981c9b29e491b739051e200214682ea, against program base 88ca815cf4d973b681449734ec9c5e088ed7629b.

All six exact-head workflows are terminal SUCCESS. All 21 check entries are terminal: 14 SUCCESS and seven explicitly expected SKIPPED (publication, coverage deployment and unselected conditional proof/test paths). Skips are not counted as executed proof. Independent source APPROVE + HIGH, Greptile 5/5, actual CodeRabbit a221 coverage, lead source QA and original hosted artifact acceptance passed. Fresh inline review threads are empty. GitHub reports mergeable/clean.

The lead reviewed the complete final corrective delta; no high-priority issue remains. Gate receipt SHA-256: 20f4eab71f60d38d69eb1ac2424117a6802dfa45990a04ab55614a3b01fcea77. Proceed with normal expected-head merge into the program branch, then verify actual ordered parents/tree. #8685 main integration needs its own exact-head review/CI gates; #8679 remains open until main acceptance. No publication, publisher/ref activation or archival is claimed.

@sfmskywalker
sfmskywalker merged commit 1e85268 into codex/elsa-integration-program Oct 9, 2026
21 checks passed
@sfmskywalker
sfmskywalker deleted the codex/elsa-integration-studio-npm-8679 branch October 9, 2026 09:59

Copy link
Copy Markdown
Member Author

Normal program merge verified: #8686 merged at 1e852684100706c0c4993dff3f3742c2ea31c5ce, with ordered parents 88ca815cf4d973b681449734ec9c5e088ed7629b and reviewed a221888910e96711d07e66f86f6e698edde6524b. Actual tree 0407d373f981c9b29e491b739051e200214682ea is identical to the reviewed source tree with zero delta. Final exact-a221 gate passed before merge. Existing final main PR #8685 now adopts this merge head; independent integration review and its own exact-head CI/Greptile gates are pending. #8679 remains open. Original hosted pair/archive evidence retains its actual A221 source/run attribution. No publication, authority activation or archival occurred.

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