feat(gadgets): add Finish gadget for early agent termination (#83)#84
Merged
zbigniewsobiecki merged 1 commit intomainfrom Jan 18, 2026
Merged
feat(gadgets): add Finish gadget for early agent termination (#83)#84zbigniewsobiecki merged 1 commit intomainfrom
zbigniewsobiecki merged 1 commit intomainfrom
Conversation
Agents that complete work before maxIterations had no way to signal completion, causing them to loop saying "I have completed the task" until hitting the iteration limit. The Finish gadget allows agents to cleanly end their session early by throwing TaskCompletionSignal. Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Merged
6 tasks
zbigniewsobiecki
added a commit
that referenced
this pull request
Apr 27, 2026
…inish in detached HEAD (#1215) * fix(finish): use ls-remote SHA comparison so PR-checkout agents can finish in detached HEAD `hasUnpushedCommits()` was wedging every PR-checkout agent (`respond-to-review`, `respond-to-pr-comment`, `respond-to-ci`, `review`) when called from the `cascade-tools session finish` gadget. Workers check out PRs in detached HEAD via `refs/pull/N/head`, where `git rev-parse --abbrev-ref HEAD` returns the literal string "HEAD" and `@{upstream}` is unset. The legacy fallback computed `git rev-list origin/HEAD..HEAD --count` (commits not on the default branch), which always returns >0 for any feature branch — so finish always rejected with "Cannot finish session without pushing changes" even when the work was fully pushed. Live incident: ucho PR #84 stayed wedged for 22 m on 2026-04-27. The agent pushed `b10ff8a`, called finish three times, gave up, and emitted text but no further tool calls. Container stayed alive (lock held); subsequent reviews on the same PR all hit `Awaiting worker slot: in-memory same-type: 1 enqueued` until the watchdog backstop killed it at 22 m 44 s. Fix: thread the PR HEAD branch from `AgentInput.prBranch` into the finish gadget, then compare local HEAD to the remote tip via `git ls-remote origin refs/heads/<branch>`. Robust to detached HEAD (never reads the local branch name). Plumbing covers both engine paths: - In-process llmist gadget: `agentInput.prBranch` → `createConfiguredBuilder` → `initSessionState` → `SessionState.prBranch` → `validateFinish`. - CLI subprocess (codex / opencode and the in-container `cascade-tools session finish`): `agentInput.prBranch` → sidecarManager sets `CASCADE_PR_BRANCH` env → CLI reads env → `validateFinish`. Security hardening: `prBranch` flows from `payload.pull_request.head.ref` (GitHub-controlled). Git's ref-format rules permit `;`, `$`, `&`, `|`, `(`, `)`, backticks etc., so the branch name MUST NOT be shell-interpolated. Both the new ls-remote call and the legacy fallback's `origin/<branch>..HEAD` interpolation now go through `execFileSync` — branch is a single argv element, no `/bin/sh -c`, no metacharacter expansion. Operator visibility: added `logger.warn` on both fail-closed paths (ls-remote failure + rev-parse failure) with the underlying error so future incidents can distinguish network/auth issues from genuinely unpushed work without re-grepping logs. Tests: - `tests/unit/gadgets/session/core/finish.test.ts`: 8 new cases covering the ls-remote happy path, mismatch, missing remote branch, fail-closed semantics, the warn-log assertion, the detached-HEAD-trap negative assertion, and two command-injection regression pins (one for the new path, one for the legacy fallback) that assert the branch name lives in its own argv slot and never in any `execSync` shell-string call. - `tests/unit/gadgets/session/core/finish-real-git.test.ts` (NEW): real-git regression net — no mocks. Builds a bare remote + working repo, performs `git checkout --detach <sha>` to mirror the production `refs/pull/N/head` shape, and exercises the production code path end-to-end. Includes a documented assertion that the legacy path (no prBranch) IS still broken in detached HEAD — pinning the original bug so any future "simplification" back into the trap fails loudly. - `tests/unit/gadgets/sessionState.test.ts`: round-trip + clear-on-reinit coverage for the new `prBranch` field. Also extracts `checkPushedChangesHook` from `validateFinish` to keep the function under the project's complexity threshold. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(vitest): bump unit hookTimeout to 30s to fix beforeAll dynamic-import flakes Three PM manifest tests (jira, linear, trello) do `await import('.../index.js')` in `beforeAll` — ~2.3s when isolated but well over the default 10s under the parallel-fork CPU pressure of the full pre-push run. Caused intermittent red builds across the repo (not just this branch). Matches the integration project's existing 30s `hookTimeout`. `testTimeout` left at the 5s default — that limit is for per-test logic, not module-load. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
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
Merges dev to main with the Finish gadget implementation.
Changes
Finishgadget that allows agents to cleanly end their session earlyFinishwhen they've completed all tasks, which throwsTaskCompletionSignalfrom llmistmaxIterationsImplementation
src/gadgets/Finish.tsusing the standardGadget()factory patternFinishto base gadgets insrc/agents/base.tsTaskCompletionSignalfor clean termination🤖 Generated with Claude Code