fix: preserve opencode review thread jq quoting - #257
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes a quoting bug in the OpenCode review workflow where a jq gsub replacement introduced a literal apostrophe (') inside a Bash single-quoted jq program, breaking the generated shell script. It replaces the apostrophe literal with a jq unicode escape (\u0027) and adds a regression test to prevent the unsafe pattern from reappearing.
Changes:
- Replace
gsub(""; "'")withgsub(""; "\u0027")in jq filters embedded in Bash single-quoted strings. - Add a regression test asserting the unsafe literal apostrophe replacement is absent and the escaped form is present exactly where expected.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/test_opencode_agent_contract.py |
Adds a regression test to ensure jq filters in the workflow never embed a literal ' that would break Bash single-quoted strings. |
.github/workflows/opencode-review.yml |
Updates jq programs to use "\u0027" instead of a literal apostrophe, preserving Bash single-quote integrity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0b67ff6 to
63c7615
Compare
OpenCode Review Overview
Pull request overviewOpenCode reviewed the current-head bounded evidence and found no blocking issues. FindingsNo blocking findings. SummaryApproval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Test: test_opencode_agent_contract.py"]
S1 --> I1["regression suite"]
I1 --> R1["Review risk: Test: test_opencode_agent_contract.py"]
R1 --> V1["targeted test run"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including tests/test_opencode_agent_contract.py.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects tests/test_opencode_agent_contract.py to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source checks are delegated to configured OpenCode web_search/Context7/DeepWiki sources when applicable; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: Playwright visual, DOM locator, ARIA snapshot, console, and responsive evidence were checked when a web UI surface was present; for non-web surfaces, API/CLI/log/docs/workflow interaction evidence was reviewed instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
- Result: APPROVE
- Reason: Added regression test for jq quoting safety
- Head SHA:
63c761530b846081a31a444944883a9cfb903eab - Workflow run: 28492373392
- Workflow attempt: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Test: test_opencode_agent_contract.py"]
S1 --> I1["regression suite"]
I1 --> R1["Review risk: Test: test_opencode_agent_contract.py"]
R1 --> V1["targeted test run"]
…bstring The first cut ended `_is_complete_docx` with `DOCX_MAIN_DOCUMENT_CONTENT_TYPE in declaration` -- a raw-bytes substring test on `[Content_Types].xml`. Reproduced bypass, 578 bytes: a ZIP with exactly the three `DOCX_REQUIRED_PARTS` where the expected MIME appears only inside an XML comment, the real `Override` for `/word/document.xml` declares `application/octet-stream`, `_rels/.rels` is `not xml at all` and `word/document.xml` is `also not xml`, was admitted and so skipped the content scan. Replace the substring test with bounded structural parsing: - `_docx_part_elements` reads each required part bounded (the read *is* the expansion bound; a declared ZIP size is never trusted), decodes strict UTF-8, refuses U+0000 and the `<!DOCTYPE` literal, then parses with the standard library's expat, collecting element names and attributes only. No DTD means no internal entity declaration, so no entity expansion and no billion-laughs; expat resolves no external resource on its own and an undefined entity is a parse error. All three parts must be well-formed. - `_docx_declares_main_document` requires the OPC content-types root and exactly one `Override` pairing `/word/document.xml` with `DOCX_MAIN_DOCUMENT_CONTENT_TYPE`. A `Default Extension` mapping, a comment, an ambiguous duplicate pair, or any other content type does not satisfy it. - `_docx_relates_main_document` requires the OPC relationships root and exactly one internal `officeDocument` relationship whose `Target` is exactly the main document part in either permitted spelling. Exact matching is the path-traversal rejection: nothing is resolved or normalized, so no target outside the package can agree. - `zlib.error` joins the except tuple; a corrupt deflate stream returned False by way of a traceback out of the gate before. `defusedxml` is deliberately not imported. The required workflow runs this module with the runner's stock `python3` and has no `pip install` or `setup-python` step at all (`.github/workflows/opencode-review.yml`, `required-workflow-bootstrap`, lines 34-293); `defusedxml` belongs to the Noema review image (`opencode-review-dispatch.yml:810-817`), not to this gate. A module-level import of it would `ImportError` in every consumer repository's required check, and depending on an unpinned runner package inside a `pull_request_target` trust boundary would breach this repo's hash-pinning discipline. The guarantee `forbid_dtd=True` provides is reconstructed above instead, with no library added either way. The existing prefix, runtime-path and HWPX boundaries are untouched, and `_is_complete_hwpx` is unchanged. Bandit is clean at the central medium/medium gate (`xml.parsers.expat` is B407, LOW severity, and no blacklisted call is used, so no suppression is introduced). Tests: 66 cases, adding the reported counterexample, `Default`-mapping MIME, missing/duplicate/mismatched `Override`, non-well-formed `_rels/.rels` and `word/document.xml` separately, a relationship target pointing elsewhere or outside the package, `TargetMode="External"`, per-part DTD/NUL/UTF-16/undefined-entity/non-UTF-8 refusal, per-part oversize, and a corrupt deflate stream -- all False -- plus positives for both target spellings, explicit `TargetMode="Internal"`, and the ISO 29500 Strict main-document namespace. `test_real_artifact_bytes_are_admitted` binds admission to the actual #257 bytes when `PINGORA_REAL_DOCX_PATH` names a local copy, so the positive and the negatives are proved in one invocation without committing another repository's artifact here. Refs: ContextualWisdomLab/late-life-anxiety-reanalysis#257 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvosNg4GVUjaV5UfrimrsX
Summary
\u0027inside jq filters embedded in Bash single-quoted stringsWhy
newsdom-apiPR #261 exposed a central OpenCode workflow failure before model review:syntax error near unexpected token ')'. The generated shell script closed the jq single-quoted program atgsub(""; "'")`.Tests
bash -nonPrepare bounded OpenCode review evidenceandApprove PR if OpenCode review passedrun blocks..\newsdom-api\.venv\Scripts\python.exe -m pytest tests/test_opencode_agent_contract.py -q..\newsdom-api\.venv\Scripts\python.exe -m pytest -qgit diff --check