Repository navigation
fix(hooks): use the HOME ZCode config in self scope - #1330
Conversation
- Resolve ZCode hooks through user-scope paths in every installation mode, and make reconciliation, hooks list and doctor inspect the same HOME CLI config. - Retire manifest-owned entries from the unread self-project file before recording their HOME replacements, preserving personal settings and unresolved team-hook ownership. - Gate team hooks to their project and retain other projects' hooks, global built-ins and the Windows launcher when removing one project's entries; persist its agent exclusion so a later pull cannot install it again. - Verify 923 related regression tests including migration/ownership, legacy sweeps, uninstall and path overrides, TypeScript, lint and build; the real CLI passes self init, hooks inject/list/remove, uninstall the ZCode doctor registration check and a post-uninstall pull.
|
No confident findings in the specified PR diff. The PR description satisfies the testing requirement: it records a successful build and representative real-CLI verification covering injection, listing, doctor, removal, uninstall, and post-uninstall pull. Additional providers and live agent applications are explicitly deferred. 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.
@nateEc Checked 6c9fd44a. The ZCode self-scope HOME location and doctor fix loop are resolved. Built-CLI comparisons fail on base/main and pass here. The two-project run also preserves the other project's hooks and personal settings; targeted ZCode uninstall stays excluded after pull.
Keeping the five HOME built-ins is intentional and documented in the PR body. They exit silently when no config applies. This does not block approval.
- [P2 non-blocking] Align removal guidance with the retained global channel.
hooks removestill says all hooks were removed, while ZCode's built-ins remain and can deliver rules while project config exists. The uninstall reference and FAQ also need the ZCode exception. State that project removal removes its team hooks, retains the shared built-ins, and that targeted uninstall records an exclusion. - [P2 non-blocking] Targeted uninstall with ZCode as the only tool prints
shared resources kept for remaining tools. The configuration is retained for the project exclusion, so the message should name that reason without claiming another tool exists.
CI review reported no confident findings and accepted the representative real-CLI record. We agree with both assessments. Current CI jobs passed. Verification exercised CLI/filesystem contracts with the Git provider; no live ZCode application or Windows session was run.
This PR owns the ZCode self-location fix. #1265 owns the broader rules/blocks/agents/MCP work and leaves the remaining hook-path migration for B4. #1316 owns ZCode shell/reply handling; #1343 owns reply-size diagnosis.
Summary
ZCode reads CLI hooks from
~/.zcode/cli/config.json, but self mode wrote them under the project. Doctor therefore kept recommending an inject command that rewrote the same unread file.Type of Change
Test Plan
npx tsc --noEmitpassesnpm run lintpassesnpx vitest runpasses completely (two unchanged local baseline failures; see below)Added/updated tests for the change
RED: reconciliation created only the project config and left the HOME file missing.
923 related unit tests pass, followed by 267 focused uninstall/lifecycle tests after the persistent-exclusion follow-up, including hook commands, doctor, uninstall, legacy sweeps, two-project ownership and user-scope path overrides.
TypeScript, lint and build pass.
Real CLI: the ZCode self fixture passes
init .,hooks inject,hooks list, the doctor registration check,hooks remove, reinjection anduninstall --agent zcode --force. Its project team hook disappears on removal/uninstall while the shared HOME built-ins remain; a following pull keeps the tool excluded.Real-CLI command:
npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/zcode-self-hooks.test.ts.Repository-wide regression: the five-patch combined run on
origin/main6c57c3b7completed with 12,268 passed, 36 skipped, 2 failed across 503 files. Both failures are the existingsubmodule-checkoutcases for preserving empty old Claude/Codex hook files (git config --get hook.teamai-post-checkout.command). An independent pristine checkout of the same base reproduces exactly those 2 failures (24 other submodule tests pass). No new full-suite failure was introduced. The final ZCode exclusion follow-up is additionally covered by 267 uninstall/lifecycle tests and a post-uninstall pull through the real CLI.The final combined build, TypeScript and lint checks also pass. A final real-CLI run with all five patches passes 6 critical cases across all 5 affected E2E files, including the post-uninstall pull.
Related Issues
Related to #1221: ZCode self/project hook delivery.
Notes for Reviewers
Resolve ZCode hook paths at user scope in init/pull, legacy reinjection, hook commands, doctor and uninstall. Migrate manifest-owned entries out of the old project file while preserving personal settings. HOME team hooks remain gated to their project; removing or uninstalling one project preserves other projects, global built-ins and the Windows launcher. Keep the tool exclusion after agent-specific uninstall so the next pull cannot bootstrap it back. A legacy-project sweep still targets the old project file rather than clearing the newly written HOME config. Both guide languages describe the location.
The CLI and filesystem contracts are exercised with an isolated HOME; this does not claim a live ZCode application session was run. Other findings in #1221 remain separate.
Local runtime tests use the Git provider and isolated filesystem fixtures; additional providers and live agent applications are deferred to CI/manual verification.