Repository navigation
New worktree on the phone's new-chat screen (Remote rev 50, chat:new-worktree) - #648
sambitcreate wants to merge 29 commits into
Conversation
An empty Workspace chat on iPhone or Android can start in its own
worktree. The phone posts the first message's text to the new
POST /chats/{chatId}/worktree (Remote revision 48, chat:new-worktree,
with git:write); the Mac makes the worktree through the desktop's
first-message coordinator with a draft's default base, names the branch
from the message, and moves the still-empty chat into it. The phone
then sends the turn as usual.
Phones show a New worktree chip beside the composer, then a "Setting up
worktree..." row with Cancel. Cancel puts the message back. Refusals are
fixed sentences without paths or Git output.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Three correctness issues need fixes before merging: refused retries remain stuck, Android restores the old workspace, and setup can overwrite a newer workspace choice.
Reviewed changes — Reviewed the complete PR across the Remote host, desktop worktree coordinator, and both native phone clients.
- Phone send flow: Adds worktree eligibility, the composer toggle, setup/cancellation state, and worktree creation before the first turn.
- Host integration: Adds the capability-gated worktree route, default-base selection, per-chat reuse, sanitized refusals, and the shared desktop coordinator.
- Contracts and delivery: Updates capability negotiation, native decoders, OpenAPI and fixtures, test registration, and documentation.
Validation: 280 Remote tests, four new Android tests and Android compilation, 27 inventory checks, and 29 desktop draft/shared tests passed. Isolated probes reproduced the three findings. Swift syntax passed; XCTest was unavailable on Linux. One interrupted-worktree recovery test fails with Branch “aiden/orphan” already exists. on both this head and the unchanged base.
gpt-6.1-sol ｜ 𝕏
- Phones mint a new Idempotency-Key once the Mac has answered (a refusal is stored under its key), and keep it only when no answer came back. - An empty chat already in a worktree answers with that worktree, so a retry after Cancel uses the worktree the Mac finished. - The workspace move rechecks the chat's revision, so a move made elsewhere while the worktree was being made stands. - A first message over 4,000 characters names the branch from its first 4,000 code points instead of failing on the phone. - Android keeps the new workspace when it accepts the turn. - With the chip on but no longer offered, the phone turns it off and keeps the message instead of sending to the main checkout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
The check read the oldest request the mock server held, which was an earlier refusal from the same test, not the long message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
Important
The new late-finish recovery path can accept a stale request for an unrelated worktree, allowing the phone to send in the wrong checkout.
Reviewed changes
Reviewed the changes since cdce0b76, including the Android test correction that landed during this review.
- Corrected retry handling: Cleared worktree attempt keys after definitive refusals on both phones and added host recovery for chats already in worktrees.
- Preserved workspace state: Added the final move-time revision check and carried Android's moved workspace through turn receipt acceptance.
- Handled send eligibility and long messages: Preserved drafts when New worktree becomes unavailable and limited branch-name seeds to 4,000 Unicode code points.
- Extended coverage and docs: Added retry, late-finish, concurrent-move and Unicode cases, isolated Android's long-message test, and documented the recovery rules.
Validation: 191 focused Remote tests, all six Android worktree tests, Android compilation, and 12 iOS shipping-inventory checks passed. Changed Swift files passed syntax parsing; native XCTest was unavailable on Linux. An isolated real-service probe reproduced the inline finding on the latest head.
gpt-6.1-sol ｜ 𝕏
…tree The stale-revision shortcut now needs the coordinator's ready record for this chat to name the workspace the chat is in. A chat moved into any other managed worktree gets the usual revision check, so the phone never sends into a checkout it did not ask for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
…cription Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
Important
The new ready-only check blocks recovery of a successfully created worktree when its final ledger write failed and the host restarted.
Reviewed changes
Reviewed the changes since c2877a42, focusing on the ownership restriction and its durable-recovery behavior.
- Restricted late-finish recovery: Required the chat's coordinator record to identify its current worktree before bypassing revision checks, and wired the record lookup into the host service.
- Extended ownership coverage: Added checks for unrelated worktrees, another chat's setup, pending records, and mismatched workspace IDs.
- Corrected documentation: Narrowed the documented revision exception and clarified OpenAPI retry semantics.
Validation: 191 focused Remote tests and two coordinator adoption tests passed. A fault-injection probe through the real file ledger, coordinator, and Remote chat service reproduced the inline recovery failure. Hosted CI was still in progress; native clients have no new production changes in this delta.
gpt-6.1-sol ｜ 𝕏
…ailed The late-finish check now matches the coordinator's record against the saved workspace's branch and ownership token, like the coordinator's own adoption, so a pending record whose workspace was saved still counts and a failed one or a token mismatch does not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the changes since db06d599, focused on recovering a saved worktree after its final ledger write failed.
- Restored pending-record recovery: Allowed recovery when the chat's record names its current workspace and the saved branch and ownership token match, while continuing to reject failed records and unrelated worktrees.
- Extended behavioral coverage: Added saved-pending recovery and ownership-mismatch cases alongside the existing per-chat isolation checks.
Validation: all 191 focused Remote router, chat and worktree tests passed, plus both focused coordinator recovery/isolation tests. No native production code changed in this delta; native suites were not rerun. Hosted CI is still pending on this exact head.
gpt-6.1-sol ｜ 𝕏
README keeps both workspace sentences; ios/CHANGELOG.md keeps both Unreleased entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
|
Desktop tests (renderer-other) failed on 06d8c76 in one test: the voice assistant's Generated by Claude Code |
Hermes Review BotConfidence: 4 Engine: SummaryThe PR adds a "New worktree" first-send path for phones: a new opt-in capability Two things a maintainer should double-check: the Android Confidence Score: 4/5Traced the route → service → coordinator → both clients → tests end-to-end and verified the documented retry/refusal semantics against 📁 Important Files Changed
FindingsNo findings. 📊 Sequence DiagramsequenceDiagram
participant Phone as iPhone / Android chat model
participant Router as Remote router
participant Chats as AidenRemoteChatService
participant WTs as aiden-remote-new-worktree
participant Coord as firstMessageWorktrees (shared)
Phone->>Router: POST /chats/{id}/worktree (If-Match, Idempotency-Key, {text})
Router->>Router: authenticate chat:write, require git:write + chat:new-worktree
Router->>Chats: moveToNewWorktree(device, chat, revision, key, body)
Chats->>Chats: classify; empty? already its worktree? else requireRevision
Chats->>WTs: create(device, sourceWorkspaceId, text, chatId)
WTs->>Coord: ensure("phone-chat:{chatId}", seed=text)
Coord-->>WTs: {workspaceId, branch} (reused on retry)
Chats->>Chats: moveEmptyToWorkspace(assertCurrent: still empty + revision)
Chats-->>Router: {chat, branch}
Router-->>Phone: 200 {chat, branch}
Phone->>Phone: patch chat.workspaceId + revision, chip off, draft cleared
Phone->>Phone: POST /chats/{id}/turns in the worktree's workspace
Machine-Readable Findings[]
|
|
The red "Hermes Review Bot" status on 06d8c76 means the review engine itself failed to run ("agy failed (3)"). It says nothing about this change. It runs on the self-hosted review machine, so I can't re-run it from here. Pullfrog reviews this PR, and GitHub CI is the merge gate. Generated by Claude Code |
package.json test:aiden-remote keeps both sides' test files (aiden-remote-lan-hostname and aiden-remote-new-worktree). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
very-hermes-bot
left a comment
very-hermes-bot
left a comment
There was a problem hiding this comment.
Inline review — 2 finding(s) anchored to the diff. See the pinned summary comment for the overview.
The Mac sends every Git projection under `result`, tagged by `kind`, but Android looked for top-level review/diff/branches fields, so the branch list never arrived and New worktree stayed on for a folder with no commits. AidenGitResultWire moves each read to its typed field and leaves a mutation under `result`; every client Git decode uses it. The test feeds the Mac's branches body through gitBranches and the shared review example through the reader. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
Drives the chat screen's model on iPhone and Android against a Mac that offers New worktree: the worktree is made first and the turn follows in the worktree's workspace (on screen and in the saved chat); a refusal shows the Mac's sentence and puts the message back; Cancel puts it back unsent, a refused request's retry gets a new key and a canceled one keeps its key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the Android Git-decoding changes in dbaea476 since the last completed incremental pass at 71b6d752; the intervening merge-only changes since the last formal review were already checked in earlier passes.
- Corrected Git reply decoding: Mapped the Mac's nested
resultdiscriminator into Android's typed read fields while preserving mutation replies, and applied the reader to all 13 Git client decodes. - Extended behavioral coverage: Exercised repository and non-repository branch replies through the HTTP client and decoded the shared Git review fixture, verifying the data used by the New worktree pre-check.
Validation: Android app/test Kotlin compilation and all 20 focused worktree/workspace tests passed. All 194 focused Remote router, chat, worktree and Git service tests passed. No device/emulator UI testing was performed; iOS production code is unchanged in this delta. Completed hosted CI on this exact head has not been established.
gpt-6.1-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the changes since dbaea476, covering the new native send-flow tests and the subsequent merge of main.
- Added first-send coverage: Exercised both real phone chat models through worktree creation followed by turn submission, checking request order, moved workspace persistence, and cleared composer state.
- Covered refusal and cancellation: Verified the Mac's refusal message, restored unsent text, no turn submission after Cancel, and fresh versus retained idempotency keys across retries.
- Reconciled the main merge: Preserved the worktree documentation and test registration alongside main's No workspace documentation and remote chat image suite.
Validation: Android app/test Kotlin compilation and all nine focused worktree tests passed, as did 192 focused Remote tests and 27 shipping/CI inventory checks. The changed Swift tests passed syntax parsing; XCTest execution requires macOS and was not run locally. No device/emulator UI testing was performed, and hosted CI is still pending on this head.
gpt-6.1-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the delta since dbaea476, focused on the new native send-flow tests and integration with the latest main merge.
- Added successful-send coverage: Exercised both real phone chat models through worktree setup followed by turn submission, checking request ordering and the moved workspace in live and cached chat state.
- Added refusal and cancellation coverage: Verified draft restoration, the Mac's refusal message, no turn submission after cancellation, and fresh-versus-retained idempotency keys across retries.
- Merged the current base: Checked the incremental merge context and reran focused host contracts and test-inventory checks.
Validation: all nine Android worktree tests and Android Kotlin compilation passed; all 282 focused TypeScript tests passed, along with iOS release/inventory and CI policy checks. Changed Swift tests passed syntax parsing; XCTest and simulator UI testing were unavailable on Linux. Hosted CI is still incomplete on this exact head.
gpt-6.1-sol ｜ 𝕏
|
Flake in Desktop tests (runtime-subagents) on 9656855: Generated by Claude Code |
very-hermes-bot
left a comment
very-hermes-bot
left a comment
There was a problem hiding this comment.
Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.
The chip was offered for an attachments-only first message, but both phones refused the empty branch seed themselves with "invalid response" and kept the chip on, so every retry failed the same way. The phone now sends "New chat", which the Mac names aiden/new-chat, as the desktop does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed 25e09d2e since the prior Pullfrog review at 96568559, focused on attachment-only first messages with New worktree enabled.
- Fixed attachment-only setup: Both phone clients now send
New chatas the branch-name seed instead of refusing empty text locally, while preserving the actual turn's text and attachments. - Updated coverage and documentation: Adjusted Android's HTTP-client test, added iOS fallback-seed coverage, removed obsolete blank-text refusal expectations, and documented the behavior in the API guide, remote-access guide and iOS changelog.
Validation: Android app/test Kotlin compilation and all nine focused worktree tests passed. All 247 focused Remote and shared branch-naming tests passed, as did 12 iOS shipping-inventory checks. The changed Swift files passed syntax parsing; XCTest and device/simulator UI testing were not run on Linux. Hosted CI remains incomplete on this exact head.
gpt-6.1-sol ｜ 𝕏
very-hermes-bot
left a comment
very-hermes-bot
left a comment
There was a problem hiding this comment.
Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.
Both phones now cover the send guard: the chip is on, another device moves the empty chat into a worktree, and the send keeps the message, turns the chip off, shows the no-longer-offered sentence and posts nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
very-hermes-bot
left a comment
very-hermes-bot
left a comment
There was a problem hiding this comment.
Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.
…rives On Android, cancelling a setup whose answer had already arrived did nothing, so the turn was still sent. A flag set by Cancel is now read after the wait: the chat moves into its worktree and the message stays unsent, as on iOS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed cd5ed70a since the completed documentation-only pass at 4b01b079, with the intervening documentation correction included since the last formal Pullfrog review at 86cdd3ad.
- Added send-guard coverage: Exercised both real phone chat models after another device moves an empty chat into a managed worktree while New worktree remains selected, verifying the explanatory error, preserved draft, disabled chip, and no outgoing chat mutation.
- Corrected replay documentation: Clarified that successful requests replay their saved response, whereas settled refusals replay as
409 internal_error; this documentation-only change was already checked in the preceding pass.
Validation: Android app/test Kotlin compilation and all 10 focused worktree tests passed. Swift test syntax parsing and all 12 iOS shipping-inventory checks passed. An initial overlapping local build failed with Unable to delete directory for generated resources; a clean serial rebuild resolved the contention. Native XCTest and device/emulator UI testing were not run on Linux, and hosted CI remains incomplete on this exact head.
gpt-6.1-sol ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed bb8b6e5b since the prior Pullfrog review at cd5ed70a, focused on Android cancellation as worktree setup completes.
- Recorded cancellation intent: Added an attempt-scoped flag so Cancel remains effective after the setup job has completed but before the send coroutine resumes.
- Preserved completed setup state: Kept the chat in its newly created worktree while restoring the unsent message, clearing setup state, and preventing turn submission when cancellation wins that race.
Validation: Android app/test Kotlin compilation and all 10 existing worktree tests passed. An isolated deterministic test through the real ViewModel also passed: it invoked Cancel after the setup Deferred completed but before the awaiting send resumed, then verified the restored draft, moved workspace, cleared chip, no error, and no turn POST. No iOS production code changed; XCTest and device/emulator UI testing were not rerun. Hosted CI is not green: Desktop tests (core-git) failed with Process completed with exit code 1., and other checks remain pending; this review does not establish merge readiness.
gpt-6.1-sol ｜ 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
very-hermes-bot
left a comment
very-hermes-bot
left a comment
There was a problem hiding this comment.
Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.
Git lists the branch an empty repository will create, so the phones saw a non-empty branch list and offered New worktree anyway. The Mac's branch list now carries unborn: true for such a repository, and both phones hide the chip when it is set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed 9e112819 since the completed documentation-only pass at 713e888d, focusing on New worktree availability for repositories with no commits.
- Exposed unborn repository state: Added optional
unborn: trueto Remote branch replies while preserving the prospective branch name and omitting the marker for committed repositories. - Corrected both phone pre-checks: Decoded the marker on iOS and Android and hid New worktree when it is present, rather than treating a nonempty branch list as proof of commits.
- Extended contracts and coverage: Updated OpenAPI and the API guide, added a real fresh-repository host test, and extended both native branch-decoding tests.
Validation: all 59 focused host tests and 23 Android worktree/workspace tests passed, along with Android compilation, the TypeScript check, Swift syntax parsing and 12 iOS shipping-inventory checks. A direct schema probe accepted legacy and unborn branch replies and rejected an invalid marker. Native XCTest and device/emulator UI testing were not run on Linux; hosted CI is still incomplete on this exact head.
gpt-6.1-sol ｜ 𝕏
very-hermes-bot
left a comment
very-hermes-bot
left a comment
There was a problem hiding this comment.
Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.
…ion conflict Appending a message changes the chat's revision, so checking the revision first answered revision_conflict for a chat that had gained its first message elsewhere. Messages are now checked first, matching the move-time check and the documented operation_stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed 17981c5e since the prior Pullfrog review at 9e112819, focused on New worktree refusals after another device sends the first message.
- Corrected refusal precedence: Checked for messages before validating the initial revision, so a nonempty chat returns the documented
409 operation_stalewhile empty-chat revision checks remain intact. - Extended behavioral coverage: Exercised both the revision held while the chat was empty and its current revision, asserting the same refusal and no additional worktree creation.
Validation: all 274 focused Remote chat, worktree, router, protocol and operation-contract tests passed, alongside all 10 Android worktree tests and 12 iOS shipping-inventory checks. Both native refusal paths were inspected; native XCTest and device/emulator UI testing were not run on Linux. Hosted CI remains incomplete on this exact head, so this review does not establish merge readiness.
gpt-6.1-sol ｜ 𝕏
very-hermes-bot
left a comment
very-hermes-bot
left a comment
There was a problem hiding this comment.
Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.
…er arrives A task that has already finished ignores cancel(), so a Cancel pressed as the Mac's answer landed could go unseen and the message was sent. Cancel now also sets a flag that the send reads after the wait, as on Android. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed 80c59733 since the prior Pullfrog review at 17981c5e, focused on iOS cancellation as worktree setup completes.
- Recorded cancellation intent: Added a setup-scoped flag so Cancel remains effective after the setup task finishes but before the awaiting send resumes, with the flag reset for each new attempt.
- Preserved the unsent message: Checked that the success path retains the moved workspace and then uses the existing cancellation cleanup to restore the draft and attachments without submitting a turn.
Validation: Swift syntax parsing passed for the changed model and its existing chat tests; all 12 iOS shipping-inventory checks passed. Native XCTest and simulator testing were unavailable on Linux. Existing tests cover cancellation during the wait, not the exact completion/resumption timing window; full application CI completion on this head has not been established.
gpt-6.1-sol ｜ 𝕏
Background jobs on phones took revision 49 on main, so New worktree on phones claims 50 across the server, OpenAPI, fixture, iOS, Android and docs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
sambitcreate
changed the title
…iation The merge left two negotiatedServer declarations, so Android didn't compile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
✅ No new issues found on the updated head.
Reviewed changes
Reviewed the changes since 80c59733, including the Android repair pushed during this review.
- Advanced the Remote revision: Moved
chat:new-worktreeto revision 50 after main's background-jobs revision 49, aligning the host, fixtures, iOS expectation, OpenAPI and documentation. - Integrated background jobs: Preserved both capabilities, their native composer controls and negotiation paths, and the combined shipping/test registration.
- Repaired Android negotiation: Removed the duplicate local declaration and chained the jobs result into worktree negotiation, preserving earlier grants when the final opt-in is skipped or refused.
- Removed an accidental artifact: Deleted the committed
node_modulessymlink.
Validation: Android app/test Kotlin compilation and all 15 focused worktree/background-jobs tests passed on 5c554b0, alongside 287 focused Remote tests. TypeScript, Swift syntax parsing, portable iOS release checks and CI registry/policy checks passed. The desktop worktree suite passed 51/52 tests, reproducing the previously recorded aiden/orphan recovery failure that the initial review also reproduced on the unchanged base. Native XCTest and device/emulator UI testing were not run on Linux. Hosted CI remains incomplete on this exact head, so this review does not establish merge readiness.
gpt-6.1-sol ｜ 𝕏
…0 ms On a busy macOS runner the pause's durable write took longer than the fixed 50 ms, so the card check finished before the close began and the Bot's next model call ran before the pause stopped it. The check now finishes when the close signals the tool, so the test always exercises a pause that closes the session while the tool still works on the card. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
…-euhzg4-phone-worktree
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the delta since 5c554b0, focused on the Bot pause test repair and the subsequent merge of main.
- Replaced timing-based synchronization: Released the held connection-readiness check when the tool receives cancellation, preserving the real pause, subsequent-send and shutdown checks without depending on a 50 ms delay.
- Integrated the current base: Checked the complete incremental diff and verified the merged Remote worktree and background-jobs contracts.
Validation: all 17 Bot pause tests, all 287 focused Remote tests, and TypeScript checking passed. The initial Bot suite invocation failed because the native lock helper was missing (aiden-lease-flock ENOENT); building that prerequisite resolved it. No native code changed in this delta, so mobile suites were not rerun. Hosted CI remains incomplete on this exact head; this review does not establish merge readiness.
gpt-6.1-sol ｜ 𝕏
very-hermes-bot
left a comment
very-hermes-bot
left a comment
There was a problem hiding this comment.
Inline review — 1 finding(s) anchored to the diff. See the pinned summary comment for the overview.
| worktreeAttempt = nil | ||
| Task { await coordinator.refreshWorkspaces() } | ||
| // Canceled as the worktree arrived: the chat stays there, unsent. | ||
| if setup.isCancelled || worktreeSetupCanceled { throw CancellationError() } |
There was a problem hiding this comment.
[P2 · logic] Cancel during worktree setup drops the message the person already sent if they type while waiting
Confidence: 0.64
The new worktree wait leaves the composer editable, and on Cancel the shared cancellation catch only restores the submitted text/attachments when the draft is empty, so text typed during the wait silently replaces the message the person sent; the Android sibling and iOS's own error path merge via AidenDraftSendReconciliation instead.
Repair: Use AidenDraftSendReconciliation.failedDraft(submitted:current:) and failedAttachments(submitted:current:) in the iOS cancellation catch, matching the error path and Android.
There was a problem hiding this comment.
Fixed in 64e5e45. The iOS cancellation catch now uses AidenDraftSendReconciliation.failedDraft and failedAttachments, the same as the error path and Android. In AidenChatTests, the second canceled wait now types into the composer and expects "Fix the header\n\nAnd the footer".
Generated by Claude Code
… iOS A New worktree wait can last as long as a fetch, and the composer stays editable. Cancel now merges the returned message with anything typed meanwhile, the way a failed send and Android already do, instead of dropping the message when the composer wasn't empty. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed 64e5e457 since the prior Pullfrog review at caed23b, focused on iOS draft restoration when worktree setup is canceled.
- Preserved newer composer content: Reused the existing failed-send helpers to restore the canceled message ahead of text typed during setup and merge attachments without duplicate IDs.
- Extended cancellation coverage: Updated the real chat-model test to type another draft during a held worktree request and verify both messages remain, with no error or turn submission.
Validation: changed Swift files passed syntax parsing, and all 12 iOS shipping-inventory checks passed. Native XCTest and simulator testing require macOS and were not run on this Linux runner. Android already uses the corresponding restoration helpers and is unchanged in this delta. Hosted CI remains incomplete on this exact head, so this review does not establish merge readiness.
gpt-6.1-sol ｜ 𝕏
…-euhzg4-phone-worktree
Requested by sbt · project thread
What this does
An empty Workspace chat on iPhone or Android can now start in its own worktree, the same as the desktop's New worktree toggle on first send.
POST /chats/{chatId}/worktree(Remote contract revision 50).chat:new-worktree, which also needschat:writeandgit:write.resolveWorktreeDefaults, now shared).{chat, branch}.Details
operation_stale);workspace_unavailable);git_capability_denied);revision_conflict).idempotency_in_flight).git:readread the workspace's branches first, and hide the chip for a folder with none (not a repository, or no commits).main/services/first-message-worktree-main.tsnow holds the coordinator singleton that the desktop IPC handler and the Remote service share.Known follow-ups (not in this PR)
confirmedForegroundstep, and its branch read does not go through the workspace owner registry. That matches the desktop toggle's behaviour.aiden serve) does not offerchat:new-worktreeyet, so a phone paired with it never shows the chip.Mobile
chatNewWorktreecapability and solo negotiation.moveChatToNewWorktreewith a strict decode and a code-point branch seed.AidenNewWorktree.swift: availability, chip, setup row and the key-retry rule.mainmerge it runs beforemain's device-credential check on sends).AidenNewWorktreeTests, client and fixture.CHAT_NEW_WORKTREE, solo negotiation andAidenServer.supportsNewWorktrees.AidenNewWorktree.kt(model, UI, key-retry rule).AidenNewWorktreeTest.resultand tagged bykind(AidenGitResultWire). Before this, no Android Git read matched, so the branch-list check never hid the chip. This also repairs the Android Git screen's reads (review, diff, branches, comparison, push capability, worktrees).AidenChatTestson iOS,AidenNewWorktreeTeston Android) drive the real chat model:Docs
docs/aiden-remote-api-v1.md: capability row, route, revision 50 section, retry and late-finish rules.docs/aiden-on-the-go-remote-access.mddocs/plans/hybrid-new-chat-workspace-plan.md(Phase 5) anddocs/plans/README.mdios/CHANGELOG.mdProtocol revision
Revision 50. #646 (
chat:jobs) merged first with 49, so after mergingmainthis PR moved to 50 and updated the server, OpenAPI, fixture, iOS, Android and docs together.Tests
mainwith See and stop dev servers the agent leaves running, on the Mac and on phones #646 and moving to revision 50:npx tsx --test main/services/aiden-remote-{protocol,router,state,chats,git,new-worktree,background-jobs}.test.ts(all pass),scripts/check-ios-shipping-target.test.mjs, tsc. Earlier: a held-creation race test that fails without the move-time revision check, and the already-in-a-worktree answer.Flake notes (hosted CI):
renderer/components/assistant/use-assistant-live.test.tsx"stop failure reconciles an open provider session and leaves a truthful retry state" failed in Desktop tests (renderer-other) on head 06d8c76 (activewas false, expected true). This PR doesn't touch the voice assistant, and the test passes 3 of 3 times locally on this head. Run: https://github.com/sambitcreate/aiden-agent/actions/runs/41073399949/job/124123236101. The failed job is re-run once.main/services/bot-runtime/live-projection.test.ts"while classifying fails, a cached routine stays hidden…" failed in Desktop tests (runtime-subagents) on head 9656855 withretries are throttled, saw 3 calls, a timing check where the third classification call came under 295 ms after the first. This PR doesn't touch the Bot runtime, and the test passes 5 of 5 times locally on this head. Run: https://github.com/sambitcreate/aiden-agent/actions/runs/41175738521/job/124462890512. The failed job is re-run once.AidenChatTests"the snapshot and the image deck never finished loading" (XCTAssertFalsein an image-deck test this PR doesn't touch) failed in iOS build and simulator tests on head 4b01b07, the same flake seen on See and stop dev servers the agent leaves running, on the Mac and on phones #646. Run: https://github.com/sambitcreate/aiden-agent/actions/runs/41299030711/job/124826130880. Not re-run: the next push (cd5ed70) started a fresh run.main/services/coding-tools.test.ts"run_command waits for resistant process-group cleanup after cancellation, timeout, and output overflow" failed in Desktop tests (core-git) on head bb8b6e5 withcancel: descendant kept writing after the command settled(36 !== 35, one extra write after settle). The test comes from main's Fix shell, Git, terminal and schedule-script process cleanup (audit 06) #660; this PR doesn't touchrun_commandor the shell runner. Run: https://github.com/sambitcreate/aiden-agent/actions/runs/41302406573/job/124836149966. Not re-run: the next push (713e888) started a fresh run.AidenBotSessionOptimisticTests.testClosingTheChatCancelsTheRequestAFrameStarted("sent"vs"sending", a known Bot-session flake; the simulator clone also logged(ipc/mig) server died) failed in iOS build and simulator tests on the same head bb8b6e5. This PR doesn't touch Bot sessions. Run: https://github.com/sambitcreate/aiden-agent/actions/runs/41302406573/job/124836150107. Not re-run, for the same reason.tests/e2e/interrupted-question-resume.spec.ts:150"an approval waiting at a quit is named on the Resume card, never carried over" failed once on head 80c5973 ([data-interrupted-reply]not visible within 15s at line 178) and passed on retry;--fail-on-flaky-testsfailed the job. This PR doesn't touch interrupted replies. Run: https://github.com/sambitcreate/aiden-agent/actions/runs/41308679680/job/124856097053. Re-run once; it passed.main/services/bot-runtime/bot-pause.test.ts"a connect card the Bot is suggesting as it pauses never wedges it…" failed in Desktop tests (runtime-subagents) on head 5c554b0 (nothing ran under the pause, 2 !== 1). It failed again on the one re-run (job 125005377406) while main passed it. Cause: the test released the card check after a fixed 50 ms, and on a busy runner the pause's durable write took longer, so the check finished before the close began and the next model call ran first. Reproduced locally by delaying the pause by 80 ms. Fixed in the test (8f4fe33): the check now finishes when the pause's close signals the tool. Run: https://github.com/sambitcreate/aiden-agent/actions/runs/41352117073/job/124984617599.tests/e2e/chat-fork.spec.ts:35"a chat forks from a reply or edits a prompt in a fork" failed once on head 64e5e45 (line 45: the second reply never reached the saved chat within 10 s, 1 vs 2) and passed on retry;--fail-on-flaky-testsfailed the job. This PR doesn't touch forking. Run: https://github.com/sambitcreate/aiden-agent/actions/runs/41367704458/job/125036636196. Re-run once: the re-run hit a different flake,tests/e2e/bot-browser.spec.ts:22"a Bot's browser keeps its own sign-ins…" (shownByBot: nothing else is, pixel [255,0,0] vs [255,255,255]), which passed on retry (job 125061023877). That spec is already a known flake. Main had moved on (Redraw only the activity steps that changed #651, Keep chat images as files beside the chats #682), so it was merged in (a606662) and CI runs fresh.Screenshots
Not taken: there is no iOS simulator or Android emulator here. Phone screenshots need the Mac before approval.
https://claude.ai/code/session_018VA6RFZmgu9xAiKJ1wo2w2
Generated by Claude Code