Repository navigation
fix(doctor): detect oversized ZCode hook rule replies - #1343
Conversation
|
No findings in the specified diff. The PR description documents sufficient testing: a real built-CLI E2E run verifies failure at the default byte cap and success after increasing it. The unavailable live-host and additional platform/provider coverage is explicitly disclosed and is not blocking. Review was read-only; I did not run, build, or install PR code. |
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
@nateEc Thanks, approving. This covers the ZCode hook-stdout part of the #1221 shared-budget item.
We built this head, its merge-base 83f6d1db and main 54622c3a, and delivered project rules whose session-start reply is 33,107 bytes, as measured from the real hook output. On this head, teamai doctor fails the new check at the 32,768-byte default ("The project rules alone need 33107 UTF-8 bytes of hook stdout …") and passes once the cap is raised to 65,536. Base and main show no check and exit 0 with the same reply. The new unit and e2e cases fail on base and pass here, the doctor and ZCode unit tests pass, and tsc, lint and CI pass.
Suggestions:
[P2 non-blocking]The fix text and the EN/zh reference docs say ZCode "rejects" an oversized reply. Going by ZCode 3.16.2-38's bundle (vendor/zcode.cjs), it truncates stdout at the cap and then silently drops the reply it can no longer parse. The outcome is the same, but "truncates and drops it" tells the user why nothing shows up. We read the bundle and did not run a ZCode session.[P3 nit]#1332 put its budget note indiagnostics.md, so this paragraph could live there too. The new test case could join the existingdescribeblock.
CI review: it reported no findings. We agree nothing blocks. The wording point above is the only addition.
Still approved, but this now conflicts with |
ad8377d to
b8f99c9
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 documents sufficient testing: a real built-CLI E2E run verifies failure at the default byte cap and recovery after increasing it. Unavailable live-agent and additional platform/provider coverage is explicitly disclosed and is not blocking. The earlier review reported no findings, so there are none to mark resolved. Review was read-only; I did not run, build, or install PR code. |
SaulMoro
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
@nateEc Approving again at b8f99c9. The rebase changes only text. The fix message and both language versions of the docs now say ZCode truncates oversized stdout and drops the reply it cannot parse, which resolves the earlier wording note. Build, tsc, lint and the PR's unit and e2e files pass. On the built CLI the check exits 1 at the default cap and 0 at a 65536 cap, and the base has no check. The new tests fail on the base.
One more thing before it can land: it now conflicts with main in skill-data/core/references/troubleshooting.md, after #1336 added its section at the end of the same file. Keeping both sections resolves it.
- [P2 non-blocking] The check measures the rules reply only. With recall on, the session-start reply also carries the recall hint: 36,272 bytes against 33,107 for rules alone in the same fixture. Rules a little under the cap would pass doctor while ZCode still drops the full reply. The PR calls it a lower bound; saying so in the check's message would help.
- [P3 nit] The earlier notes still apply: the budget note could sit with #1332's in
docs/guide/diagnostics.md, and the new e2e case is outside anydescribe. The body dates the follow-up 2026-10-25.
The CI review reports no findings. We agree that nothing blocks. All CI jobs pass.
- Measure the serialized project-rules hook reply in UTF-8 bytes against ZCode hooks.maxOutputBytes, using the documented 32768-byte default. - Fail doctor with the required byte count and concrete recovery steps while keeping the user cap and full rule content unchanged. - Describe the rules size as a lower bound before other hints and match the dispatcher output without an appended newline; update bilingual reference and troubleshooting. - Verify multibyte/default/custom/exact-boundary regressions and a built-CLI pull/doctor scenario, type checking and lint; live ZCode is unavailable locally.
- 明确 ZCode 会截断超限标准输出,并静默丢弃无法解析的回复,与英文说明和诊断提示保持一致。 - 已在当前主分支上重放改动,保留会话归档排查说明。 - 构建、类型检查和 lint 通过;52 个相关单测和 43 个真实 CLI 端到端用例通过。
b8f99c9 to
a86be69
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 documents sufficient testing: a real built-CLI E2E run verifies failure at the default byte cap and recovery after increasing it. Unavailable live-agent and additional platform/provider coverage is explicitly disclosed and is not blocking. Earlier reviews reported no findings, so there are none to mark resolved. Review was read-only; I did not run, build, or install PR code. |
Summary
Doctor could report a registered, enabled ZCode hook as healthy when its project-rules reply alone exceeded ZCode's stdout budget and never reached the model. Measure the actual JSON serialization in UTF-8 bytes against the configured hooks.maxOutputBytes, defaulting to 32768, and provide a concrete repair instead of silently passing.
Type of Change
Test Plan
npx tsc --noEmitnpm run lintnpm run buildAdded behavioral regression coverage
Entire unit suite green (baseline failures documented below)
npx vitest run src/__tests__/project-hook-rules.test.ts: 49 passed, 1 Windows-only case skipped. A multibyte rules reply fails the default byte budget, passes a larger configured cap, passes at the exact serialized byte count, and fails one byte below it. The missing diagnostic reproduces on unchanged main.Real built CLI: after
npm run build, the new byte-cap case ininstruction-targets.test.tsthrough the E2E config passes. Pull/doctor detects 11,000 multibyte characters exceeding the default cap, then doctor passes after the fixture explicitly increases the user cap.A live ZCode host session is unavailable locally. The config field/default and session snapshot behavior come from official ZCode hook documentation.
Broader regression record: a full combined runtime/test snapshot of these five fixes on
main83f6d1dbrannpx vitest run --maxWorkers=8: 12,266 passed, 36 skipped, 4 failed across 502 files. Two failures were Pi normal-file diagnostic labels, subsequently corrected; the final combined rerun ofinstruction-targets.test.tsanduser-rules-files.test.tspasses 373/373, including both regressions. The other two are existingsubmodule-checkout.test.tsempty legacy Claude/Codex settings cases: a separate clean83f6d1dbworktree reproduces exactly 24 passed / 2 failed on Apple Git 2.50.1 (missinghook.teamai-post-checkout.command). They are not hidden or skipped here.The final combined snapshot passes type checking, lint and build. Four focused real built-CLI files pass 14 tests on the updated main; the final Pi/ZCode integration rerun passes 2/2 after the diagnostic-label correction. An earlier broader run of those four E2E files passed 56/56.
Local verification used isolated HOME directories and local Git remotes on macOS / Node 22.22.1. Additional live agent, native Windows and GitLab/GitHub provider combinations were not run locally.
Related Issues
Related #1221: addresses the missing ZCode hook-stdout budget diagnostic. This does not claim to solve the separate Hermes plugin-section budget.
Notes for Reviewers
The rules reply is explicitly a lower bound before other session hints. A passing check means the rules alone fit, not that every runtime hint combination fits. TeamAI neither truncates rules nor raises the user's configured cap. The byte count matches the dispatcher's stdout write without an added newline. Bilingual reference and agent troubleshooting docs explain shortening rules or increasing the cap before starting a new ZCode session.
Follow-up verification (2026-10-25)
npm run build,npx tsc --noEmit, andnpm run lint: passed.project-hook-rules.test.ts: 52 passed, 1 skipped.instruction-targets.test.tsE2E: 43 passed, including the oversized ZCode reply and raised-cap recovery. No live ZCode application was available.Updated head:
a86be694a6d43cefb52c9116c0a7f8d3d001f656. Review base:a26f74d5bef51c498cc465b90599faa0130dc666.