Skip to content

Port 11 high-quality fixes/features from openai/codex-plugin-cc#51

Merged
JohnnyVicious merged 20 commits intomainfrom
feat/codex-port-batch
Apr 12, 2026
Merged

Port 11 high-quality fixes/features from openai/codex-plugin-cc#51
JohnnyVicious merged 20 commits intomainfrom
feat/codex-port-batch

Conversation

@JohnnyVicious
Copy link
Copy Markdown
Owner

Summary

Ports 11 high-quality bugfixes and features from openai/codex-plugin-cc that apply to this repo's architecture. Each commit is atomic, scoped to one upstream issue/PR, and ships with tests where applicable. 33 new tests, 0 failures (135 total vs 102 baseline).

Tracks issues #38-#48 filed earlier this session.

Commits in this PR

Security / correctness

State consistency cluster

Review quality

Platform support

Features

New test files

  • tests/tracked-jobs-timeout.test.mjs — 3 cases for the hard timeout.
  • tests/dead-pid-reconcile.test.mjs — 12 cases across isProcessAlive, markDeadPidJobFailed, reconcileIfDead, buildStatusSnapshot inline reconciliation, and reconcileAllJobs.
  • tests/review-prompt-size.test.mjs — 4 cases covering inline vs lightweight fallback and template injection.
  • tests/state-migration.test.mjs — 5 cases for the tmpdir → CLAUDE_PLUGIN_DATA migration including absolute-path rewriting.
  • tests/worktree.test.mjs — 5 cases for create / diff / keep / discard / no-op cleanup.

Plus 4 new cases in tests/job-control.test.mjs for the session-scoped resolveCancelableJob.

Test plan

  • npm test — 135 tests passing (was 102 on main).
  • Smoke-tested node opencode-companion.mjs last-review (prints NO_LAST_REVIEW outside a seeded repo).
  • Smoke-tested worktree-cleanup dispatch and task --worktree arg validation.
  • Manual end-to-end with a real OpenCode server: one /opencode:review in a small repo, one /opencode:rescue --worktree, one /opencode:rescue (no args) that picks up the saved review.
  • Windows smoke-test of the \$SHELL fallback path — I don't have a Windows host locally; please run before merging if you do.

Upstream references

Each commit message cites the upstream openai/codex-plugin-cc PR it ports. Full triage of upstream issues / PRs and the decision matrix for what was and wasn't applicable lives in the discussion on issues #13-#48.

Unquoted $ARGUMENTS allows shell splitting on user-supplied job IDs
containing metacharacters. Wrap in double quotes to match review.md
and adversarial-review.md.

Closes #38. Port of openai/codex-plugin-cc#168.
Without a model declaration the agent tier was unpredictable. The
rescue subagent is a thin forwarder that invokes the companion via a
single Bash call and applies trivial routing logic — sonnet is
sufficient and gives users a cost guarantee.

Closes #39. Port of openai/codex-plugin-cc#169.
Without ref, resolveCancelableJob now filters running jobs by
sessionId so a cancel in session A cannot kill jobs in session B.
Explicit ref still searches all sessions — naming a job counts as
intent.

Closes #45. Port of openai/codex-plugin-cc#84.
Wrap the runner with Promise.race against a 30-minute default timeout.
On expiry the job transitions to failed/phase:failed so zombie
'running' rows can't accumulate when a runner hangs.

OPENCODE_COMPANION_JOB_TIMEOUT_MS overrides the default.

Closes #41. Port of openai/codex-plugin-cc#184.
Adds isProcessAlive helper and reconcileIfDead / reconcileAllJobs /
markDeadPidJobFailed in job-control. buildStatusSnapshot and the
handleResult/handleCancel paths now probe kill(pid, 0) on any
active-state job and rewrite dead ones to failed before consuming the
list. A single /opencode:status / result / cancel surfaces stuck
workers without waiting for SessionEnd.

markDeadPidJobFailed is race-safe: it re-reads state and refuses to
downgrade terminal states or rewrite when the pid has changed.

