Skip to content

fix(doctor): accept live hook launchers from another CLI installation - #1409

Merged
SaulMoro merged 1 commit into
Tencent:mainfrom
nateEc:fix/1221-installed-launcher-readiness
Oct 26, 2026
Merged

SaulMoro merged 1 commit into
Tencent:mainfrom
nateEc:fix/1221-installed-launcher-readiness

Conversation

@nateEc

@nateEc nateEc commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Doctor compared an installed handler byte-for-byte with source generated by the inspecting CLI. Two valid copies of the same TeamAI build therefore reported healthy hooks as missing or stale whenever their saved Node/CLI entry paths differed.

Compare the handler body while normalizing only the known generated launcher metadata, and require the saved absolute launcher paths to exist. Pi/OMP extension checks, OpenCode plugin checks and Hermes instructions-plugin checks share this rule. Handler changes, missing saved launchers and manifest changes still fail readiness. Inspection does not rewrite installed files. English/Chinese diagnostics and troubleshooting explain the behavior.

Follow-up to the review notes on #1365 and #1367 (also applicable to #1336/#1372). Refs #1221.

Validation on main fc8a229e:

  • RED: the new Pi and OpenCode cross-install readiness regressions both fail before the fix.
  • npx vitest run src/__tests__/installed-launcher-readiness.test.ts src/__tests__/instruction-targets.test.ts src/__tests__/opencode-hooks.test.ts: 328 passed. Covers other live entries, removed entries, changed handler bodies, and Bun/Hermes launcher forms.
  • npx tsc --noEmit, npm run lint, npm run build, git diff --check: passed.
  • After build, npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/installed-launcher-readiness.test.ts: 1 passed. The real CLI injects through entry A (space/apostrophe path); entry B checks doctor against the same build without rewriting the extension. Removing A makes the same check fail.

Local verification: macOS, Node 22.22.1, isolated Git/HOME fixtures. Native Windows, live vendor applications and additional provider combinations were not run locally.

- 仅规范化已知启动器路径元数据,保留 handler 源码与插件 manifest 的一致性检查。
- 验证已安装入口仍存在,跨 CLI 副本检查不再误报;路径失效和逻辑修改仍判定异常。
- 补充中英文诊断说明;328 项定向单测和跨入口真实 CLI 场景通过。
@github-actions

Copy link
Copy Markdown

No confident findings in the specified PR diff.

The PR description includes sufficient testing: a post-build real-CLI run verifies cross-install readiness, confirms inspection does not rewrite the extension, and checks failure after removing the saved launcher. The stated platform/provider limitations do not block merge.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This was generated by AI during triage.

@nateEc Approving at 3452982. Two separate review passes agree.

  • Built CLI: with two entries of the same build, doctor reports the hook healthy on this head while the other entry's launcher exists. It fails on the merge base and on main, and doctor leaves the file byte-for-byte unchanged. Removing that launcher fails the check on all three.
  • First pull from a second entry (OpenCode V2): it delivers the new rule and culture, rewrites the plugin to the second entry's launcher, and leaves every doctor check passing. The merge base ends in the same state after writing V1 instructions and deleting them in the same pull. A second pull changes nothing on either build.
  • Normalizer: a code change can't hide in the normalized lines. Each captured value must be plain JSON holding only absolute paths, and the rest of the file must match exactly. Paths with spaces, apostrophes, quotes, backslashes and newlines check correctly. hooks inject, pull and install still rewrite a file that names another install's launcher.
  • Tests: the new tests fail on the baseline. The PR merges cleanly into current main, where tsc, the three unit files and the three e2e files pass.

Non-blocking notes:

  • [P2 non-blocking] Readiness only checks that the saved paths exist. A directory, swapped Node and entry values, or NODE=/bin/sh all pass. Readiness is also read by the recall toggle (recall-toggle.ts) and the local agent (local-agent.ts), which don't rewrite the launcher, so with such a value they hold back their fallback entries. Checking that the saved Node's basename is node or bun (.exe on Windows) would close most of it. The docs say "another installation of the same build", but an older release whose generated handler is unchanged also passes. That's fine behavior, though the wording could say "same handler".
  • [P3 nit] The Hermes arm only matches #1367's launcher form, so this should land before or with #1367. The JSDoc rewrite in src/opencode-hooks.ts (around 415-418) is unrelated. The commit body is in Chinese, and a squash merge keeps it on main.

The one red job (Node 20 ubuntu) failed in agent-observation.test.ts with ENOTEMPTY during cleanup. That's the git-gc race #1408 fixed in another file, outside this diff. I've re-run it. The CI review lists the right behaviors, and we add the notes above.

@nateEc

nateEc commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

The P2 launcher-validation note is addressed in #1431. The four directory/swapped-runtime/shell cases fail before the fix and now pass; saved paths must be files and the runtime role must name node or bun. The same-handler wording is corrected too. 333 related unit tests and the post-build cross-entry doctor CLI scenario pass; the full unit suite is running.

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.

3 participants