Skip to content

feat: add Cursor Cloud Agent review harness - #6

Merged
oakimov merged 5 commits into
mainfrom
feat/cloud-agent-review-harness
Nov 9, 2026
Merged

oakimov merged 5 commits into
mainfrom
feat/cloud-agent-review-harness

Conversation

@oakimov

@oakimov oakimov commented Nov 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adopt the cursor-opencode-provider Cloud Agent + caeliq-reviewer review harness from provider main after merged PR 175 (including --token-env and the publisher_env read-path fix).
  • OCP adaptations: no --live / OpenCode dual install; isolated checks run install, build, typecheck, unit tests, and architecture; Cloud Agent bootstrap builds the workspace only.
  • Add review skill, .cursor/rules mirroring OCP Greptile (plus resource-lifecycle), docs/pr-review.md, eval suite, and publisher/script tests. Greptile and caeliq-reviewer findings share CONTRIBUTING weight.

Test plan

  • bun test test/review-publish.test.ts test/review-eval.test.ts test/review-scripts.test.ts (106 pass, 5 skip, 0 fail)
  • Manual: launch a Cloud Agent with the docs/pr-review.md prompt against a sample PR; confirm isolated-check and preview/show without posting

Port the provider caeliq-reviewer flow, including token-env publication from
provider PR 175, with OCP-adapted isolated checks, no live OpenCode path, and
full eval parity.
Comment thread test/review-scripts.test.ts Dismissed
Enable unprivileged user namespaces on ubuntu-latest so isolate-run tests
can create a boundary, and assert indexed fixture elements for
noUncheckedIndexedAccess.
CodeQL flagged spawnSync cwd as shell injection; the helper never sets
shell:true. Add the shell-looking path regression used to dismiss that alert.
Keep both Unreleased changelog sections (review harness Added + ocp-dev DSH Fixed).
Adversarial review of the ported harness found:

- isolated-check could never pass: workspace build/typecheck scripts call
  bunx, which the boundary did not mount and the Cloud Agent install did
  not link. Mount and link bunx; drop the dead check_tool_path helper.
- publish.yml ran ./test without the user-namespace step, so a v* tag would
  fail on the isolation tests. Add the probe and a test that keeps the
  ci.yml, publish.yml and ci-enable-userns.sh copies identical and ahead of
  every test step.
- The eval baseline was mechanically renamed and no longer matched its
  SOURCE_SHA; restore it byte-for-byte. Baseline and calibration pins are
  cursor-opencode-provider commits, so fetch calibration from that repo.
- provider-boundary.mdc was narrower than its unscoped Greptile rule; add a
  parity test for every .cursor/rules mirror.
- Document the CURSOR_REVIEW_TRUST_SHA pin, the isolation mechanism, and
  the macOS limits of a nested run.
- The shell-metacharacter regression now runs the real review scripts from
  a shell-looking checkout path instead of a stand-in probe.
@oakimov
oakimov merged commit 2e99dbc into main Nov 9, 2026
5 checks passed

@caeliq-reviewer caeliq-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 actionable finding.

Checks:

  • diff: passed. Read the diff from ddecf75 to a8177a2, the pull request body, and the changed scripts, tests, and docs.
  • adversarial: passed. Candidate validation, the omission pass, and the failed-work retry check all finished. One candidate stayed actionable.
  • isolated-check: passed (exit 0). scripts/isolated-check.sh 6 with CURSOR_REVIEW_TRUST_SHA pinned to a8177a2 exited 0. INSTALL, BUILD, TYPECHECK, and UNIT_TESTS passed (938 pass, 0 fail) and ARCHITECTURE passed (11 pass, 0 fail). An earlier invocation exited 1 at install before bun was on PATH and did not run the suite.

Limitations:

  • Interactive stock-host acceptance was not run. This pull request does not change mode switches, approval dialogs, plan review, or execution handoffs, and the review docs define no live check.
  • The macOS sandbox-exec path was not executed.
  • SIGINT, SIGHUP, and SIGQUIT to the shell pid were not signaled separately; they share the unhandled shell path observed for SIGTERM.

Runtime: linux.
Reviewed #6 at a8177a2.

Comment thread scripts/isolated-check.sh Resolved
oakimov added a commit that referenced this pull request Nov 9, 2026
…llation

A TERM, INT, HUP, or QUIT sent only to isolated-check.sh or isolate-run.sh
ran the script's EXIT trap and removed the archive or stage while the
sandboxed command kept running, reparented and with network access.
Neither script signalled its child, and the namespace init forwards only
signals it receives itself. A process-group signal reached the boundary
directly, which is why the existing termination test passed.

isolate-run.sh now runs the boundary as its own session and waits for it.
Cancellation signals are forwarded to that session (unshare ignores TERM
while it waits; the namespace PID 1 shares its process group), and the
session is killed after ISOLATE_TERM_GRACE seconds (default 10). Leftover
session members are killed before staging files are removed. INT and QUIT,
ignored in a non-interactive background job, are restored for the command.

isolated-check.sh runs isolate-run.sh in the background, passes a
cancellation on as TERM, waits for the boundary to stop, then exits with
128 plus the signal number so its EXIT trap removes the archive last.

New tests signal only the script pid. On Linux (namespaces) and macOS
(sandbox-exec) they fail on the previous scripts, leaving the command
running, and pass with this change. SIGKILL still cannot be forwarded.

Reported by caeliq-reviewer on #6 (resource-lifecycle).
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