Repository navigation
fix(opencode-review): discover multi-line run: blocks in safe_pytest_command - #949
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthrough
Changespytest 명령 탐색 확장
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🔵 Low · up to The PR expands safe pytest discovery to recognize commands inside YAML block scalars while preserving the existing execution restrictions, restoring configured coverage and docstring checks. It is mergeable with owner awareness of a bounded test-isolation issue that may retain a temporary executable path across tests, plus minor documentation follow-up. Sequence Diagram(s)sequenceDiagram
participant Workflow as GitHub Actions workflow
participant Discover as discover_commands
participant Extractor as _iter_run_command_lines
participant Validator as _is_pytest_argv
Workflow->>Discover: workflow text 전달
Discover->>Extractor: run 항목 탐색
Extractor-->>Discover: 단일 행 및 블록의 물리적 명령 행
Discover->>Validator: argv 안전성 검증
Validator-->>Discover: 인식된 pytest 명령
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Review follow-up on current head. CodeRabbit's first-module finding is valid and fixed: `python attacker.py -m pytest` and `coverage run attacker.py -m pytest` are now rejected. The first `-m`/`--module` after recognized flags must be pytest (or `python -m coverage run … -m pytest`). Reality tests cover those attacker shapes. The 2026-08-13 session-log citation was replaced with a permanent PR/test reference. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent @cwl-noema-review Review-only request for exact current head |
Pull request was converted to draft
|
Returned to Draft because the exact current tree mixes the safe multi-line pytest-command discovery repair with an unrelated trusted-uv materializer branch. The bounded buyer-facing fix is strong: offer lines from YAML literal/folded Rebuild from protected |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
f564761dc7cac7b67e0233bd3da4d84b52f0057a. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Python 3.10 compatibility contract check run: cancelled (https://github.com/ContextualWisdomLab/.github/actions/runs/31716471481/job/94502355219)
- Python 3.14 full quality gate check run: cancelled (https://github.com/ContextualWisdomLab/.github/actions/runs/31716471481/job/94502355471)
- Strix Changed Path Quality CI/exact-head-path-policy: CANCELLED (https://github.com/ContextualWisdomLab/.github/actions/runs/31716471506/job/94502355203)
- Trusted uv Materializer Quality CI/Python 3.10 compatibility contract: CANCELLED (https://github.com/ContextualWisdomLab/.github/actions/runs/31716471481/job/94502355219)
- Trusted uv Materializer Quality CI/Python 3.14 full quality gate: CANCELLED (https://github.com/ContextualWisdomLab/.github/actions/runs/31716471481/job/94502355471)
- exact-head-path-policy check run: cancelled (https://github.com/ContextualWisdomLab/.github/actions/runs/31716471506/job/94502355203)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (4 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (4 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs (2 files)"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs (2 files)"]
R2 --> V2["docs review"]
Evidence --> S3["CI script (2 files)"]
S3 --> I3["review and security gate shell path"]
I3 --> R3["Review risk: CI script (2 files)"]
R3 --> V3["bash -n plus Strix self-test"]
Evidence --> S4["Test (2 files)"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test (2 files)"]
R4 --> V4["targeted test run"]
OpenCode Review Overview
|
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
73636bbc5d79f05845ac12a43e6cc5d80f3ea54e. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Strix Security Scan/strix: FAILURE (https://github.com/ContextualWisdomLab/.github/actions/runs/32536544525/job/96938440001)
- Strix Security Scan/strix: failure (https://github.com/ContextualWisdomLab/.github/actions/runs/32536544525/job/96938440001)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Docs (2 files)"]
S1 --> I1["operator or user guidance"]
I1 --> R1["Review risk: Docs (2 files)"]
R1 --> V1["docs review"]
Evidence --> S2["CI script: safe_pytest_command.py"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script: safe_pytest_command.py"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test: test_safe_pytest_command.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_safe_pytest_command.py"]
R3 --> V3["targeted test run"]
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
73636bbc5d79f05845ac12a43e6cc5d80f3ea54e. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Strix Security Scan/strix: FAILURE (https://github.com/ContextualWisdomLab/.github/actions/runs/32536544525/job/96938440001)
- Strix Security Scan/strix: failure (https://github.com/ContextualWisdomLab/.github/actions/runs/32536544525/job/96938440001)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Docs (2 files)"]
S1 --> I1["operator or user guidance"]
I1 --> R1["Review risk: Docs (2 files)"]
R1 --> V1["docs review"]
Evidence --> S2["CI script: safe_pytest_command.py"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script: safe_pytest_command.py"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test: test_safe_pytest_command.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_safe_pytest_command.py"]
R3 --> V3["targeted test run"]
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head evidence but found unresolved reviewer or review-agent threads before approval.
Findings
1. HIGH .github/workflows/opencode-review.yml:1 - Unresolved reviewer thread blocks automated approval
- Problem: OpenCode reached an APPROVE control result, but the approval step found unresolved, non-outdated human or review-agent thread evidence on the current pull request.
- Root cause: Reviewer and review-agent feedback can arrive after bounded model evidence is prepared, so the approval step must re-query GitHub immediately before publishing an approval.
- Fix: Address or resolve the listed reviewer thread(s), then re-run OpenCode on the current head.
- Regression test: Keep the approval gate querying reviewThreads(first: 100) after model output and before create_pull_review APPROVE, including bot review agents other than OpenCode itself.
Review thread evidence
Latest unresolved reviewer thread evidence
docs/doctoring/safe-pytest-first-module-target.md line 36
-
Latest reviewer comment: @opencode-agent at 2026-08-22T05:24:00Z
-
Comment URL: #949 (comment)
-
Comment excerpt: ### P1 Make the security-standard attribution independently verifiable / - Location: 'docs/doctoring/safe-pytest-first-module-target.md:36' / - Problem: The document presents the first-module-target behavior as preventing a CWE-88 risk and cites OWASP, but the trusted review evidence contains only the bare external URL and no authoritative source content or verification result. This leaves a material security rationale unsupported. / - Root cause: The documentation relies on an external standards attribution without including a pinned/local excerpt or independently trusted source evi
-
Result: REQUEST_CHANGES
-
Reason: unresolved reviewer or review-agent thread(s) were present before approval.
-
Head SHA:
75c6dbdfde34ac7e729e83f44aa0261e76f475d4 -
Workflow run: 32553294848
-
Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Docs (2 files)"]
S1 --> I1["operator or user guidance"]
I1 --> R1["Review risk: Docs (2 files)"]
R1 --> V1["docs review"]
Evidence --> S2["CI script: safe_pytest_command.py"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script: safe_pytest_command.py"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test: test_safe_pytest_command.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_safe_pytest_command.py"]
R3 --> V3["targeted test run"]
opencode-agent
Bot
added
merge: urgent-review
|
Caution Review failedAn error occurred during the review process. Please try again later. 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. Comment |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthrough
Changespytest 명령 탐색 확장
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🔵 Low · up to The parser currently misses valid multiline YAML commands that use explicit indentation indicators, so some eligible pytest runs may be skipped; this is a bounded discovery correctness issue, and the PR is otherwise mergeable with owner follow-up to support those headers and add regression coverage. Sequence Diagram(s)sequenceDiagram
participant WorkflowFile as GitHub Actions workflow
participant DiscoverCommands as discover_commands
participant RunIterator as _iter_run_command_lines
participant PytestMatcher as _is_pytest_argv
WorkflowFile->>DiscoverCommands: 워크플로 텍스트 전달
DiscoverCommands->>RunIterator: run 명령 후보 추출
RunIterator-->>DiscoverCommands: 물리적 명령줄 반환
DiscoverCommands->>PytestMatcher: argv 안전성 검증
PytestMatcher-->>DiscoverCommands: 검증된 pytest 명령 반환
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 95.65% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 46 functions across 3 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
opencode-agent
Bot
added
priority: critical
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
afadc7670b9e5ba0f5f3d316c48ebfeb92e52d6e. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Strix Security Scan/strix workflow run: cancelled (https://github.com/ContextualWisdomLab/.github/actions/runs/33240316162)
- strix: FAILURE (https://github.com/ContextualWisdomLab/.github/actions/runs/33240516245)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Docs: safe-pytest-command-multiline-run-blocks.md (2 files)"]
S1 --> I1["operator or user guidance"]
I1 --> R1["Review risk: Docs: safe-pytest-command-multiline-run-blocks.md (2 files)"]
R1 --> V1["docs review"]
Evidence --> S2["CI script: safe_pytest_command.py"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script: safe_pytest_command.py"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test: test_safe_pytest_command.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_safe_pytest_command.py"]
R3 --> V3["targeted test run"]
…finding
contextual-orchestrator#950 (cited by this PR as "the upstream half of this
bug, not yet merged") was closed as redundant/superseded. The fix actually
merged as contextual-orchestrator#949 ("fix(discovery): route OpenRouter by
model evidence", 8cd99f139915131ba0239bce12a5d6a5fd85394e); .github#1477
already advances ORCHESTRATOR_PIN_SHA to that commit. Corrects every stale
#950 reference in the PR's own docstrings and docs/product-technical-gap-
baseline.md, and folds in two things only visible from #949's actual diff:
- #949 also added DiscoveredModel.spend_admitted (default True, False for a
priced OpenRouter row when openrouter_paid_inference_available() cannot
confirm usable credit). orchestrator/free never considers priced rows, so
it was never exposed to this, but orchestrator/auto (real, reachable via
CONTEXTUAL_ORCHESTRATOR_POOL=auto, no other code change needed) does
consider priced rows and had no spend_admitted check anywhere in this
repo's pipeline. _routable_discovered_models() now excludes a
spend_admitted=False row the same way it excludes evidence_only=True,
with regression coverage including an end-to-end auto-pool composition.
- A fresh Devin Review red finding on this PR argued the OpenRouter
evidence_only exemption could let private/--require-zdr review content
reach ZDR-forbidden routes. Traced end to end: build_zdr_prioritized_
catalog() independently re-applies is_zdr_model()'s real OpenRouter ZDR-
feed check as its own admission gate whenever require_zdr=True, entirely
independent of evidence_only. Confirmed false alarm with a regression
test proving a non-ZDR-attested OpenRouter row is excluded from a
require_zdr=True catalog even while every discovered OpenRouter row still
carries the evidence_only=True bug signature; documented in the gap
baseline and replied/resolved on the GitHub review thread.
docs/product-technical-gap-baseline.md gets a new 2026-08-31 correction
subsection recording all of the above. Full suite: 2098 passed, 1 skipped,
21 subtests passed; 100% coverage on scripts/ci/; 100% docstring coverage.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
Devin Review (this PR, scripts/ci/contextual_orchestrator_review_launcher.py lines 129-169) flagged that the docstrings describe #1477 (the ORCHESTRATOR_PIN_SHA bump to contextual-orchestrator#949's merge commit) as still pending. Verified: #1477 merged on 2026-08-31 -- before this PR's original evidence_only/spend_admitted fix even landed -- and `git merge-base --is-ancestor 8cd99f139915131ba0239bce12a5d6a5fd85394e 045d17da5e2aea56a97e241ee158ab1628d78660` against contextual-orchestrator confirms the current pin already descends from #949's fix commit. The "not yet pinned" / "once #1477 merges" framing was therefore stale runtime history that would mislead future maintenance into thinking the pin bump was still outstanding. Updates both affected docstrings in the launcher module and the two docstrings in tests/test_contextual_orchestrator_review_runtime_preflight.py that repeated the same stale framing. Docs-only: no behavior change, no assertion changed. docs/product-technical-gap-baseline.md's own #1477 references are left untouched -- they are dated, explicitly time-qualified ("open as of this correction") historical entries in this repo's append-only decision log, not living claims about current state, so rewriting them would falsify the audit trail rather than correct it. Verified: `python -m pytest tests/test_contextual_orchestrator_review_runtime_preflight.py tests/test_contextual_orchestrator_review_sidecar_contract.py -q` -- 95 passed; `interrogate scripts/ci/contextual_orchestrator_review_launcher.py` -- 100%. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
1 participant
Current repair
Safe pytest discovery now recognizes multiline GitHub Actions
run: |/run: >blocks while preserving the bounded runner grammar andshell=Falseexecution. The first execution target must still be pytest; file operands,-c,--, earlier modules, shell control syntax, and unrecognized executables fail closed.Exact identity
811c0321f661b85c9270a85f36fc20fc0da13b42main@55a8b576725451dfe0a21a57d36a2f1a41619b24c9efc974e0b112082895a425e81b124f4fde70d5Main convergence
The predecessor branch had drifted into a tree that deleted protected main's exact-artifact SBOM workflows, verifier, doctoring, and tests and replaced unrelated shared architecture/changelog content. The current non-forced merge commit uses protected main as the tree authority and overlays only this PR's four owned paths. No attestation, release, scheduler, Strix, shared architecture, or unrelated changelog source is deleted or rewritten.
Verification
42 passed.1325 passed, 16 subtests passed.git diff --check origin/main HEAD: passed.The previous
CHANGES_REQUESTEDreview targets the superseded head and is not current-head authority. Fresh exact-head protected checks and a substantive formal verdict remain required before merge.Summary by CodeRabbit
새 기능
run블록에서도 안전한 pytest 명령을 개별적으로 탐지합니다.보안 개선
문서 및 테스트