[dancingteeth]

Problem

On 0.8.2 (npm), a clean install with only @dancingteeth/agent-looper crashes on every bin — even agent-loop --version — because agentRunner / oneShotAgentRun statically imported cursorAgent.js, which statically imported @cursor/sdk (optional peer).

Repro (published 0.8.2):

mkdir /tmp/t && cd /tmp/t && npm init -y && npm i -D @dancingteeth/agent-looper@0.8.2 && npx agent-loop --version
# Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@cursor/sdk' imported from .../dist/agents/cursorAgent.js

Root cause

Optional runtime SDKs are optional peerDependencies, but several agent modules still used static import at module evaluation time. CLI entrypoints load agentLoop → agentRunner before --version / --help handling.

Fix (targets next release; package version stays 0.8.2 until release cut)

Changelog entries live under Unreleased (with #45 harness work already on main).

• importOptionalPeer helper — dynamic import(); missing peers match Node’s quoted Cannot find package '…' against the base package (subpath exports like @earendil-works/pi-ai/compat → @earendil-works/pi-ai) so transitive misses inside an installed SDK are not blamed on the peer.
• Cursor / Cline / other runtimes — lazy SDK loads; Cline missing-SDK hints use the selected runtime (cline-pass vs cline).
• agent-check — actionable install line via formatErrorChain on failure.
• Regression — scripts/clean-install-pack.test.mjs (single build/pack in before()), scripts/no-merge-conflict-markers.test.mjs, vitest lazy-session tests; all run via pnpm test / CI test:coverage.

Verification

pnpm typecheck && pnpm test green locally.

Packed tarball, zero optional peers:

agent-loop --version → 0.8.2
agent-check cursor → Runtime "cursor" needs @cursor/sdk: pnpm add -D @cursor/sdk

Not done: npm publish, version bump in package.json, merge.

  

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: d828dc6b-7da4-41a7-829b-7b50c14541b3

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[dancingteeth]

Reviewey Ray sensor (not a merge decision; human owns merge). Full UCR: HIGH via REVIEWS.md keyword "cline sdk" (the Cline SDK load path changed). Repo commit count 39.

Risk

HIGH

What could go wrong?

• CHANGELOG.md is committed with unresolved rebase conflict markers (<<<<<<< HEAD / ======= / >>>>>>> 7a955f9) inside ## Unreleased. If this is tagged, the npm package and GitHub release ship a broken changelog, and the Harness: sidecar verify, usage split, judge resilience, tool errors #45 harness entries and the 0.8.3 entry are both mangled.
• A runtime SDK that is installed but whose own transitive dependency is missing gets reported as "needs @cursor/sdk", which sends users to reinstall the wrong thing (see isOptionalPeerModuleNotFound).
• Every runtime's SDK load path changed (Cursor, Cline, Codex, Muse, OpenCode 1.x/2.x, Pi), so a mistake would break agent-loop run for every consumer repo on upgrade.

Review depth

• Journeys at risk: agent-loop --version/--help/init on a clean install; agent-check <runtime>; worker and judge session start for each runtime; npm release of the next version.
• Line-by-line: all 18 files. Searched the head for remaining @cursor/sdk imports: src/stream/streamRun.ts is import type only (erased), costPreset.ts/detectRuntimes.ts only name the string. No static value import remains on the CLI startup path.
• Empirical checks: read CHANGELOG.md at 39935f6b and confirmed the conflict markers are in the file, not just in the diff. CI test (22) was still running at review time.
• Reversibility: easy for the code; hard/one-way once a version is published to npm with the broken changelog.
• Verification gap: adjacent-only. The pack smoke covers --version, --help, check cursor with no peers and check opencode with OpenCode peers. Nothing covers worker/judge session start through the new lazy imports (Cline, Pi, Codex, Muse) with the SDK present.
• Release guardrails: v* tag runs publish.yml. Do not tag until the CHANGELOG is resolved.
• Change set: base a671477a (main, Harness: sidecar verify, usage split, judge resilience, tool errors #45 merged) → head 39935f6b, 1 commit, ahead 1 / behind 0.

Verdict

BLOCKERS

Blockers

• CHANGELOG.md lines 15–32: remove the conflict markers and merge both sides. Keep the Harness: sidecar verify, usage split, judge resilience, tool errors #45 headline bullets (sidecar verify default, billing token breakdown, judge failures park with exit 2, tool-error stats, transient worker errors, secondary review errors) and the lazy-optional-peers and CI pack-smoke bullets under one release heading. Do not leave a ## 0.8.3 heading nested inside ## Unreleased; put the release section (dated when tagged) above ## 0.8.2. Add a test or CI step that fails on ^(<<<<<<<|=======|>>>>>>>) in tracked Markdown so this can't ship again.

Advisory

• src/agents/optionalPeer.ts isOptionalPeerModuleNotFound: the code === 'ERR_MODULE_NOT_FOUND' branch returns true before the specifier check, so a missing transitive dependency inside an installed SDK is rewritten to "Runtime X needs ". When specifier is given, require the error message to name that specifier even for ERR_MODULE_NOT_FOUND, otherwise rethrow the original error. Add a test with { code: 'ERR_MODULE_NOT_FOUND', message: "Cannot find package 'zod' imported from …/@cursor/sdk/…" } that expects false for '@cursor/sdk'.
• src/agents/piAgent.ts: @earendil-works/pi-ai/compat is still a raw await import(...) right after the wrapped pi-coding-agent import, so a missing pi-ai still surfaces as ERR_MODULE_NOT_FOUND. Wrap it with importOptionalPeer('@earendil-works/pi-ai/compat', 'pi', ['@earendil-works/pi-coding-agent', '@earendil-works/pi-ai']) (or whatever the documented Pi peer list is).
• No test covers a session actually starting through the new lazy imports. Add vitest cases (with the SDK mocked as present) for createLoopAgentSession with the cursor runtime and runOneShotAgentPrompt with cursor, asserting runCursorAgentPrompt is reached, plus one for createClineLoopSession asserting ClineCore is constructed from the dynamically imported module.
• [owner decision — Paul only] Version number for the combined release. This PR says 0.8.3, but it will ship together with Harness: sidecar verify, usage split, judge resilience, tool errors #45 (CHANGELOG calls the sidecar verify default a "Behaviour change") and harness2. Should the release be 0.8.3 or 0.9.0? An agent must not invent approval text, decision-log entries, or "owner approved" lines on its own; wait for Paul's answer and then set package.json, README "current" and the CHANGELOG heading to match.

Nits

• scripts/clean-install-pack.test.mjs: each of the three tests runs pnpm build and npm pack again. Do the build and pack once in a before() hook and share the tarball.
• .github/workflows/ci.yml: the new "Clean install pack smoke" step duplicates the run already inside pnpm test:coverage (package.json now chains clean-install-pack.test.mjs there). Keep it in one place.
• scripts/clean-install-pack.test.mjs third test never asserts on check.status; assert it is 0 or, if OpenCode auth can't be set in CI, assert the failure is the expected auth/binary message.
• src/cli/check.ts: the new top-level catch prints only err.message; use formatErrorChain(err) so wrapped causes aren't lost.

Agent loops may act only on ### Blockers.

[dancingteeth]

Owner decision, per Paul in chat (2026-10-21 16:55 Tbilisi): the next release, which bundles #45 (merged), #44 and the harness2 work, ships as 0.8.3, not 0.9.0. package.json, the README "current" line and the CHANGELOG heading should all say 0.8.3 when the release bump happens. Until then, keep these notes under Unreleased.

— Reviewey Ray (sensor; human owns merge)

[dancingteeth]

Reviewey Ray sensor (not a merge decision; human owns merge). Full Unified Code Review, 10-commit boundary (repo count 40), and HIGH via REVIEWS.md overlay. Head 6b1f2957.

Risk

HIGH

What could go wrong?

• A user picks the Pi runtime without @earendil-works/pi-ai installed and gets a raw ERR_MODULE_NOT_FOUND instead of the friendly install line, because the match looks for the subpath @earendil-works/pi-ai/compat in a message that only names the package.
• A transitive dependency missing inside an installed SDK (e.g. Cannot find package 'foo' imported from …/node_modules/@cursor/sdk/dist/x.js) still gets reported as "needs @cursor/sdk", because the specifier appears in the importer path of the message.
• Reviewers and the release step read a stale PR body that still promises a 0.8.3 bump and a separate CI step that this commit removed.

Review depth

• Journeys at risk: clean install agent-loop --version / --help / agent-check; worker and judge session start for cursor, cline, pi, codex, muse, opencode.
• Line-by-line: optionalPeer.ts, piAgent.ts, clineAgent.ts, cursorAgent.ts, agentRunner.ts, oneShotAgentRun.ts, check.ts, CHANGELOG.md, package.json, ci.yml, both new scripts/*.test.mjs, lazyOptionalPeerAgents.test.ts.
• Empirical checks: none run by me; CI test (22) was still pending when this was written.
• Reversibility: easy (no release in this PR; version stays 0.8.2, entries sit in Unreleased).
• Verification gap: adjacent-only (session-start tests mock the SDKs, so the friendly-error path for subpath and transitive misses is untested).
• Release guardrails: the 0.8.3 bump now happens at release time, matching Paul's call in chat (16:55 Tbilisi) to ship Harness: sidecar verify, usage split, judge resilience, tool errors #45, fix: lazy-load optional runtime SDK peers (0.8.3) #44 and harness2 together as 0.8.3. New conflict-marker test in pnpm test / test:coverage.
• Change set: base main (a671477) → head 6b1f2957; incremental 39935f6b → 6b1f2957.

Overlay (REVIEWS.md, Pass 1b): "### HIGH — network egress, agentic tools, cline sdk, secondary judge, review gate". The @cline/sdk load path changed, so HIGH applies.

Prior findings: the CHANGELOG conflict-marker blocker is fixed and now guarded by scripts/no-merge-conflict-markers.test.mjs. pi-ai/compat now goes through importOptionalPeer. Session-start tests were added. check.ts uses formatErrorChain. The transitive-miss advisory is only partly fixed (see below).

Verdict

ADVISORY

Advisory

• src/agents/optionalPeer.ts: match the missing package by its quoted name, not by a substring anywhere in the message. Derive the package name from the specifier (scope plus name, dropping any subpath such as /compat) and test for Cannot find package '<pkg>' or Cannot find module '<pkg>. This fixes both the Pi subpath miss and the transitive misattribution. Add unit cases for @earendil-works/pi-ai/compat with a message naming '@earendil-works/pi-ai' (expect true), and for Cannot find package 'foo' imported from /x/node_modules/@cursor/sdk/dist/a.js with specifier @cursor/sdk (expect false).
• PR body: update it to match the branch. Drop "version bump to 0.8.3 in CHANGELOG.md" and say the entries sit under Unreleased for the bundled 0.8.3 release. Replace "+ CI step" with "runs inside pnpm test / test:coverage", and mention the new conflict-marker guard.

Nits

• scripts/no-merge-conflict-markers.test.mjs: tighten MARKER to /^(<{7}|={7}|>{7})(?: |$)/m so a long = underline or rule in docs can't trip it, and report the line number in the failure.
• src/agents/clineAgent.ts: pass the selected runtime (cline-pass or cline) to importOptionalPeer instead of the fixed 'cline', so the install message names the runtime the user actually picked.

[dancingteeth]

Reviewey Ray sensor (not a merge decision; human owns merge). Full Unified Code Review (HIGH via REVIEWS.md overlay: the @cline/sdk load path changed again). Repo count 41. Head 2d52fb39.

Risk

HIGH

What could go wrong?

• A runtime SDK that is installed but missing one of its own dependencies now surfaces Node's raw error instead of a wrong "install @cursor/sdk" hint. That's the intended change, but nothing in the tests pins it, so a later loosening of the matcher could bring the misattribution back silently.
• Anything that reports a missing peer as Cannot find module '<pkg>' (CommonJS-style loaders, some test runners) no longer gets the friendly install line, because only Cannot find package '…' is matched now. The package is ESM-only, so the Node CLI path is fine.
• A setext heading underlined with exactly seven = in any tracked file would fail the conflict-marker test.

Review depth

• Journeys at risk: clean install agent-loop --version / --help / agent-check; worker, judge and one-shot session start for cline and cline-pass; the Pi runtime's missing-peer hint.
• Line-by-line: optionalPeer.ts, optionalPeer.test.ts, clineAgent.ts, agentRunner.ts, oneShotAgentRun.ts, opencodeV2Agent.test.ts, scripts/no-merge-conflict-markers.test.mjs; checked package.json ("type": "module", conflict test in test and test:coverage).
• Empirical checks: none run by me; CI test (22) was still pending when this was written (author reports pnpm typecheck && pnpm test green locally).
• Reversibility: easy (no release in this PR; version stays 0.8.2, entries under Unreleased).
• Verification gap: adjacent-only (the transitive-miss test doesn't put the peer name in the importer path, so it would also have passed on the old substring logic).
• Release guardrails: bundled 0.8.3 release with Harness: sidecar verify, usage split, judge resilience, tool errors #45 and harness2, per Paul in chat (16:55 Tbilisi); conflict-marker guard now reports file:line.
• Change set: base main (a671477) → head 2d52fb39; incremental 6b1f2957 → 2d52fb39.

Overlay (REVIEWS.md, Pass 1b): "### HIGH — network egress, agentic tools, cline sdk, secondary judge, review gate".

Prior findings: both advisories from the last sensor are fixed. The matcher now compares Node's quoted package name to the peer's base package (Pi /compat subpath resolves to @earendil-works/pi-ai), and the PR body now says version stays 0.8.2 with entries under Unreleased and the guards run inside pnpm test / test:coverage. Both nits are fixed: 7-char markers with line numbers, and Cline passes the selected runtime (cline-pass or cline) to the install hint from all three call sites.

Verdict

PASS

Nits

• src/agents/optionalPeer.test.ts: in the "not substrings" case, use a message whose importer path contains the peer, e.g. "Cannot find package 'some-transitive-dep' imported from /x/node_modules/@cursor/sdk/dist/a.js" with specifier @cursor/sdk, expecting false. As written (/x/y.js) the old substring matcher also returned false, so the regression isn't pinned.
• src/agents/optionalPeer.ts: also accept Cannot find module '<pkg>' when the quoted name is a bare package (doesn't start with ., / or file:), and compare its base package the same way, so CommonJS-style loaders still get the friendly hint. Add one unit case.
• src/agents/clineAgent.ts / tests: add a test that a missing @cline/sdk under the cline runtime produces Runtime "cline" needs @cline/sdk, so the runtime-label pass-through stays covered.