feat: port remaining zcode-cli source modules and test tooling - #6
robotlearning123 wants to merge 64 commits into
Conversation
Port of the remaining public-safe modules and test tooling so this
repo is the single canonical source:
- packages/: the internal-only feature modules already exercised by the
ported test suite (daemon zagentd + daemon-request, IM bots
zagent-{feishu,telegram,wechat}, driver relay/rpc-frame/rpc-bridge/
controller-router/chat-turns/mentions/attachments/feishu/telegram,
TUI journey/fake-host/screen-replay harness) plus the driver
sub-package manifest and cp-fixture.zip needed by test-extract.
They stay out of the npm payload via the existing forbidden list in
scripts/verify-public-package.mjs.
- scripts/: discover-tests.mjs and offline-test-preload.mjs (already
imported by scripts/test-all.mjs, which could not run without them),
export-public-source.mjs and outsider-smoke.mjs (referenced by the
ported release-gate/journeys-index/commands/public-export tests).
- .github/public-workflows/test-matrix.yml template the exporter maps
onto .github/workflows/test-matrix.yml.
- .github/workflows/test-matrix.yml now also runs the pure-unit
node --test list cross-platform, as the xplat coverage ledger requires.
- plugins/zquota-panel marketplace sample + bin/zplugin-validate and
bin/zz-tui-weave tooling; test-user-flow.mjs live smoke.
- .gitignore, GEMINI.md, .github/copilot-instructions.md,
.claude/verify.sh agent-facing entry files (zcode-cli strings renamed
to zagent).
node scripts/test-all.mjs: 110/110 files pass (4 live-runtime skips).
node --test unit matrix (new CI step): 57/57 pass.
npm test (verify-public-package): PASS, 91-file payload unchanged.
…lity is the ledger's open ratchet
|
agent-review (none) at reviewno reviewer lane produced a verdict |
|
agent-review (none) at reviewno reviewer lane produced a verdict |
|
agent-review (none) at reviewno reviewer lane produced a verdict |
|
agent-review (none) at reviewno reviewer lane produced a verdict |
|
agent-review (devin) at reviewWarning: --sandbox always uses the autonomous permission mode; ignoring --permission-mode dangerous. Verification receipts
Findings
The blocker is the gate failure introduced by the last two commits — fix is either dropping the unit step from the VERDICT: REQUEST_CHANGES |
The public-workflows template is copied verbatim onto the exported tree's .github/workflows/test-matrix.yml, and the export ships no test files — the node --test step added in ec9d306 referenced 57 files that cannot exist there, breaking test-public-export.mjs and, downstream, the public repo's CI. The unit suite stays in this repo's own workflow; the template comments now say why the step is absent.
The driver lockfile still carried the pre-rename 0.0.160-rc.2 version and a floating ws range, so npm ci in that tree would flag it out of sync. Regenerated offline; ws is now the exact 8.21.3 the root manifest pins, and test-packaging asserts lockfile/manifest parity so a version bump cannot silently leave it behind.
The if-chain fell through silently and exited 0, so a typoed or empty invocation looked like success. Also drop the unused spawnSync import.
Predictable $TMPDIR/zagentd-<uid>.{sock,pid} names let a local user squat
the socket (sticky /tmp makes our own cleanup EPERM) or plant a pid file
so stop kills a recycled same-uid pid. Socket and pid now live in
$XDG_RUNTIME_DIR/zagent-<uid>, else a uid-keyed 0700 dir under os.tmpdir();
the dir's ownership and mode are verified on every call and a
squatter-owned or world-accessible one is refused. The compact clients
resolve the same path via the shared zagentd-paths module. stop (and the
already-running check in start) now require the recorded pid to actually
be a zagentd serve process — identified via /proc cmdline or ps — before
SIGTERM, and a planted foreign pid is removed without being signalled.
A non-object daemon response (null, array) fell through as a false success; reject it the same way zagentd compact already does.
The webhook answered the url_verification challenge before checking the verify token the handshake also carries, and a bot/app sender inside an allowed chat still drove agent turns. The challenge is now refused when a configured token does not match, and non-user senders are ignored before any message work.
resolveMentions claimed to rewrite each token to its resolved path but returned the input verbatim; found mentions now rewrite to their absolute path. Tokens resolving outside the workspace root (@/etc/passwd, @../x) are reported as rejected, and preprocessForBot refuses them before a turn is spent.
buf.length counts UTF-16 units, so ~4MB of multibyte input slipped under the 1MB cap; accumulate the Buffer chunk lengths instead.
…ta CLI The commands told the agent to run node bin/zagent-quota from the repo root, which only works inside a dev checkout; zagent quota is the installed entry point.
Internal tooling dirs and a non-ported spike path do not belong in the public repo's ignore list or comments.
|
agent-review (devin-sol) at reviewdevin-sol account devin rate- or quota-limited (try 1)
Verification: The focused mentions, Feishu, Telegram, daemon, export, release-gate, extraction, coverage and screen-replay tests passed. VERDICT: REQUEST_CHANGES |
Containment was checked only on the lexically resolved path, while statSync follows links — a symlink inside the workspace pointing outside it was accepted and rewritten into a bot prompt. The check now runs on the realpath (with the root realpathed too), dangling links pointing out are refused via readlink, and found paths report their canonical target.
The helper rmSync'd three fixed /tmp paths before and after use, so a stale or foreign directory at those names was deleted unowned, and the sandboxed HOME symlinked the real ~/.local — letting TUI side-effects escape the sandbox it claimed to provide. All dirs are now per-run mkdtemp targets and the runtime is pinned via ZCODE_RUNTIME instead.
zz-tui-weave copied quota.md to ~/.zcode/skills/quota without the extension, which the skill catalog does not recognize, and copyFileSync silently replaced any existing file. Destinations keep their .md name and an existing file is reported and preserved.
The documented `npm i -g @wechatbot/wechatbot` could never satisfy the bare ESM import, which only searches node_modules above the file. The loader now tries the bare import first (repo-local install) and then resolves `npm root -g` explicitly, so both install shapes work.
The loop advanced the long-poll offset past the whole batch before handling it, so a failed sendMessage acknowledged an update whose answer never arrived — and a redelivery would have re-run the turn's side effects anyway. The cursor now settles per update, a completed reply is cached and resent on redelivery without re-running the handler, and an undeliverable chat is dropped after maxDeliveryAttempts so it cannot stall the queue.
verify.sh claimed the skipped PTY journeys ran under a serial test-all.mjs in CI; no such lane exists (the workflow runs the package smoke plus a named unit list on ubuntu). The comments now say the serial gate is local. The unit step gains the hermetic ported tests it was missing — feishu, telegram, rpc-frame, rpc-bridge, zagentd, wechat-sdk, zz-tui-weave — and the four driver files move from UNTRIAGED to the matrix bucket in the xplat ledger (baseline 23 -> 19).
The public workflow template and the exporter's comments described a private source repository, its Actions billing, and withheld areas — none of which belong in the public repo. Comments now describe the mechanics without the source repo's name, cost model, or contents.
Neither scripts/tui-smoke.mjs nor docs/TUI-CORE-SPEC.md exists in this repo; name the real consumer (journey.mjs) and let the DECSTBM comment stand on its own.
The device/session id cache was written with default umask permissions — readable by every local user. The writer now creates the directory 0700 and the file 0600, and tightens a pre-existing loose file since the write mode only applies at creation. Added to the CI unit list.
slice(0, 150*1024) counts UTF-16 units, so a CJK reply could exceed the documented byte limit roughly threefold. The cap now truncates the UTF-8 encoding at 150KB, backing off to a code-point boundary so no partial sequence is emitted.
The snapshot guard locks its checkpoints dir with chattr +i on Linux and chflags uchg on macOS; the runner only cleared the Linux form, so teardown died EPERM mid-suite and took the remaining files with it. Cleanup now runs the matching unlock per platform.
Some tests cannot pass on a platform for a reason that is neither a
product bug nor a skippable assertion (the harness facility the file
drives does not exist there). PLATFORM_EXCLUDES beside NEEDS_RUNTIME/
NEEDS_LINUX_PTY carries {file, os, reason}; the runner honors it, and the
ledger asserts every entry names a real discovered file, a matrix OS, and
a concrete reason — never a second way to skip what NEEDS_* excludes.
/bin/true failed the macOS CI image's executable probe, and there was no win32 leg at all. process.execPath is a real executable on every host, so the forward-verbatim pair leg exercises the kernel's check portably.
|
Backlog-loop verification: FAILING (not pre-existing): macOS and Windows matrix jobs fail on the PR head (d71f82c). Failing files: packages/cli/test-signin-card.mjs and packages/cli/test-zagentd.mjs (macOS); packages/cli/test-cli-ux.mjs (Windows). test-signin-card.mjs and test-cli-ux.mjs are modified by this PR; test-zagentd.mjs is new in it. I did not reproduce them locally (Linux box; the ubuntu job and my local run pass), and since the files are new or modified here I cannot show the failures on base. Not proven pre-existing, so the conservative verdict is fail. Recommend needs_work: the head owner should fix the cross-platform failures (exit-code NaN in the signin-card spawn tests on macOS, test-zagentd on macOS, test-cli-ux on Windows). zcode-cli#578 depends on this PR. Independent review: FIX-FIRST, but reviewer-lane-unavailable. No independent grok review was obtained (a PONG probe hung twice, killed at 90s and 100s; fleet-quota-gate reported ok:true, so not a quota block). This is not a code verdict; FIX-FIRST stands in for REQUEST_CHANGES. Diff stat vs origin/master: 93 files, 4311 insertions, 216 deletions. I did not read the diff and ran no repo tests or lint. Head is a foreign branch, so this is review-only; no repo mutation was made. Next step: retry with another reviewer lane or re-probe grok. |
|
Session re-verification (by execution) at head d71f82c, from run 36276832359
Verdict: FIX-FIRST. The independent reviewer lane was unavailable this cycle, so this verdict rests on the CI evidence above, not on a code review. Review-only; branch not modified. |
resolveMentions reports and rewrites each token's canonical (real) path — the symlink-escape check needs real paths. On macOS the CI tmpdir sits behind the /var -> /private/var symlink, so expectations built with path.resolve compared a lexical spelling against a canonical one and four assertions failed. Derive them with realpathSync instead; identical on Linux, where the two spellings coincide.
The CLI children hash process.cwd(), and getcwd(3) returns the canonical path spelling — on macOS the runner's /var/folders tmpdir resolves to /private/var, so the 12 appends landed under a different workspace id than the one the test read back (1 !== 13). realpathSync the workspace once so both sides agree.
zagent-inspect hashes process.cwd(), which getcwd(3) canonicalizes — the macOS /var -> /private/var symlink made the spawned CLI's key differ from the test-planted hash dir, so inspect --json reported wiki: null. Derive the workspace cwd from realpathSync so planter and child agree.
displayPath deliberately retries on real paths when the lexical probe misses (symlinked homes), so with home=/ on macOS — where /etc is a symlink to /private/etc — it yields /private/etc/hosts. The expectation was hardcoded to the lexical /etc/hosts. Derive it from realpathSync so the assertion stays exact on every POSIX host.
findRuntime probes the per-OS desktop roots (/Applications/ZCode.app on macOS, %LOCALAPPDATA%\Programs on win32), but the fixture marked the Linux deb literal (DEFAULT_RUNTIME) as the existing desktop bundle — on macOS that literal is never probed, the local app-cli install won, and the preference order failed. Derive the fixture entries from desktopRuntimeEntries for the simulated env/home.
The --cwd value is a Windows path; embedded \ made the built RegExp interpret \t as TAB and \\U as U, so the verbatim-forwarding assertion could never match. Quote \\ alongside the other regex metacharacters.
BSD script(1) has neither -e nor -c, always exits 0, and on the CI runners the positional-command form captured no session output at all — every leg reported exit NaN with an empty screen. Drive the pty with /usr/bin/expect instead (ships with macOS): the payload rides in Tcl braces, each key byte goes out as a \xHH escape on the same send cadence, and the child's real exit status propagates via wait. The util-linux script -qfec leg is unchanged.
start reported "daemon started" right after spawn(), with the serve child's output discarded to /dev/null — a daemon that died at boot left a successful start, a missing socket, and no way to see why (the macOS CI failure was exactly this shape: ten silent seconds, then "socket never appeared"). The serve child's stdout/stderr now append to zagentd.log in the private runtime dir, start polls for the bind (lock released first, so the serve child can take it for socket setup), and a child that dies or never binds turns into a nonzero exit carrying the log tail. The socket assertion in the test reports that log when it fails.
sun_path is a fixed-size sockaddr field — 104 bytes on macOS, 108 on Linux — and the kernel SILENTLY TRUNCATES a longer bind instead of failing it (reproduced on Linux: listen() at a 130-char path reported success with the socket created at 108 chars). The macOS CI tmpdir geometry put zagentd.sock at ~119 bytes, so the daemon bound a truncated name no client computes: round 1's waitFor never saw the socket, round 2's start-side bind poll never saw it either while the child ran on, unkillable and un diagnosable until the harness timeout. daemonRuntimeDir now walks shorter private bases (XDG_RUNTIME_DIR, tmpdir, ~/.zagentd) in a fixed order and skips any base whose socket path would not fit the room — the same 0700/owner verification applies to the base that wins, and every caller runs the same deterministic walk on the same env. The serve child additionally refuses to keep a 'listening' socket that is not at its computed path, and start reaps the child (and the pid/sock records) when it gives up within 10s instead of hanging to the caller's timeout.
A deep XDG_RUNTIME_DIR whose zagentd.sock would overflow AF_UNIX sun_path (108 bytes here) must be skipped, not truncated into: assert the walk lands on the tmpdir base with 0700 intact, the skipped base is left untouched, and a real daemon starts, binds at the relocated path, and stops from there.
Round 2 on macOS leaked the pasted secret and left 'orld' on screen: the card paints seconds after spawn on the slow runners, and fixed-delay sends landed in the cooked line discipline, which echoes whatever it receives — before readline's raw mode engages. The expect driver now waits for 'choose a sign-in path' before the first key and for 'paste ZAI_API_KEY:' before the secret, so sends only ever strike a pending question. Validated against a late-painting mock under expect 5.45: the ungated round-2 shape echoes the secret, the gated one does not.
On machines where every caller-controlled root is too long for AF_UNIX sun_path (the macOS CI geometry overflowed XDG, tmpdir and ~/.zagentd alike at 104 bytes), the runtime-dir walk ran out of candidates and threw — and because zagentd.mjs and both compact CLIs resolved paths at module scope, a mere import crashed: zagentd ask died before it could say anything. The walk now always has somewhere to land: /tmp/zagentd-<uid> is the final candidate, short by construction on any POSIX host (<=36 bytes for the socket), created 0700 and owner-verified on exactly the same terms as the other bases — /tmp's sticky bit gives it the protection the shared-tmpdir fallback always relied on. And path resolution is deferred to first use in every CLI, so an unusable environment fails the one command that needs the socket with the resolver's error naming what it tried, instead of crashing every importer.
The XDG-preference legs ran against a base nested in the sandbox tmpdir, which overflows sun_path on macOS — an over-long base is (correctly) skipped for a shorter one, so XDG could never be 'preferred' there. Give the preference, tighten and squatter fixtures a short mkdtemp under /tmp, and clean it up; the long-XDG relocation leg keeps proving the skip.
Two macOS leaks, one mechanism each: - The first send was gated on the card banner, which paints before readline binds the question — a key sent in that gap lands in the cooked line discipline and echoes. Gate on the question prompt itself. - 'Esc eats the burst' sent 'orld' 700ms after the Esc+w cancel, striking a tty the (correct) cancel had already restored to cooked+echo: expect keeps the pty master open after the child exits, so the driver itself echoed the send back — a harness leak no correct card can avoid. The burst is now ONE send, 'Esc world Enter' in a single read — the faithful fast-input hazard, delivered while the card is provably alive, consumed by the closed interface. Verified against the real card under expect 5.45: one-burst stays clean; the split shape leaks.
The offline test preload re-roots every mkdtemp prefix under TMPDIR, so
mkdtempSync('/tmp/zagentd-xdg-') landed back in the long sandbox path and
overflowed the macOS sun_path room, making the XDG-preference leg fall
through to the tmpdir base. Create the base with mkdirSync under /tmp and
a random suffix; it is still removed in the finally block.
… legs The fallback and relocation assertions expected the sandbox tmpdir to win, but on macOS that base is long enough to overflow the sun_path room and is correctly skipped. Give those legs a short /tmp base (created and removed by the test) so they check the preference order rather than the host's tmpdir length; the over-long XDG base stays under the sandbox.
…at/port-zcode-cli-remainder-20260925 # Conflicts: # packages/driver/test-doctor.mjs
The concurrent-start leg scoped pgrep hits to this test's daemons by reading /proc/<pid>/environ, which macOS does not have, so every pid was filtered out and the single-daemon assertion saw zero. Read the environment through BSD ps -E off Linux.
Spawn the CLI through fileURLToPath rather than URL.pathname (which yields /D:/... on Windows) and skip the POSIX 0600 mode assertion on win32, as the other credential suites do.
|
agent-review (grok) at review我先读 PR 元数据、完整 diff 和仓库检查命令，再按文件核对正确性、安全和测试。HEAD 和检查命令先对齐，再按安全敏感模块逐文件读 diff。diff 大约六千行。我先扫安全敏感实现，同时跑仓库自带的 Findings1. major —
|
1 participant
Summary
Ports the remaining feature modules into zagent so this repo is the single canonical source. This is a squash port — module files copied verbatim unless noted under "Sanitized" below; development history is not imported.
Ported
zagentd/daemon-request(daemon),zagent-{feishu,telegram,wechat}+ driverfeishu/telegram/relay/rpc-frame/rpc-bridge/controller-router/chat-turns/mentions/attachments(IM/remote surfaces), TUIjourney/journey-entry/fake-host/screen-replay(hermetic PTY harness),driver/test.mjs,driver/test-release-gate.mjs,driver/fixtures/cp-fixture.zip(needed by publictest-extract.mjs),driver/package.json/package-lock.json(sub-package manifest,@zagent/driver). These modules stay out of the npm payload:forbiddeninscripts/verify-public-package.mjsalready names them.discover-tests.mjs+offline-test-preload.mjs(imported byscripts/test-all.mjs, which was un-runnable without them),export-public-source.mjs+outsider-smoke.mjs(read/spawned by the portedtest-commands,test-release-gate,test-public-export,test-journeys-index)..github/public-workflows/test-matrix.yml— template the exporter maps onto the real workflow..github/workflows/test-matrix.yml— adds the pure-unitnode --testsuite, whichtest-xplat-coverage.mjs's bucket ledger requires.bin/zplugin-validate,bin/zz-tui-weave— the plugin sample + its validator.test-user-flow.mjs— manual live-runtime smoke (ungated helper)..claude/verify.sh,.gitignore,GEMINI.md,.github/copilot-instructions.md— dev/agent entry files.Sanitized during port
Renames to
zagentinplugins/marketplace.json,plugins/zquota-panel/commands/*.md,packages/driver/package*.json, plus two comment strings (zagentd.mjs,controller-router.mjs)..gitignore/GEMINI.md/copilot-instructions.mdtrimmed of non-public paths.Not ported
Non-distribution trees: local receipts and benches, research notes, scratch dirs, internal docs, and release tooling that carries internal paths. The npm payload is unchanged —
verify-public-package.mjsenforces the allowlist.Cross-platform fixes
The test matrix now runs the whole discovered suite on ubuntu, macOS and Windows. Getting it green there found:
sun_pathis 104 bytes on macOS and 108 on Linux, and the kernel silently truncates a longer bind. The daemon then listened on a name no client could compute. The runtime dir now walks XDG, then tmpdir, then~/.zagentd, then/tmp/zagentd-<uid>, skipping any base whose socket would not fit. Every base is 0700 and owner-verified. Paths resolve lazily, so importing the module never throws.startfails within 10s with the log tail instead of hanging, and a concurrentstartis serialized behind a lock./varvs/private/varrealpaths; BSDscript(1)replaced byexpect(1), with sends gated on the card's output; Windows backslash escaping andfileURLToPathinstead ofURL.pathname; POSIX mode checks skipped on win32; process environment read viaps -Ewhere there is no/proc; short/tmpbases for the zagentd preference legs, because the offline preload re-rootsmkdtempunder the long sandbox tmpdir.Test plan
npm test(scripts/verify-public-package.mjs): PASS; payload unchanged, no ported file shipsnode scripts/test-all.mjsafter merging master (fix(cli): seed personal provider config so -p --model works on a fresh host #7): 117/117 test files pass on Linux (4 live-runtime tests skipped by design)da53314