fix: review batch — docs intent words, subagent watchdog, bin cache, workflow reader unification - #60
Merged
Conversation
…t.ts copy - README quick-start example now uses explicit capability intent words (在后台/子代理) so the default explicit router actually loads them - remove stale 'no project license' claim contradicting MIT - background terminal and workflow follow-up results now carry the 'already shown to the user, do not repeat' hint (subagents had it) - delete subagents/src/format.ts, a self-confessed copy of shared/context-utilization.ts; point 3 imports at the shared helper
The download destination resolved to a directory inside the installed npm package (node_modules/@tt-a1i/openpi/bin), which is wiped on every package update, forcing a fresh ~10MB download each time. The "repositoryBinDir" comment also wrongly claimed it resolved ~/.pi/agent, and the "bundled" resolution layer can never hit first in a published install - it is purely a download cache. Resolve binaries from the Pi agent's private managed bin directory (~/.pi/agent/bin, via getAgentDir(), the same pattern used by the setup and child-session extensions) so cached binaries survive package updates. Rename the misleading "bundled" source to "cached" and fix the comments to describe the actual semantics. SHA-256 pinning, resolution order, and all other layers are unchanged.
Direct subagents had no guard against a provider that accepts a request but never emits its first assistant event: the run stayed "running" forever and occupied one of the model pool's concurrency slots, while the workflow runner already settles such runs via its 45s first-response watchdog (the asymmetry flagged in the peer runtime audit). Arm a per-run watchdog in the manager, mirroring the workflow runner's semantics: it starts when a run becomes live (spawn or RunStarted) and clears on the first assistant event (AssistantDelta/AssistantMessage). On expiry the run settles as an explicit failure through the normal settle path — releasing its slot and letting the result flow back — and the stalled session is force-disposed like the abort-deadline path so it cannot revive into a zombie. The timeout defaults to FIRST_RESPONSE_TIMEOUT_MS (45s, matching the workflow runner) and is injectable via makeSubagentManagerLayer for tests. The stub backend gains a "HANG:" prompt mode standing in for a stalled provider request; manager.test.ts covers both the timeout-settles-and-frees-slot and first-response-clears cases.
… reader listRuns (/workflows non-TUI listing) and runDetailText parsed workflow.json raw while the dashboard and resolveRunDetails went through normalizePersistedWorkflowDetails + recoverStaleWorkflowDetails, so the same old-format run could show different statuses per surface. Add listPersistedRunIds() and readPersistedWorkflowDetails() beside the existing normalizer in dashboard.ts as the single persisted-run read entry (parse + normalize + optional result/transcript hydration), and make listRuns, runDetailText, resolveRunDetails, and loadRunEntries thin consumers of it. Dashboard output is unchanged; listRuns now reports old-format runs with the same normalized status as every other surface.
…ests Add a module-level test-only seam in index.ts — __setWorkflowTestAgentSessionFactory() — that runChild forwards to runAgent's existing sessionFactory option, so the replay/acceptance/ worktree glue in the production path is reachable from tests without spawning real provider sessions. Add execute.e2e.test.ts on the target-resolution.test.ts harness (temp agent dir, fake pi, real sandbox child process): a foreground agent-less run with artifact assertions and activeRuns cleanup, idle and busy background follow-up delivery (followUp+triggerTurn vs nextTurn), a failing script recorded as failed, and a full reviewer-agent run through the injected fake session whose resume replays the journal without touching the session factory.
…reads Follow-ups from review of the unified persisted-run reader: - runDetailText now applies recoverStaleWorkflowDetails, so /workflows <id> no longer disagrees with the list it follows (aborted vs running) - runDetailText and workflow_status's resolveRunDetails hydrate result/ transcript side artifacts, so cross-session status no longer surfaces the internal '[stored in result.json]' marker to the model
…match Review follow-ups on the explicit-intent router and watchdog: - CAPABILITY_INTENT now accepts the word forms the README advertises: background adds 启动 (在后台启动 dev server), delegate/workflow/search accept 用 as a verb prefix, delegate accepts 委派给, workflow accepts bare '用 Workflow' - the README TIP lists only phrases that are tested to match - new test pins the README quick-start example (loads background + delegate) and every advertised trigger phrase, so docs and regex cannot drift apart again - the test harness now replicates production hideLifecycleTools wiring (deferred tools disabled on session_start) - arm the first-response watchdog in send()'s restart window too: a backend that accepts the send but never emits RunStarted would hold the slot forever, the same hang class the watchdog exists to kill; regression test included (762 node:test + 30 vitest green)
6 tasks
tt-a1i
added a commit
that referenced
this pull request
Aug 22, 2026
…ls (#66) * feat(git-read): read-only git_show/git_diff/git_log as child-safe tools (closes #61) Reviewer/advisor subagents had no way to inspect git history: their (correct) bash-free tool boundary excluded diffs entirely, capping review quality at final-state reading (the review round on PR #60 hit exactly this). - new extensions/git-read: git_show, git_diff, git_log following the file-search pattern (structured schema, bounded preview, complete output persisted up to 10 MiB, process-group kill on cap) - argv construction is pure and fail-closed: revisions validated against a strict shape (no leading dash, no separators), paths are repo-relative only, and user values always sit behind a separator so they can never parse as flags - tools classified in CHILD_SAFE_PACKAGE_TOOL_NAMES; drift guard now enforces their classification - built-in roles explorer/reviewer/advisor/implementer gain the git tools (still read-only; allowlist only narrows) - search capability group now owns fileSearch + gitRead; intent regex accepts 用 git diff/log/show phrasing, with advertised-phrase tests - replay-safety allowlist extended: git tools are as side-effect-free as fd/rg, so reviewer-type workflow agents stay replay-journaled (e2e proved this gap: preflight failed before the allowlist fix) 772 node:test + 30 vitest green; drift guard passes. * fix(git-read): harden read-only execution contracts --------- Co-authored-by: tt-a1i <tt-a1i@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Batch of fixes from a multi-agent code review round (8 commits, all reviewed and follow-ups applied).
Documentation & model-facing prompts
CAPABILITY_INTENTpattern, so the flagship demo loaded zero capability groups. Regexes now accept the advertised word forms (后台启动, 用子代理, 用工作流, 用 fd 搜索, 委派给), and the README TIP only lists phrases that are tested to match.Runtime fixes
~/.pi/agent/bin: the old location lived inside the npm package directory, so every package update wiped the cache and re-downloaded ~10MB. The "bundled" source was renamed to "cached" to match reality.Workflows
readPersistedWorkflowDetails+listPersistedRunIdsare now the single normalization entry;listRunsno longer raw-castsPartial<WorkflowDetails>, so/workflowsand the dashboard report one status per run.runDetailTextapplies stale-run recovery (list and detail no longer disagree) and hydrates artifacts (no more[stored in result.json]marker leaked to the model).__setWorkflowTestAgentSessionFactoryseam (the runner already hadsessionFactory; the extension entry did not) plus 4 e2e cases covering foreground runs, background follow-up delivery (followUp+triggerTurnwhen idle,nextTurnwhen busy), script failure, and a full agent run with resume hitting the replay cache (proven by session-creation count staying at 1).Cleanup
subagents/src/format.ts, a self-confessed copy ofshared/context-utilization.ts.Test plan
bun run check(format/lint/typecheck) greenb1e36d6,fa00a19)