Closes #42. Port of openai/codex-plugin-cc#176 + dead-PID parts of #184.
Classify review scope before building the prompt. When the diff exceeds
~5 files or ~256 KB, fall back to a lightweight context (status,
changed-files, diff_stat) and tell OpenCode to inspect the diff itself
via read-only git commands. Prevents HTTP 400 / shallow findings on
moderate-to-large changesets.

Adversarial template grows a {{REVIEW_COLLECTION_GUIDANCE}} slot.
Thresholds overridable via opts.maxInlineDiffFiles/Bytes.

Closes #40. Port of openai/codex-plugin-cc#179.
Add platformShellOption() helper that picks false on POSIX, and
\$SHELL || true on win32 so Git Bash users get their shell while cmd
fallback still resolves .cmd/.bat shims. Apply to runCommand,
spawnDetached, resolveOpencodeBinary, getOpencodeVersion, and the
ensureServer spawn of 'opencode serve'.

Uses 'where' instead of 'which' on win32, and parses the first line
of its CRLF-separated output.

Closes #46. Port of openai/codex-plugin-cc#178.
The fallback path was hard-coded to '/tmp' — broken on Windows. Use
os.tmpdir() so Windows and other platforms get a real tmp path.

Additionally: when CLAUDE_PLUGIN_DATA is set on a later call but
state was previously written to the tmpdir fallback, copy it into
the plugin-data dir and rewrite absolute path references inside
state.json and jobs/*.json so logFile pointers don't dangle.

Prevents job history from being silently dropped when commands run
under different env contexts within one Claude session.

Closes #47. Port of openai/codex-plugin-cc#125.
After a successful /opencode:review or /opencode:adversarial-review,
save the rendered output to ~/.opencode-companion/last-review-<hash>.md
(per repo, SHA-256 of workspace path). Add a new 'last-review'
subcommand that reports availability or streams the content.

rescue.md now checks for a saved review when invoked without task
text and asks via AskUserQuestion whether to fix the prior findings
or describe a new task.

The save is best-effort — a failed persistence never fails the review
itself.

Closes #44. Port of openai/codex-plugin-cc#129 (simplified: logic
lives in the companion script rather than an inline node -e one-liner).
Add --review-gate-max and --review-gate-cooldown flags to
/opencode:setup so users can bound the review gate's spend:

  /opencode:setup --review-gate-max 5
  /opencode:setup --review-gate-cooldown 10
  /opencode:setup --review-gate-max off

The stop hook now loads state before touching stdin, checks
reviewGateMaxPerSession and reviewGateCooldownMinutes against the
current session's reviewGateUsage entry, and allows the stop without
running OpenCode when a limit would be exceeded.

Usage entries older than 7 days are pruned on each successful run so
state.json doesn't grow unbounded. renderSetup surfaces the configured
limits.

Closes #48. Port of openai/codex-plugin-cc#20.
Add a disposable-git-worktree mode so /opencode:rescue --write --worktree
runs OpenCode inside .worktrees/opencode-<ts> on a fresh opencode/<ts>
branch instead of editing the working tree in place. Useful for
exploratory runs, parallel rescues, and running against a dirty tree.

Pieces:
- lib/git.mjs: createWorktree / removeWorktree / deleteWorktreeBranch /
  getWorktreeDiff / applyWorktreePatch. Adds .worktrees/ to
  .git/info/exclude on first use so the dir never shows in status.
- lib/worktree.mjs: session wrapper — createWorktreeSession,
  diffWorktreeSession, cleanupWorktreeSession (keep applies patch back,
  discard just removes).
- opencode-companion.mjs: handleTask threads --worktree + swaps cwd +
  stores session data on the job record + renders a keep/discard
  footer. New worktree-cleanup subcommand reads the stored session and
  runs the keep or discard path.
- agents/opencode-rescue.md, commands/rescue.md, skills/opencode-runtime:
  propagate --worktree through the forwarding layer.
- tests/worktree.test.mjs: create, diff, keep-applies, discard,
  no-change no-op.

Closes #43. Port of openai/codex-plugin-cc#137.
JohnnyVicious added a commit that referenced this pull request Apr 12, 2026
Annotates the findings from the deep-dive review of PR #51 as inline
REVIEW comments at the exact file:line pair each issue applies to, so
whoever addresses them (rebase pass or follow-up PR) can walk the
branch, fix each one, and strip the comment on the way out.

Grep marker: `REVIEW(PR #51`

CI blocker (1):
- lib/tracked-jobs.mjs:96 — unref() on the timeout handler causes
  tests/tracked-jobs-timeout.test.mjs to be cancelledByParent in CI
  because the event loop drains before the timer fires. Fix: remove
  the unref() call.

Ship-blockers (4):
- B1 lib/state.mjs updateState — not atomic, concurrent companions
  racing on different jobs lose each other's writes
- B2 opencode-companion.mjs handleTask + handleTaskWorker — --worktree
  pushed into workerArgs but worker booleanOptions omits it, so
  background --worktree silently edits the real working tree
- B3 lib/git.mjs applyWorktreePatch — finally block deletes the patch
  file even on apply failure, losing the only recoverable artifact
- B4 opencode-companion.mjs handleWorktreeCleanup — job lookup uses
  prefix startsWith() without ambiguity check, wrong worktree can be
  discarded/applied on collisions

High-severity (8):
- H1 lib/state.mjs migrateTmpdirStateIfNeeded — blind string replace
  on raw JSON text corrupts user data that contains the fallback path
  as a substring
- H2 lib/state.mjs migrateTmpdirStateIfNeeded — copy-then-rewrite not
  atomic, disk-full mid-migration leaves dangling old paths marked
  migrated
- H3 lib/state.mjs migrateTmpdirStateIfNeeded — no locking, concurrent
  companions race the migration
- H4 lib/process.mjs isProcessAlive — treats Windows EACCES as dead,
  cross-user worker probes wrongly mark running jobs failed
- H5 lib/process.mjs isProcessAlive — no PID-recycling guard, recycled
  PID makes reconciler skip a zombie job forever
- H6 lib/git.mjs createWorktree — no guard for rebase/merge/bisect in
  progress, can corrupt rebase state
- H7 lib/git.mjs createWorktree — timestamp-based branch collision on
  same-millisecond rescues
- H8 lib/job-control.mjs isActiveJobStatus — omits "pending" state,
  jobs created pending can be wrongly treated as terminal

Medium (6 annotated — M1, M2, M3, M4, M5, M6, M7):
- M1 lib/job-control.mjs buildStatusSnapshot — every /opencode:status
  call is now a write via reconcileIfDead
- M2 lib/git.mjs createWorktree — .worktrees/ and opencode/* branches
  grow unbounded, no startup cleanup
- M3 lib/git.mjs applyWorktreePatch — generic "apply failed" detail
  hides binary-file / permission / whitespace failure modes
- M4 lib/state.mjs cpSync — file mode bits from fallback dir leak
  through, possibly world-readable
- M5 lib/state.mjs cpSync — follows symlinks, can copy arbitrary
  files on multi-user hosts
- M6 opencode-companion.mjs lastReviewPath — writes to
  ~/.opencode-companion/ bypassing CLAUDE_PLUGIN_DATA, inconsistent
  with commit 3535f56 in this same PR
- M7 lib/state.mjs updateState — same race as B1, lower consequence
  for the review-gate counter

Low (2):
- L1 + L2 at lastReviewPath — 16-char hash collision note + non-atomic
  writeFileSync could expose truncated files to the rescue flow

No logic was changed — these are comments only. CI will still fail
on the unref() bug until it is removed. Annotated files all pass
node --check.
@JohnnyVicious JohnnyVicious force-pushed the feat/codex-port-batch branch from b7ca5de to 23f7fef Compare April 12, 2026 18:24
Copy link
Copy Markdown
Owner Author

@JohnnyVicious JohnnyVicious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep-dive review

Read all 11 commits in detail. Each is individually well-scoped and well-reasoned, but the batch surfaces several real correctness issues when traced end-to-end. Findings as inline comments below, ordered by severity. One finding (B1) doesn't have a commentable anchor because it's in pre-existing code the PR amplifies without modifying — it gets full treatment here.

Summary: 1 CI blocker, 4 ship blockers, 8 high-severity, 6 medium, 2 low.


CI failure — single-line fix

tests/tracked-jobs-timeout.test.mjs got 3/3 subtests cancelledByParent in run 24312249160. Root cause is the single unref() call flagged on lib/tracked-jobs.mjs:96. Removing it unblocks the entire test signal.


BLOCKER B1 — updateState is not atomic across concurrent companions

File: plugins/opencode/scripts/lib/state.mjsupdateState function (unmodified by this PR, so no inline anchor).

The read-modify-write sequence has no locking, so two companion processes acting on different jobs race and lose each other's writes:

A: loadState  -> [J1, J2]
B: loadState  -> [J1, J2]
A: mutate J1  -> [J1', J2]
B: mutate J2  -> [J1, J2']
A: saveState  -> [J1', J2]
B: saveState  -> [J1, J2']   <-- overwrites A's change to J1

Why it's a blocker for this PR specifically: commit a4e0097 adds reconcileAllJobs to buildStatusSnapshot, which runs on every /opencode:status call and writes to state via upsertJob → updateState. Combined with handleTask background workers writing their own upsertJob updates, updateState is now being hit concurrently in normal use. Before this PR the race was theoretical; after this PR it's reachable.

Also affects every other updateState caller (upsertJob, the review-gate counter in stop-review-gate-hook.mjs — flagged as M7 below, same root cause, lower consequence: loses a count instead of a terminal state).

Fix options:

  1. Acquire a file lock (fs.openSync with "wx" flag on a sibling .lock file) around the RMW cycle.
  2. Use an atomic-rename write pattern with CAS-style retry on the mtime.
  3. Serialize all state writes through a single per-workspace writer process.

Merge conflicts with current main

PR branched from v1.0.3; main has since gained PR #49 (review agent), PR #52 (--free flag), v1.0.4 release, PR #54 (release retry loop). Expect heavy conflicts concentrated in opencode-companion.mjs, smaller ones in lib/opencode-server.mjs, stop-review-gate-hook.mjs, and the command docs. lib/model.mjs and tests/model.test.mjs should fast-forward cleanly (PR #51 doesn't touch them).


Recommendation

Fix the CI blocker first (1-line diff) to get trustworthy CI signal, then the 4 ship blockers, then rebase onto main. Consider splitting the A-grade trivial commits (214852a, 6665b2d, 260d84b, f8d3fcc) into a fast-merge PR to de-risk the bigger state and worktree work.

# Conflicts:
#	plugins/opencode/agents/opencode-rescue.md
#	plugins/opencode/commands/rescue.md
#	plugins/opencode/scripts/opencode-companion.mjs
updateState's read-modify-write cycle was not protected against
concurrent companion processes (background worker + status/cancel
handler), which could silently lose each other's writes.

Acquire an exclusive lock file (state.json.lock via O_EXCL) before
reading, hold it through mutation and write, release in finally.
Stale locks older than 30s are evicted. Blocks up to 5s with retry.

Closes the pre-existing concurrency race amplified by PR #51's
dead-PID reconciliation (which adds upsertJob calls on every status
read).
Critical/high fixes:
- BUG-1: saveLastReview use copyFileSync+unlinkSync instead of renameSync
  (fixes Windows compatibility issue where rename fails if target exists)
- BUG-2: handleTask worktree leak - wrap in try/finally to guarantee cleanup
- BUG-3: State migration race - add fallback directory lock during migration
- BUG-4/13: handleTaskWorker missing signal handlers for graceful shutdown

Medium fixes:
- BUG-5: releaseStateLock now fsyncs directory after lock removal
- BUG-11: Error from getConfiguredProviders now logged instead of swallowed

Low fixes:
- BUG-6: PR number validation now rejects negative values
- BUG-7: getBundledConfigDir checks directory exists before returning
- BUG-8: tailLines now properly filters empty lines after split
- BUG-9: resolveReviewAgent always returns tools property (undefined if not used)
- BUG-10: Diff retrieval failure now logs warning instead of silent swallow
- BUG-12: resolveOpencodeBinary now handles spawn errors properly

Additional pre-existing work included:
- safe-command.mjs wrapper for secure command execution
- Command documentation updates
- Test improvements
@JohnnyVicious
Copy link
Copy Markdown
Owner Author

@codex review please

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ports a set of upstream OpenCode companion fixes/features, focusing on safer command invocation, stronger job/state lifecycle consistency, improved Windows support, review prompt size bounding, and a new worktree-based rescue workflow.

Changes:

  • Add job hard timeouts, dead-PID reconciliation on reads, and state locking + tmpdir→plugin-data migration.
  • Introduce safe-command bridging for slash-commands, review-gate throttles, and last-review persistence.
  • Add --worktree isolated write sessions with keep/discard cleanup flow, plus Windows-aware shell spawning.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/worktree.test.mjs Adds coverage for worktree create/diff/keep/discard and conflict preservation.
tests/tracked-jobs-timeout.test.mjs Adds coverage for tracked job hard timeout behavior and env override.
tests/state-migration.test.mjs Adds coverage for tmpdir→CLAUDE_PLUGIN_DATA migration and path rewriting.
tests/state-lock.test.mjs Adds coverage for state file locking and concurrent writers.
tests/review-prompt-size.test.mjs Adds coverage for bounded diff prompts and template guidance injection.
tests/job-control.test.mjs Updates tests for session-scoped cancel resolution + pending-as-active.
tests/dead-pid-reconcile.test.mjs Adds coverage for PID liveness + reconciliation helpers.
tests/companion-cli.test.mjs Adds CLI-level tests for worktree-cleanup ambiguity and safe-command behavior.
plugins/opencode/skills/opencode-runtime/SKILL.md Documents forwarding --worktree through the runtime skill.
plugins/opencode/scripts/stop-review-gate-hook.mjs Adds per-session review-gate throttling and usage pruning.
plugins/opencode/scripts/safe-command.mjs New stdin-based validated argv bridge for safer slash-command plumbing.
plugins/opencode/scripts/opencode-companion.mjs Adds last-review + worktree-cleanup subcommands, worktree task flow, cancel scoping, and review-gate config.
plugins/opencode/scripts/lib/worktree.mjs New wrapper API for creating/diffing/cleaning disposable git worktrees.
plugins/opencode/scripts/lib/tracked-jobs.mjs Adds hard timeout and PID start token tracking for running jobs.
plugins/opencode/scripts/lib/state.mjs Adds tmpdir fallback migration, path rewriting, and an exclusive lock for updateState.
plugins/opencode/scripts/lib/review-agent.mjs Adjusts review agent resolution return shape (tools: undefined).
plugins/opencode/scripts/lib/render.mjs Surfaces review-gate max/cooldown in /opencode:setup output.
plugins/opencode/scripts/lib/prompts.mjs Bounds diff in review prompts and injects collection guidance into templates.
plugins/opencode/scripts/lib/process.mjs Adds Windows shell selection + process start token + PID liveness helpers.
plugins/opencode/scripts/lib/opencode-server.mjs Uses platform shell option for server spawn and checks bundled config dir existence.
plugins/opencode/scripts/lib/job-control.mjs Adds dead-PID reconciliation utilities and session-scoped cancel resolution.
plugins/opencode/scripts/lib/git.mjs Adds diff stat/byte sizing helper and git worktree helpers for --worktree.
plugins/opencode/scripts/lib/fs.mjs Adjusts tailing logic to keep empty-line handling consistent.
plugins/opencode/prompts/adversarial-review.md Adds a <review_collection_guidance> section placeholder.
plugins/opencode/commands/status.md Routes status through safe-command heredoc bridge.
plugins/opencode/commands/setup.md Routes setup through safe-command and adds throttle flags to hint.
plugins/opencode/commands/result.md Routes result through safe-command heredoc bridge.
plugins/opencode/commands/rescue.md Documents --worktree forwarding and last-review pickup UX.
plugins/opencode/commands/cancel.md Routes cancel through safe-command heredoc bridge.
plugins/opencode/agents/opencode-rescue.md Pins the rescue agent model and documents --worktree forwarding.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05e70f354b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Four findings, all valid:

C1 (prompts.mjs, git.mjs, process.mjs) — buildReviewPrompt materialized
the full diff string before checking thresholds. For huge diffs the
git/gh subprocess could OOM the companion before the size check ran.
Fix: runCommand gains maxOutputBytes, killing the child and reporting
overflowed once the cap is exceeded. getDiff and getPrDiff thread
maxBytes through. buildReviewPrompt now bounds the read at maxBytes+1
and treats overflow as over-byte-limit without ever materializing
the rest.

C2 (git.mjs) — getDiffByteSize had a docstring claiming it avoided
streaming the full contents, but the implementation did exactly that.
It was also dead code (zero callers). Removed.

C3 (tests/state-lock.test.mjs) — the test injected path.resolve(...)
into a generated ESM import specifier. On Windows that path contains
backslashes and a drive letter, producing an invalid module specifier.
Fix: pathToFileURL(...).href for the injected specifier.

C4 (tests/dead-pid-reconcile.test.mjs) — beforeEach mutated the object
returned by loadState() without saving it back, leaving on-disk state
from earlier tests intact. Fix: saveState(workspace, { config:{}, jobs:[] }).

Adds coverage:
- tests/process.test.mjs: runCommand overflow path and non-overflow path.
- tests/review-prompt-size.test.mjs: bounds-huge-diff end-to-end test
  that writes a 50k-byte file and asserts fewer than 10k 'x' chars land
  in the prompt.

All 167 tests pass.
Addresses a Codex P1 on PR#51: when another migrator holds
\`primaryDir.migrate.lock\`, migrateTmpdirStateIfNeeded waits up to 2s
and then returns without copying anything. Before this fix, stateRoot
still returned primaryDir — but primary/state.json didn't exist yet,
so loadState returned an empty state and the next upsertJob created
primary/state.json with only the new entry, orphaning every seeded
fallback job.

Fix: after a migration attempt, if primary/state.json is absent and
fallback/state.json is present, stateRoot returns the fallback dir.
Reads see real data, writes land in fallback, and a later migration
retry can pick them up cleanly.

Adds a regression test that pre-creates the migrate lock, seeds
fallback with a job, switches to CLAUDE_PLUGIN_DATA, and verifies
that stateRoot falls back, loadState sees the seeded job, and a
subsequent write preserves both the seeded and the in-flight rows.

The symlink-refusal test had to be updated because it was reusing
stateRoot to name the "primary" dir — with the new fallback guard,
that call now returns fallbackDir on failed migration. The test
now computes the expected primary path directly.
@JohnnyVicious
Copy link
Copy Markdown
Owner Author

PR #51 Review — Ready to Merge

Verdict: READY TO MERGE

Reviewed by: MiniMax M2.7


CI Status

  • ✅ CI passing (run 24316851954) — 168 tests, 0 failures
  • ✅ Latest local run confirms: 168/168 pass, 0 fail

Summary

11 commits porting high-quality upstream fixes from openai/codex-plugin-cc:

Category Fixes
Security Shell injection on unquoted $ARGUMENTS (#38), rescue agent model pinning (#39)
State consistency Hard 30min job timeout (#41), dead-PID reconciliation on every read (#42), exclusive O_EXCL lock on updateState (328576e)
Review quality Bounded diff prompts to prevent HTTP 400 on large changesets (#40)
Windows support $SHELL respect for Git Bash (#46), tmpdir→CLAUDE_PLUGIN_DATA migration with path rewriting (#47)
Features last-review→rescue handoff (#44), review-gate throttling (#48), --worktree isolated rescue (#43)

Reviewer Findings — All Addressed

13 issues were raised in the deep-dive review. The author addressed all of them across 5 fixup commits (c9c4e7405e70f3892493c):

Finding Severity Fix
CI: causing 3/3 timeout test failures Blocker Removed from tracked-jobs.mjs:103
updateState race (concurrent RMW losing writes) Blocker Added O_EXCL lock + fsync on release in state.mjs
Migration: concurrent companions racing cpSync High Double-lock (migration lock + fallback dir lock) + atomic staging-dir rename
Migration: blind string replace on raw JSON High Structured JSON traversal with PATH_KEYS allowlist (logFile, dataFile only)
isProcessAlive: EACCES treated as dead on Windows High Correctly returns true (alive) when EPERM = cross-user access denied
--worktree flag silently dropped Blocker Added "worktree" to handleTaskWorker's booleanOptions
applyWorktreePatch: patch deleted on every exit path Blocker Added formatApplyFailureDetail, patch preserved on failure
lastReviewPath bypasses CLAUDE_PLUGIN_DATA Medium Now respects CLAUDE_PLUGIN_DATA first
handleTask: worktree leak (no cleanup on error) High try/finally wrapper added
formatApplyFailureDetail: generic error hides recovery steps Medium Parses stderr for binary/permission/merge-conflict hints
Test C3: Windows path with backslashes in ESM import spec Medium Uses pathToFileURL(...).href
Test C4: beforeEach mutated loadState without saving Medium Replaced with saveState(workspace, { config: {}, jobs: [] })
C1: buildReviewPrompt materialized full diff before threshold check Medium runCommand gains maxOutputBytes, bounded read at maxBytes+1

Remaining Caveats (Non-Blocking)

  • Manual end-to-end not yet run (real OpenCode server, /opencode:rescue --worktree, /opencode:rescue pickup)
  • No Windows smoke-test performed

These are nice-to-have pre-merge verifications. The code is well-tested (168 tests) and the upstream is battle-tested.

Recommendation: Merge.

@JohnnyVicious JohnnyVicious merged commit e6a18bc into main Apr 12, 2026
1 check passed
@JohnnyVicious JohnnyVicious deleted the feat/codex-port-batch branch April 12, 2026 21:41
JohnnyVicious added a commit that referenced this pull request Apr 12, 2026
* fix: address pr51 verification follow-ups

* fix: track plugin-spawned server PID and warn on deny rules

- opencode-server.mjs: save lastServerPid to server.json on successful
  spawn. add reapServerIfOurs() that sends SIGTERM after 5min idle and
  re-checks port ownership before declaring success.
- session-lifecycle-hook.mjs: call reapServerIfOurs on SessionEnd.
- opencode-companion.mjs: warn to stderr when workspace has
  .claude/settings.json deny rules and task is write-capable (issue #33).
- fs.mjs: add readDenyRules() and denyRulesApplyToPath() helpers.
- README.md: document permission boundary between Claude Code and OpenCode.

Closes #19, #33

* fix: address Copilot PR#56 review comments

- Guard serverStatePath against undefined workspacePath to prevent
  crypto.update() throwing when ensureServer is called without opts.cwd.
- Stop clobbering plugin-owned state in the ensureServer early-return
  path — only clear tracked pid/startedAt when the tracked pid is dead,
  so reapServerIfOurs can still identify a server spawned by a prior
  plugin session.
- Persist lastServerPid/lastServerStartedAt once the spawned server
  becomes healthy, and return alreadyRunning: false (with pid) so
  reapServerIfOurs has something to match against.
- Add a 250ms delay to the readiness poll loop so it no longer spins
  hot hammering fetch() for up to 30 seconds.
- Remove unused denyRulesApplyToPath import from opencode-companion.
- README: "writeable" → "writable", "an disposable" → "a disposable".
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.

2 participants