Repository navigation
Extend comment scanner with piped-stdin, runner-string, MDX, and worktree TypeScript support - #2821
Conversation
There was a problem hiding this comment.
oracle-review-shadow: failure for 8e6c2484736dd23dff4579f5e9d9ca08194d2672
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (470 code lines exceed 150).
Decision: SHIP WITH FIXES.
Reviewer: sol (codex gpt-6.1-sol, effort medium).
Four scanner defects must be fixed before merge: they permit false clean policy verdicts, including two regressions in previously detected comments. Review used the supplied diff and read-only inspection of the head checkout; no tests or network operations were run.
|
/oracle-review |
There was a problem hiding this comment.
oracle-verdict: failure for 6cb8c1203fe71ccf2ec227d21968c5dace9f8beb
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (559 code lines exceed 150).
Decision: SHIP WITH FIXES.
Reviewer: sol (codex gpt-6.1-sol, effort medium).
The four earlier defects are addressed, but five additional defects can produce incorrect comment-policy verdicts. Fix these before merge. Review was static and read-only; no tests were run.
Verified fixed: D1, D2, D3, D4. Resolve their review threads.
Scope: files changed since head 8e6c2484736dd23dff4579f5e9d9ca08194d2672 was reviewed.
6cb8c12 to
f4eed2d
Compare
There was a problem hiding this comment.
oracle-verdict: failure for f4eed2d403ec53aa254b96e249c52bf232b09038
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (614 code lines exceed 150).
Decision: SHIP WITH FIXES.
Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).
New runner/pipe/MDX coverage is largely sound and the five earlier defects are fixed, but three must-fix holes remain: split sudo/env/nohup commands are never unwrapped, printf %% is doubled, and the pipe-side guard suppresses a scannable first pipe.
Verified fixed: D5, D6, D7, D8, D9. Resolve their review threads.
Scope: files changed since head 6cb8c1203fe71ccf2ec227d21968c5dace9f8beb was reviewed.
Reviewers not available
- sol: absent (error)
- luna: absent (error)
There was a problem hiding this comment.
oracle-verdict: failure for 91d3d419c07592d17f2ea712a065153027efa769
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (651 code lines exceed 150; touches 2 areas).
Decision: SHIP WITH FIXES.
Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).
Prior D10/D11/D12 are fixed with fail-closed handling and regression tests. Four new must-fix defects remain in the changed files: runner-wrapped piped consumers and multi-hop pipes silently miss comments, MDX JSX matching false-positives on inline code, and the CI TypeScript install lands in the medium-test job rather than the comment-policy gate job.
Verified fixed: D10, D11, D12. Resolve their review threads.
Scope: files changed since head f4eed2d403ec53aa254b96e249c52bf232b09038 was reviewed.
…tree TypeScript support Teach the shell scanner to route echo/printf payloads piped into interpreters to the matching language parser, failing closed on dynamic producers and bare unmodeled-interpreter stdin sinks. Decompose single-string ssh, watch, xargs, env, sudo, and nohup arguments through a RUNNER_COMMANDS table with nested shell parsing, on both the shell and Python sides. Scan .mdx files with a dedicated handler covering prose fences, JSX comment expressions, and top-level import/export blocks. Resolve the TypeScript package from COMMENT_POLICY_TYPESCRIPT, the local results-explorer install, or the primary clone through git-common-dir so linked worktrees run the JavaScript scanner tests without extra setup.
… imports, and pipeline routing
91d3d41 to
cacbff9
Compare
There was a problem hiding this comment.
oracle-verdict: failure for cacbff9f340e2e0ee8e0b7d2002bbc5faeb9d503
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (736 code lines exceed 150; touches 2 areas).
Decision: SHIP WITH FIXES.
Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).
Prior defects D13-D16 are fixed. Three new must-fix defects remain in scripts/comment_payloads.py: piped env options silently miss stdin payloads, unparseable piped chunks bypass the coverage gate, and spaced MDX JSX comments are missed.
Verified fixed: D13, D14, D15, D16. Resolve their review threads.
Defects without a review thread
- High
scripts/comment_payloads.py:1519: D18: Unparseable piped-interpreter chunk silently passes coverage gate
The PR adds pipe-to-interpreter shapes to SHELL_INLINE_INTERPRETER, so shell_command_payloads() parses sources containing e.g. '| python3'. But the unparseable-chunk fallback still uses chunk_can_hold_comment(), which only checks SHELL_MODELED_INLINE and SHELL_UNMODELED_NAME and has no pipe-to-interpreter branch. An unparseable chunk whose only executable shape is a pipe (e.g. an unterminated quote containing '| python3') is therefore silently skipped instead of raising a coverage-error, weakening the fail-closed gate for the newly added shape.
Defects outside the diff count like the others: fix them before merge.
Scope: files changed since head 91d3d419c07592d17f2ea712a065153027efa769 was reviewed.
|
D18 is fixed in 0429b86. The unparseable-shell fallback now fails closed for non-heredoc pipe-to-interpreter and dynamic-pipe shapes; heredoc payloads remain handled by the dedicated shell adapter. The regression requires |
There was a problem hiding this comment.
oracle-verdict: failure for 0429b866258e9298c71344bd2dbddc8a65cd940d
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (767 code lines exceed 150; touches 2 areas).
Decision: SHIP WITH FIXES.
Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).
Three new silent-miss coverage holes in the piped-stdin adapter block merge; earlier D17-D19 are fixed.
Verified fixed: D17, D18, D19. Resolve their review threads.
Scope: files changed since head cacbff9f340e2e0ee8e0b7d2002bbc5faeb9d503 was reviewed.
There was a problem hiding this comment.
oracle-verdict: failure for 5c33b6075990429f875b441bc43665f6a1e1a991
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (876 code lines exceed 150; touches 2 areas).
Decision: SHIP WITH FIXES.
Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).
Prior piped-runner gaps D20-D22 are now fail-closed or scanned. Two new silent-miss paths remain in the added piped logic: chained sudo/env consumers and bash -O/+O operands, both dropping piped stdin that real shells execute.
Verified fixed: D20, D21, D22. Resolve their review threads.
Scope: files changed since head 0429b866258e9298c71344bd2dbddc8a65cd940d was reviewed.
|
/oracle-review |
There was a problem hiding this comment.
oracle-verdict: failure for 019949d28e23a9dff593f9e2da3b72430cc61ac1
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (876 code lines exceed 150; touches 2 areas).
Decision: SHIP WITH FIXES, carried from head 5c33b6075990429f875b441bc43665f6a1e1a991.
The patch is unchanged since head 5c33b6075990429f875b441bc43665f6a1e1a991 was reviewed, so its decision is carried to this head without running a reviewer. Its open review threads still apply.
Still open
- High
scripts/comment_payloads.py:1790-1799: D23: Chained sudo/env piped consumers silently missed - High
scripts/comment_payloads.py:1907: D24: Bash -O/+O option argument treated as script file
|
/oracle-review |
There was a problem hiding this comment.
oracle-verdict: failure for 1bee4cc6771ce1939c9eee57c0f15b26157ef2ad
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (878 code lines exceed 150; touches 2 areas).
Decision: SHIP WITH FIXES.
Reviewer: muse (muse muse-spark-1.3-contributor, effort medium).
New piped-stdin and MDX scanning closes D23/D24 but adds two silent-miss holes: SQL piped consumers are dropped instead of failing closed, and MDX type-like exports are never sent to the TypeScript scanner.
Verified fixed: D23, D24. Resolve their review threads.
Scope: files changed since head 019949d28e23a9dff593f9e2da3b72430cc61ac1 was reviewed.
|
/oracle-review |
1 similar comment
|
/oracle-review |
|
Merge order is deliberate: this scanner PR first. The conflict scan found direct overlap with #2829 in |
There was a problem hiding this comment.
oracle-verdict: pending for e741df1b75275da3c42c83afd990c97e94f5dcbb
This result supersedes those for earlier heads; their open review threads still apply.
Tier: medium-high (972 code lines exceed 150; touches 2 areas).
Scope: files changed since head 1bee4cc6771ce1939c9eee57c0f15b26157ef2ad was reviewed.
Reviewers not available
- muse: absent (invalid)
- sol: absent (error)
- luna: absent (error)
- sonnet: skipped: same family as the author (claude)
- agy: disabled (CI authentication and quota output are not yet calibrated, and the workflow has no agy job)
|
Stand-in oracle review: APPROVE e741df1 |
…and worktree TypeScript support (#2821)" This reverts commit 3b4d7e5. The revert also refreshes the remediation-contract audit replay stamps (the restored replay_sha is unreachable; replay re-ran at the revert tree: 112 passed) and applies behavior-preserving executable-payload adapter restructures to seven workflow/shell files. The adapter changes are required because transition coverage-errors are never grandfathered: without them the revert cannot land. They mirror the proven remediation on the #2907 branch.
Implements scanner support for shell-piped interpreter input, runner command strings, MDX content, and linked-worktree TypeScript discovery.
echoandprintfproducers are reconstructed only for supported transformations; dynamic or unsupported input fails closed.