PCSM-313: opencode/github integration improvements - #227
Draft
chupe wants to merge 9 commits into
Draft
Conversation
chupe
force-pushed
the
PCSM-313-opencode-step-timeout
branch
from
May 6, 2026 11:36
fe16107 to
f308612
Compare
chupe
changed the title
Collaborator
Author
|
/summary |
Collaborator
Author
|
/review |
chupe
changed the title
chupe
force-pushed
the
PCSM-313-opencode-step-timeout
branch
from
May 7, 2026 23:34
aa7cda8 to
bd7f74e
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Go Review Summary
Verdict: Approve
Effort to Review: 1/5
Context
This PR (PCSM-313) is entirely a CI/automation change. It restructures the /summary and /review GitHub Actions workflows (.github/workflows/opencode-pr-summary.yml, .github/workflows/opencode-review.yml) and updates the agent prompt files (.github/opencode/summary-prompt.md, .github/opencode/review-prompt.md). The substantive design points — bumping step timeouts, switching /review away from the anomalyco/opencode/github wrapper to mirror /summary's pre-fetch + opencode run --pure + validate + post pattern, and tightening the secret-scrub regex to concrete provider tokens — are workflow and prompt concerns, not Go concerns.
No Go source files (*.go), no go.mod/go.sum, and no project package code is touched in this diff. There is nothing in scope for a Go review (correctness, error wrapping with errors.Wrap, concurrency, allocations, idiomatic style, exported-API doc comments). Per the review prompt's scope discipline and confidence gate, silence on those categories is the correct signal.
The workflow and prompt changes themselves look internally consistent with the stated design (file-handoff contract preserved on /summary, token isolation moved to step-level env:, narrowed validation regex, OPENCODE_DISABLE_PROJECT_CONFIG and per-pattern bash allow-list considered and ultimately dropped in favor of the symmetric --pure design), but reviewing CI hardening trade-offs is outside the Go-review remit configured for this run.
chupe
force-pushed
the
PCSM-313-opencode-step-timeout
branch
from
May 22, 2026 17:21
48e4065 to
1226a34
Compare
chupe
force-pushed
the
PCSM-313-opencode-step-timeout
branch
2 times, most recently
from
June 1, 2026 13:28
95bc485 to
57e0ac3
Compare
Restore the anomalyco/opencode/github action wrapper at pinned SHA 77fc88c8 (github-v1.2.19) for the /review path and pair it with a defense-in-depth tool-denial layer instead of the file-handoff plus post-job split. This trades the artifact-as-attestation property for the opencode-agent[bot] identity and session-link UX while keeping the load-bearing PCSM-317 controls. Hardening kept on /review: - maintainer slash-command gate via gh api collaborators check - prompt loaded from the trusted default branch, not the PR head - pinned action SHA - no GITHUB_TOKEN reaches the agent's environment Hardening added on /review: - OPENCODE_DISABLE_PROJECT_CONFIG=true so opencode.json/AGENTS.md/skills on the PR head cannot influence the agent - OPENCODE_PERMISSION denying bash, edit, task, webfetch, websearch and question, leaving the agent only able to read repo files and produce review markdown Hardening dropped on /review: - prefetch of PR JSON (the wrapper does its own fetch) - file handoff via REVIEW_BODY_FILE - output validation regex (no exfiltration channel remains for the agent to abuse) - generate/post job split For /summary the same observations apply but PR-body update is a real write so the file-handoff plus validate-then-PATCH pattern stays. Generate and post are collapsed into a single job; token isolation moves from job-level permission boundary to step-level env boundary because only the final 'Update PR body' step receives GH_TOKEN. Validation regex is narrowed to concrete provider tokens (sk-ant-, github_pat_, gh[pousr]_, BEGIN PRIVATE KEY) plus the literal Anthropic key and a 60000-byte size cap; the generic 96+ hex, 120+ base64 and broad VAR=value patterns are dropped because they false-positive on legitimate review content. review-prompt.md is rewritten for the wrapper world: the agent has no shell, no file editor, no web/search/task/question tools, produces the review as its final assistant message, and the wrapper posts that message verbatim. summary-prompt.md keeps the existing file-handoff contract. The workflow_dispatch trigger added in #230 is preserved on both workflows so this work can be smoke-tested via 'gh workflow run' from the feature branch before merging.
Smoke test on PR-227 showed the /review wrapper hung 15 minutes when
OPENCODE_PERMISSION blanket-denied bash. The agent emitted only one
opening message ('I'll review this PR. Let me start by gathering
context on what's changed.') then no progress until the step timeout.
The wrapper checks out the PR head but does not pre-fetch a diff;
without bash the agent cannot discover changed files and no permission
prompt resolves in headless mode.
Replace the blanket bash deny with a per-pattern allow list for read-
only git/ls/find/grep. Everything else stays denied. Bump the action
to v1.14.41 (commit 98e09179...) — the prior pin was at v1.2.19, well
behind upstream.
Threat surface unchanged otherwise: agent has no GitHub token, no
edit, no web fetch, no task spawning, no question. Cannot exfiltrate
secrets, call GitHub, or modify files. Worst case is a rude review
comment; the maintainer slash-command gate still gates entry.
The previous run never tried bash. Agent looked for prefetched PR JSON files (/tmp/**/*pr*, **/pr_*.json), found none, then attempted Read on the workspace directory and hung 15 minutes. The wrapper does not pre-fetch JSON; it checks out the PR head into the workspace. The agent should run `git diff origin/main...HEAD` to see changes. Make this explicit in the prompt and bump fetch-depth to 0 so the merge base is in local history.
The wrapper-based /review hung on the second Anthropic API call after the agent's first tool use, regardless of whether bash was blanket-denied or restricted to a per-command allow list. Two attempts confirmed the wrapper does not compose with our PR-data-handling expectations. Drop the wrapper. /review now mirrors /summary: - Pre-fetch PR JSON via gh in a step that owns GH_TOKEN. - Run `opencode run --pure --dangerously-skip-permissions` with only ANTHROPIC_API_KEY in env, fed the prompt via stdin. - Agent reads PR data from $PR_PAYLOAD_FILE / $PR_COMMITS_FILE / $PR_FILES_FILE and writes the review markdown to $REVIEW_BODY_FILE. - Validate the body for size and known secret patterns before posting. - Post via `gh pr review --comment` from a step that owns GH_TOKEN. Loses the opencode-agent[bot] identity; the review now posts as github-actions[bot]. In exchange the workflow is symmetric with /summary and preserves the full PCSM-317 token-isolation hardening.
Switch /review from a single markdown blob to GitHub-native inline
review comments. The agent now emits a structured JSON review; a
trusted Python validator curates it against the PR file manifest and
posts via the GitHub review API.
- Agent writes {verdict, effort, summary?, comments[]} to
$REVIEW_JSON_FILE. Each comment carries path/side/line and may use
GitHub's suggestion fence for actionable fixes; multi-line ranges
via start_line/start_side are supported.
- .github/scripts/validate_review.py is the trust boundary. It parses
PR file patches, drops comments whose path/line miss the diff hunks
on the requested side, scrubs literal Anthropic keys and concrete
provider tokens (github_pat_, gh[pousr]_, sk-ant-, private-key
marker), caps at 5 comments, and writes the GitHub review payload.
- Lenient curation: invalid comments are dropped with a footnote
rather than failing the whole review. A summary that contains a
secret pattern is still fatal.
- 'Approve with zero comments' stays a first-class output. Effort is
surfaced as an HTML metadata footer (<!-- review-effort: N/5 -->).
- Workflow posts via 'gh api --method POST .../reviews' with the
curated payload; GH_TOKEN remains scoped to the final step.
Two follow-ups after the inline-review smoke test on PR-228. Install retry: the opencode.ai install script fetches release metadata over the network and occasionally fails on shared GitHub runner IPs (seen as 'Failed to fetch version information' on run 25808815681). Wraps the curl|bash in a two-attempt loop with 30s backoff. Same fix applied to both /review and /summary install steps so neither workflow is one-flake away from a failed dispatch. Inline-review span cap: the agent emitted a 103-line multi-line range to associate a manual RUnlock with the matching RLock 'to show the danger zone'. GitHub renders that as a giant highlighted block that buries the finding under scroll. Multi-line ranges should be used only when the critique applies uniformly to the entire span or a suggestion fence replaces it. Belt + suspenders: - review-prompt.md: stronger 'Multi-line ranges (use sparingly)' section. Aim ≤10 lines, validator caps at 15, anchor critiques about distant lines as single-line with prose reference. - validate_review.py: MAX_MULTILINE_SPAN = 15. Comments with valid start_line but span > 15 are silently collapsed to single-line at 'line'. Finding is preserved; only the highlight range narrows.
chupe
force-pushed
the
PCSM-313-opencode-step-timeout
branch
from
July 24, 2026 20:54
57e0ac3 to
9266e06
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PCSM-313
Problem
/summaryand/reviewrun a single OpenCode step pinned totimeout-minutes: 10. On larger PRs the agent reads each changed-file patch from$PR_FILES_FILEthrough individualjqcalls, which spends most of the budget on exploration before it can write the new body to$NEW_BODY_FILE. Recent run 25431222573 on a multi-file PR hit the cap exactly: the agent finished its exploration loop and announced it was about to write the body, then the step timed out before the file was produced.Validate PR body,Upload PR body, and thepost-summaryjob all skipped because the generate step failed./reviewshares the same step shape, so it would fail the same way on similar PRs.Correction: the diff goes well beyond a timeout bump. The author body describes only the
timeout-minuteschange, but the commits and file patches reshape both workflows:/reviewis moved back to theanomalyco/opencode/githubwrapper at pinned SHA77fc88c8(github-v1.2.19), and/summarycollapses itsgenerate-summary+post-summarytwo-job split into a singlesummaryjob that runsopencode run --pure --dangerously-skip-permissionsdirectly undertimeout. The artifact upload/download handoff between jobs is removed on/summary; the file handoff via$NEW_BODY_FILEto a finalgh api PATCHstep stays.Two related gaps the author body does not name: on
/reviewthe wrapper checks out the PR head internally, so attacker-controlledopencode.json/AGENTS.md/ skill files on that head could influence the agent unless project-config loading is disabled; and the previous secret-scrub regex ([A-Fa-f0-9]{96,}and[A-Za-z0-9+/]{120,}={0,2}plus broadVAR=value) false-positives on legitimate review content like commit hashes and base64 fixtures.Solution
Bump
timeout-minutesfrom10to20on theOpenCode PR summarystep in.github/workflows/opencode-pr-summary.ymland on theOpenCode Go reviewstep in.github/workflows/opencode-review.yml. Smallest correct change. No prompt or workflow shape changes; the existing handoff and validate/upload pattern stays the same. If we still see timeouts on outlier PRs we can tighten the prompt to read all patches in one pass instead of per-filejqcalls.Correction: the actual change is broader than a timeout bump and the final timeouts differ from the values in the author text. The PR ends up at
timeout-minutes: 25(with a 30-minute job cap) on the/summaryopencode step andtimeout-minutes: 15(20-minute job cap) on the/reviewopencode step.Key design points grounded in the diff:
/reviewreturns to the wrapper action so comments post asopencode-agent[bot]with a session link, trading the artifact-as-attestation property of the file-handoff path for that identity/UX. Hardening replaces the lost isolation:OPENCODE_DISABLE_PROJECT_CONFIG=trueblocks PR-head config/skill files, andOPENCODE_PERMISSIONdeniesbash,edit,task,webfetch,websearch, andquestion, leaving the agent only able to read repo files and produce its final assistant message, which the wrapper posts verbatim./summarykeeps the file-handoff contract because updating the PR body is a real write. Token isolation moves from a job-level boundary to a step-levelenv:boundary — only the finalUpdate PR bodystep receivesGH_TOKEN; the opencode step has onlyANTHROPIC_API_KEY. Validation regex is narrowed to concrete tokens (sk-ant-,github_pat_,gh[pousr]_,BEGIN PRIVATE KEY) plus the literal Anthropic key and a 60000-byte cap; generic hex/base64/VAR=valuepatterns are dropped to stop false positives.review-prompt.mdis rewritten for the no-tools wrapper world (final assistant message is the review).summary-prompt.mdadds a hard "stop after writing$NEW_BODY_FILE" rule, a "never wait for clarification" rule, and an explicit instruction to handle title/body-vs-diff contradictions with a bolded in-marker correction rather than rewriting author text.workflow_dispatchis preserved on both workflows for smoke-testing from a feature branch.Other changes
---document start and a# yamllint disable rule:line-lengthdirective./summaryjob renamed fromgenerate-summarytosummary;/reviewjob renamed fromgenerate-reviewtoreview.id-token: writepermission removed from/summary(no longer needed without the wrapper) and kept on/reviewfor the wrapper's OIDC App-token exchange.Read PR payloadstep on/reviewis removed; the wrapper does its own PR fetch.Install OpenCodestep on/summaryinstalls the CLI viahttps://opencode.ai/installand prepends$HOME/.opencode/bintoPATH.