Repository navigation
fix(hooks): launch OpenCode through the installed Node entry - #1336
Conversation
|
No findings in the specified diff. The PR description satisfies the testing requirement: it records built-CLI verification with an empty Review was read-only; I did not run, build, or install anything from the PR. |
There was a problem hiding this comment.
This was generated by AI during triage.
@nateEc Thanks. The launch fix works, but it breaks an existing e2e test, which CI also shows.
We built this head, its merge-base 6c57c3b7 and main 83f6d1db. With PATH='', this head's plugin dispatches session-start through the built CLI, while base and main don't. With teamai on PATH, base dispatches too, so the control holds. The new launch e2e fails on base and passes here, and the unit tests, tsc and lint pass. A live OpenCode V1 (1.18.23) run passes too.
[P1 blocking]src/__tests__/e2e/opencode-versions-915.test.ts:282("rewrites a plugin from an older build and takes the V1 entries out in the same pull; back on V1 they return") fails here: 7/8 pass, while base passes 8/8. CI's "E2E (fork-safe, no credentials, shard 3/3)" fails on the same test at this head. The test compares the pulled plugin withbuildPluginSource(home)computed in the vitest process, so the embedded entry path differs from the CLI's. Passing the built CLI path as the second argument there makes it pass 8/8 in a scratch copy.[P2 non-blocking]On OpenCode V1, a stale embedded Node path fails silently, because doctor checks the plugin's readiness only on V2. On V2, a doctor or pull from a different install rewrites the plugin to its own path (read from the code, not run).[P3 nit]The errorbuildPluginSourcethrows doesn't say what to do. The spawn template copies OpenClaw's (#1307); a shared helper would keep them in step.
CI review: it reported no findings and accepted the testing record (the empty-PATH run and a live OpenCode V1 run, both reproduced here). We disagree with "no findings": it missed the opencode-versions-915 regression, which CI's own E2E shard 3/3 caught.
@nateEc this now conflicts with |
- Embed the installing Node executable and resolved TeamAI CLI entry in the built-in OpenCode plugin and pass the script as a separate spawn argument, without a Windows shell or PATH lookup. - Preserve an existing plugin with an actionable warning when the entry cannot be resolved, and make plugin readiness use the same entry-aware rendering. - Update bilingual diagnostics and troubleshooting; adapt the real OpenCode payload-capture fixture to an installed-entry wrapper instead of a PATH shim. - Verify 221 focused tests, including recall attribution through the entry-prefixed argv harness, a real OpenCode V1 session-start load, and a CLI-generated plugin invoking the real dispatcher with an empty PATH and a spaced HOME; type checking, lint, and build pass. Live V2 is deferred because no binary is configured.
- 在插件升级回归用例中显式传入编译后 CLI 入口，匹配实际 pull 生成的绝对启动路径。 - 保留旧版本往返、V1 配置清理和无法写入插件时的保护断言。 - 59 个相关单测、9 个真实 CLI 用例、构建、类型检查和 lint 通过。
fb0ecd9 to
d25a286
Compare
|
Addressed the review follow-up at
The PR description contains the updated verification record. Ready for another review. |
|
No findings in the specified diff. The PR description satisfies the testing requirement: it records real built-CLI verification with an empty The earlier review raised no findings to resolve. Review was read-only; I did not run, build, or install anything from the PR. |
SaulMoro
left a comment
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Approving at d25a286. The opencode-versions-915.test.ts:282 regression is fixed: the test now passes the built CLI path to buildPluginSource, the file passes 8/8 locally, and CI's E2E shard 3/3 is green. The rebase onto 6ebb31c leaves the PR's content unchanged. Build, tsc, lint and the PR's unit and e2e files pass. With PATH='', this head's plugin dispatches session-start through the built CLI and the 6ebb31c baseline does not. The new launch e2e is red on the baseline.
Still open and non-blocking, from the earlier review:
- [P2 non-blocking] On OpenCode V1, a stale embedded Node or CLI path fails silently, because doctor checks the plugin only on V2 (
src/doctor-delivery.ts:740-742). A plugin withNODEpointing at a missing path exits 0 and logs no dispatch.hooks injectorpullrepairs it. - [P3 nit] The error
buildPluginSourcethrows doesn't say what to do, and the spawn template copies OpenClaw's (#1307).
The CI review reports no findings for this head and accepts the testing record. We agree for this head. All CI jobs pass.
Summary
OpenCode's generated plugin launched bare
teamai, so service environments without it on PATH silently lost hook dispatch. Embed the installing Node executable and resolved CLI entry and pass the entry as its own argument without a Windows shell. If the entry cannot be resolved, warn and retain the installed plugin; readiness uses the same rendering.Type of Change
Test Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest runpasses entirely (known baseline failures documented below)Added/updated tests for the change
npm run buildpassesnpx vitest run src/__tests__/opencode-hooks.test.ts src/__tests__/recall-attribution.test.ts: 221 passed, including POSIX/Windows spawn arguments with spaced paths, entry-resolution failure, and V1/V2 payload bridges. The two launch-argument regressions fail on main. The recall acceptance harness now reads the event after the entry-script argument; its end-to-end attribution assertions remain unchanged.Real built CLI and OpenCode:
opencode-hooks.test.tsthrough the E2E config: V1 passed, V2 skipped because no binary is configured. OpenCode 1.18.23 loads the CLI-generated plugin, forwards one session-start payload, and runs the enterprise hook. The capture fixture now injects through an installed-entry wrapper that delegates other commands to the real CLI.Real built CLI, empty PATH:
opencode-hook-launch.test.tspassed. A generated plugin under a HOME containing spaces runs withPATH=''and invokes the real dispatcher, confirmed by its persisted dispatch record.Broad validation: combined type check, lint, build, and 10 critical real-CLI E2E cases passed. The full five-branch unit run took 614.07s: 499 files; 12,244 passed, 36 skipped, 7 failed. Five failures came from the recall acceptance harness reading the old OpenCode argv position; a one-line test-only adapter fix preserves the attribution assertions. All 221 OpenCode/recall tests pass on the final combined snapshot after that correction; runtime source did not change.
The remaining two failures are independently reproduced on clean
6c57c3b7with Apple Git 2.50.1:submodule-checkout.test.tscannot readhook.teamai-post-checkout.commandafter preserving empty legacy Claude/Codex settings (24 passed, 2 failed on clean main). The full-suite checkbox stays unchecked for these baseline failures.Local verification used macOS, Node 22.22.1, and local Git fixtures. GitLab/GitHub provider runs and additional live agent/Windows coverage were not run locally; remote CI cases may skip without credentials.
Related Issues
Related #1221: addresses the OpenCode plugin PATH-dependent spawn finding, matching the installed-entry approach already used for OpenClaw.
Notes for Reviewers
After moving/reinstalling Node or TeamAI, run
teamai hooks injectorteamai pulland restart OpenCode to refresh the recorded paths. This is documented in English/Chinese diagnostics and troubleshooting. Live OpenCode V2 and native Windows were not run locally.Follow-up verification (2026-10-25)
opencode-hooks.test.ts: 59 passed. Built-CLIopencode-versions-915plus the empty-PATH launch case: 9 passed, retaining V1/V2 round-trip and failure protection assertions.Updated head:
d25a286ed4cdf0a4cf3c50cbcaddcfb7592e8eb5. Rebased onto main6ebb31ce.