[nateEc]

A member file blocking Hermes's skills directory made delivery skip the tool, doctor report success, and uninstall fail with ENOTDIR. Check directory usability independently of last-delivery records and guard uninstall traversal. Keep the member file while cleaning the tool's other owned resources; no-op uninstall must not claim a removal.

Review follow-up:

Keep Hermes cleanup in removal-plan emptiness, but only when an owned script/plugin or exact hook registration/approval actually exists. A missing plugin directory is writable, not evidence of an installation. Repeated targeted uninstall now reports Nothing to uninstall and preserves the blocking member file and the other tool's resources.

Merged main at f70ce259 (after #1337 and #1340). The conflicts were only in the docs and skill-data troubleshooting: main's generated-hook paragraph and Pi section are kept beside this branch's text, and the zh-CN diagnostics sentence now states its condition once. No commit on main since the previous base touches this branch's source files. Added unit tests for hasHermesHooks() with a hook registration left alone and an approval left alone.

Test Plan

Head ef5928b0, macOS, Node 24.21.0:

• npx tsc --noEmit, npm run lint, npm run build: passed.
• npx vitest run src/__tests__/doctor-delivery.test.ts src/__tests__/uninstall.test.ts src/__tests__/hermes-hooks.test.ts src/__tests__/hermes-config.test.ts: 293 passed.
• npx vitest run: 16425 passed. The only failures were 15s timeouts in workflow-reconciliation.test.ts under full-suite load; that file passes alone (7 passed) and is not touched here.
• npm run test:e2e -- src/__tests__/e2e/blocked-skills-directory.test.ts: 2 passed.

Built CLI, isolated HOME/GIT_CONFIG_GLOBAL, local seed repo, user scope with Hermes and Claude enabled, ~/.hermes/skills a regular file:

$ teamai pull --force
⚠ Skipped fixture (hermes): …/.hermes/skills is not a directory; the existing file was left unchanged. Move the file aside and run `teamai pull` again.
exit=0
$ ls ~/.hermes/hooks ~/.hermes/plugins
teamai-status-report.sh
teamai-instructions
$ teamai uninstall --agent hermes --force
⚠ Kept …/.hermes/skills: …/.hermes/skills is not a directory; the existing file was left unchanged.
✔ Removed teamai Hermes hook from …/.hermes/hooks/teamai-status-report.sh
✔ teamai uninstalled
exit=0
$ ls ~/.hermes/hooks/teamai-status-report.sh ~/.hermes/plugins/teamai-instructions
No such file or directory (both)
$ cat ~/.hermes/skills
member file
$ teamai uninstall --agent hermes --force
⚠ Kept …/.hermes/skills: …/.hermes/skills is not a directory; the existing file was left unchanged.
ℹ Nothing to uninstall
exit=0

Native Windows, live vendor applications and other provider/agent combinations were not run locally and are left to CI.

Refs #1221; this PR addresses only its stated independent defect.

[github-actions]

No findings in the specified PR diff.

The PR description documents sufficient testing, including a built-CLI E2E record covering blocked-directory diagnostics, repair, and targeted uninstall. Deferred provider/agent coverage does not block merge.

Review was read-only; no PR code was run, built, or installed.

[SaulMoro]

This was generated by AI during triage.

@nateEc Thanks. The doctor half and the ENOTDIR crash are fixed. One thing blocks.

We built this head and its merge-base, and put a file at ~/.hermes/skills:

• doctor: base exits 0 with "All checks passed!". This head exits 1 and names the blocked path.
• uninstall --force: base and current main exit 1 with ENOTDIR. This head exits 0 and keeps the file.
• After moving the file aside, a plain pull delivers Hermes again, and doctor passes.

The new tests fail on base for the intended reason, and tsc and lint pass.

• [P1 blocking] The targeted uninstall your e2e runs doesn't clean Hermes. teamai uninstall --agent hermes --force with the blocking file present exits 0 with:

  ⚠ Kept …/.hermes/skills: …/.hermes/skills is not a directory; the existing file was left unchanged.
  ℹ Nothing to uninstall
  

  The Hermes hook script and the instructions plugin stay. The body and the new diagnostics and troubleshooting text say uninstall "continues cleaning the tool's other managed resources". isPlanEmpty (src/uninstall.ts:1435) doesn't count hermesCleanup, so the early return at :2483 runs before the Hermes cleanup at :2401. The early return predates this PR; main also stops early when Hermes has nothing else in the plan. But this PR turns the blocked case into a silent incomplete uninstall and documents the opposite. Counting hermesCleanup in isPlanEmpty, with an assertion in the e2e that the hook script is gone, would fix it. Narrowing the docs and body to "uninstall no longer fails" is the alternative.
• [P2 non-blocking] #1265 changes writesOf() to return objects and conflicts with this PR in src/doctor-delivery.ts, so whichever lands second needs a rebase.
• [P3 nit] The new troubleshooting section repeats the diagnostics guide, and the zh-CN diagnostics sentence says the same clause twice.

CI review: it reported no findings and accepted the testing record. We agree the e2e record is real, but not with "no findings": it missed the targeted-uninstall gap above.

[SaulMoro]

This was generated by AI during triage.

@nateEc this now conflicts with main in src/doctor-delivery.ts (#1265 changed writesOf() to return objects) and skill-data/core/references/troubleshooting.md. When you rebase, the blocking finding from my review is still open: teamai uninstall --agent hermes --force with the blocking file present exits 0 but leaves teamai's Hermes hook and plugin behind.

[nateEc]

Addressed the review follow-up at b351302e46a90b2d74526c6d159a6e2997b82658 on current main.

• Count Hermes adapter cleanup in isPlanEmpty, so targeted uninstall proceeds even when a member file blocks the skills directory. Adapted the directory check to main's location objects.
• Strengthened the real-CLI regression to assert that both the Hermes hook script and instructions plugin disappear while the blocking member file and Claude skill remain. It failed before the plan fix and passes afterwards.
• Doctor/uninstall unit tests: 269 passed. Built-CLI blocked-skills E2E: 2 passed. Build, type checking and lint passed.

The PR description contains the updated verification record. Ready for another review.

[github-actions]

No findings in the specified PR diff. The earlier review raised no findings to resolve.

The PR description documents sufficient testing, including built-CLI E2E verification of blocked-directory diagnostics, repair, and targeted uninstall. Deferred provider/agent coverage does not block merge.

Review was read-only; no PR code was run, built, or installed.

[nateEc]

Addressed the review follow-up at 784c693f376709157367c7561efac6339ac3acca on current main.

• Count Hermes adapter cleanup in isPlanEmpty, so targeted uninstall proceeds even when a member file blocks the skills directory. Adapted the directory check to main's location objects.
• Strengthened the real-CLI regression to assert that both the Hermes hook script and instructions plugin disappear while the blocking member file and Claude skill remain. It failed before the plan fix and passes afterwards.
• Doctor/uninstall unit tests: 269 passed. Built-CLI blocked-skills E2E: 2 passed. Build, type checking and lint passed.
• Rebased again onto main a26f74d after fix(hooks): launch OpenCode through the installed Node entry #1336 merged; retained both troubleshooting sections. Rebase verification: 269 focused unit tests and 2 built-CLI E2E cases, build, types and lint passed.

The PR description contains the updated verification record. Ready for another review.

[github-actions]

No findings in the specified PR diff. The earlier reviews raised no findings to resolve.

The PR description documents sufficient testing, including built-CLI E2E verification of blocked-directory diagnostics, repair, and targeted uninstall. Deferred provider/agent coverage does not block merge.

Review was read-only; no PR code was run, built, or installed.

[SaulMoro]

This was generated by AI during triage.

@nateEc Thanks, approving at 784c693. The blocking finding is fixed: isPlanEmpty now counts hermesCleanup (src/uninstall.ts:1466). With the built CLI and a file blocking ~/.hermes/skills, uninstall --agent hermes --force exits 0, removes the Hermes hook and the instructions plugin, and keeps the file. The base exits 1 with ENOTDIR and leaves both behind. doctor --json now exits 1 with hermes skills directory is usable failing and the blocked path in its fix, where the base said "All checks passed!". Both halves of the #1221 item are met. The latest push is a rebase onto a26f74d that only keeps #1336's troubleshooting section next to this one.

• [P2 non-blocking] A no-op Hermes uninstall now reports a removal. User scope, Hermes enabled, never pulled: uninstall --agent hermes --force prints ✔ Removed teamai Hermes hook from …/teamai-status-report.sh and ✔ teamai uninstalled, while the base says Nothing to uninstall. The hook file is absent before and after. hermesCleanup is true for any user-scope uninstall that includes Hermes, and removeHermesHooks() logs success unconditionally (src/hermes-hooks.ts:201). Logging only when something Hermes-owned existed would fix it.
• [P3 nit] The troubleshooting section repeats docs/guide/diagnostics.md, and the zh-CN diagnostics sentence states its condition twice. The body's "Follow-up verification (2026-10-25)" and the five-branch "Broad validation" paragraph describe more than this diff.

The CI review reports no findings and says the earlier review raised none. We agree that nothing blocks now, but our earlier review did raise the uninstall finding above. CI at this head is still running.

[SaulMoro]

This was generated by AI during triage.

@nateEc This is approved, but it now conflicts with main in skill-data/core/references/troubleshooting.md after #1343 landed (#1343, #1333 and #1331 merged together). Each of these PRs appends a section at the end of that file, so keeping both sections resolves it. Once it's rebased and CI is green, I'll merge it.

[github-actions]

No findings in the specified diff (fc8a229...2f417681). Earlier reviews raised no findings to resolve.

The PR description documents sufficient testing, including built-CLI E2E verification of blocked-directory diagnostics, repair, and targeted uninstall. Deferred provider/agent coverage does not block merge.

The body references a newer head; this review remains limited to the requested commit range. Review was read-only: no PR code was run, built, or installed.

[nateEc]

Updated and rebased onto main fc8a229e. Keep Hermes cleanup in removal-plan emptiness, but only when an owned script/plugin or exact hook registration/approval actually exists. A missing plugin directory is writable, not evidence of an installation. Repeated targeted uninstall now reports Nothing to uninstall and preserves the blocking member file and the other tool's resources.

Verification: 290 focused unit tests passed; types, lint and build passed. Post-build real-CLI blocked-skills-directory: 2 passed. Native Windows and live vendor/provider combinations remain CI coverage. The PR body now describes this branch's current verification rather than an older combined snapshot.

Updated head: 2f417681.

[SaulMoro]

This was generated by AI during triage.

@nateEc Re-checked at 2f41768. Our approval stands. With the built CLI, a targeted uninstall now removes the Hermes hook and plugin and exits 0, where the base exits 1 with ENOTDIR. A repeat uninstall prints "Nothing to uninstall". Build, tsc, lint and the related unit and e2e files pass, and CI was green at this head.

It conflicts with main again since #1340 merged a few minutes ago. Please rebase; we will merge as soon as CI is green on the rebased head.

Non-blocking notes:

• [P2 non-blocking] hasHermesHooks() and hasHermesHookCommands() have no unit test for the registration-only and approval-only cases. A dangling registration works when checked by hand.
• [P3 nit] The success line names the script even when only the plugin was removed (read from the code, not run). The duplicated troubleshooting text and the zh-CN sentence from our earlier review are still there.

[github-actions]

No findings in the specified diff (f70ce259...ef5928b0). Earlier reviews raised no findings to resolve.

The PR description documents sufficient testing, including built-CLI E2E verification of blocked-directory diagnostics, repair, targeted uninstall, and repeated no-op uninstall. Coverage deferred to CI does not block merge.

Review was read-only; no PR code was run, built, or installed.

[SaulMoro]

Thanks @nateEc. I merged main into the branch (the troubleshooting and diagnostics conflicts with #1340, keeping both sections) and added the registration-only and approval-only tests for hasHermesHooks. The targeted uninstall still behaves as approved with the built CLI, and CI is green at ef5928b.