chore(release): prepare maintained v0.29.0.3 - #14
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces several significant changes, including the implementation of durable upgrade delegation for the runner, support for session-attached jobs, and native history rewind capabilities for Claude sessions. The code review highlights a critical concern regarding the reliability of process tree termination on Windows when using the new upgrade delegation mechanism, suggesting that the current approach might leave orphaned runner processes and lead to resource contention. I have kept this comment as it identifies a potential critical bug.
| const stopped = await killChild(options.child, true).catch(() => false) | ||
| let exitTimer: ReturnType<typeof setTimeout> | undefined | ||
| try { | ||
| await Promise.race([ | ||
| exitPromise.catch(() => undefined), | ||
| new Promise<void>((resolve) => { | ||
| exitTimer = setTimeout(resolve, 3_000) | ||
| }), | ||
| ]) | ||
| } finally { | ||
| if (exitTimer) clearTimeout(exitTimer) | ||
| } | ||
| return { ready: false, safeToFallback: stopped } |
There was a problem hiding this comment.
On Windows, reliably killing a process tree, especially one spawned via a .cmd shim, is non-trivial. The detached: useProcessGroup option is not effective on Windows, so there's no guarantee that killChild (presumably killProcessByChildProcess) can terminate the entire process tree started by the upgrade delegate.
If killChild only terminates a wrapper process (like cmd.exe) but leaves the actual runner process orphaned, stopped could become true while the runner is still alive. This would lead to safeToFallback: true, causing the old CLI to start and resulting in multiple runner instances. This could lead to resource contention and unpredictable behavior.
Consider using a more robust method for process tree termination on Windows, such as spawning with taskkill /t /f on the child process's PID.
There was a problem hiding this comment.
This path already uses the proposed Windows mechanism. settleDurableDelegate defaults killChild to killProcessByChildProcess; on Windows that delegates to killProcess, whose killProcessWindows invokes taskkill /T /PID <pid> and prepends /F for this timeout call. It returns false on spawn failure or a nonzero result while the PID remains alive, so safeToFallback remains false in that case. See cli/src/utils/process.ts:72-100 and :206-221. No code change is needed for this finding.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces several major features and robustness improvements across the HAPI CLI and agent runtimes. Key additions include the implementation of a new job command and the session_job MCP tool to manage session-attached, long-running background tasks. It adds native conversation rewind and truncation support for Claude Code (v2.1.223+), along with robust local Claude session summary listing and transcript paging. For Codex and Copilot/Kimi, it introduces support for reasoning effort configuration, context management settings propagation, and live transcript usage scanning. It also hardens the agy hook carrier lifecycle by moving carriers under HAPI_HOME/agy-carriers and implementing boot-scoped, namespace-aware liveness sweeps to prevent over-deletion. Additionally, the PR implements RPC request cancellation via AbortController, structural model error classification for Cursor, and durable upgrade-target delegation with robust readiness checks. There are no review comments to provide feedback on.
Summary
tiann/hapi@980a921ba15665c54998a6ddb658103d467ff4cb0.29.0.3and record the exact replay manifestValidation
bun typecheck: passed for every packagebun run test: 8,093 passed, 16 skipped, 0 failedbun run test:cli:integration: 13 passed, 1 skippedpython3 -m unittest discover -s tools/maintenance/tests -p "test_*.py": 42 passed46f87665c718a05de7faccb1ebc67ccdfb3b2555Do not tag until the latest exact head has clean GitHub review and CI.