Repository navigation
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
…ding a refused Send now had, this PC only
…storm, read prompts without a client's width, not on Windows
# Conflicts: # server/core/server-core.ts # src/lib/i18n/settings.ja.ts # src/lib/i18n/settings.ko.ts # src/lib/i18n/settings.zh.ts
…after a restart; no Portal scheduling; save before changing
|
Review rounds (omo + Codex) and how each finding was handled. Fixed in this PR
Known limits, left as follow-ups
|
…ed owner's record; the queue waits behind an uncertain scheduled paste; no open-LAN scheduling
…s guards are open
# Conflicts: # src/lib/settings.legacy-fixture.ts # src/lib/settings.ts
|
Final round (head after this push): the lead's last fix round is in: barriers lift only on a status change, an expired uncertain record leaves the pane marked, the queue's automatic drain waits behind an uncertain scheduled paste, both widths are read for a wrapped prompt, open-LAN windows and Portal visitors are refused, and a past time is never saved (the dialog asks for a new time; a held message edited with a past time stays held). The final omo and targeted Codex reviews still report reproduced HIGHs in the guards (pane move, queued submit on an idle pane, wide-character width). As the lead decided, the feature stays off by default and is labelled experimental in Settings, and the open items are tracked in #2951. The merge is the lead's call. |
… uncertain scheduled paste, and a possible password prompt with no client
…the paste only; no schedule with nothing in front; edits go under the editing device; no past times; no routes on Windows
|
Parking this PR as a draft. Each review round of the input guards keeps reproducing a new case where a scheduled message could be typed into the wrong prompt, and that risk is not worth shipping for this feature. Open findings at a6b5cfd are listed in #2951. It comes back with a simpler delivery rule (for example: deliver only while a client is attached and the pane is idle, else wait for the user's OK). |
# Conflicts: # server/core/server-core.ts # server/core/socket.ts # shared/protocol.ts # site/demo/transport.ts # src/components/Composer.tsx # src/components/PaneTerminal.tsx # src/components/SettingsDialog.tsx # src/lib/i18n.test.ts # src/lib/i18n/settings.ja.ts # src/lib/i18n/settings.ko.ts # src/lib/i18n/settings.zh.ts # src/lib/settings.legacy-fixture.ts # src/lib/settings.test.ts # src/lib/settings.ts
…open (#2951) Replace the width guess and colon fallback with one rule: a scheduled message is typed only while a device attachment to its pane exists and is not held by another bridge, read again before every check and key. Also: - a pane whose status has not changed since this server started counts as possibly holding an earlier paste (needs your OK); - the stranded mark and the unconfirmed turn follow a pane move; - a pane that ends holds its scheduled messages as gone, uncertain ones too, so every pane's list shows them; - discarding an uncertain scheduled message lets the pane's queue go again; - the dialog checks the time again when it is sent.
- a resize while the screen is read makes a scheduled message wait - Send now goes to the bound session unless the row said it changed, and is refused while another paste may sit in the pane's input - a pane ending keeps an uncertain message non-resendable and settles one on its way as gone; every pane lists messages of a gone pane - a state file with an unreadable entry is copied aside before a save
…of holding (#2951) A scheduled message due on a pane whose status has not changed since the server started now stays scheduled (awaiting_status) and goes at that change, where it was held for good before. Also copy the needs-OK fault when holding, and keep onSchedule's doc next to its prop.
|
Review rounds for the #2951 rework (one delivery rule: a scheduled message is typed only while a device has its pane open). omo at 80658c8: BLOCK on one MEDIUM (the restart barrier held a message for good) - fixed in 6e96312 (it now waits,
Codex (Terra) at 80658c8: fixed in cdbd884 with failing-first tests - a resize during the screen read waits; Send now keeps the bound session and is refused behind an uncertain paste; a pane ending keeps an uncertain message non-resendable and settles one on its way as gone; a state file with an unreadable entry is copied aside. Not changed (limits of the queued-message path on main, listed in the PR body): drafts are read only for Claude Code and Hermes; Codex follow-up at 6e96312: 3 HIGH + 1 MEDIUM, reproduced (re-run on that head), in edge paths of the guards (overlapping Send now, a failed backup copy, a resize away and back during one read, move-then-gone during a send). The PR had used its one fix round, so they are tracked in #3169. The feature stays experimental and off by default; the merge is the lead's call. |
- a second Send now waits while one of the pane's messages is on its way, and the pane's lane re-checks scheduled uncertainty before typing - a store that cannot write its state file refuses every claim, edit and discard instead of keeping them in memory only - a scheduled send compares the attachment and a grid change count across the screen read, so a resize away and back makes it wait - a pane that moved and then ended during a refused send settles as gone Fixes #3169
…s read - create() refuses when the pane ended or moved during its identity read - a scheduled send to a pane that no longer exists says pane_not_found, so every pane lists it, instead of waiting for a device forever
…fore a write (#3172) ## What A queued next-turn message (and anything else typed through `dispatchPendingText`) is checked right before herdr is written to, but two of those last checks had gaps. Both were reproduced by a Codex review of Send later (#2942, #3169); the code is the same on main and guards today's queued messages. 1. **Same-PID replacement.** `stillInFront` compared only the PID in front. A process that ended and left its PID to another, during the awaits before the paste, passed. And `samePendingIdentity` accepted a bound process start that can no longer be read at the same PID. - `stillInFront` now checks the bound PID (a bound process no longer in front is refused, not looked past), and compares its start last, after Claude's session read, through `samePendingIdentity` (macOS keeps its one-second leeway, now in `sameProcessBirth`). An unreadable start refuses. Pi's last check after its receipt anchor now passes the bound identity too. - `samePendingIdentity` refuses a known start of the same PID that now reads as unknown. A different PID was already refused; an identity that learns its start later is unchanged. 2. **A masked prompt that comes up during the last process check.** The screen was read before the last process check, so a password prompt that appeared while that check was answered got the paste (or the Enter). The screen is now read once more right before every write - the paste, the Enter, each OpenCode skill key, and Codex steering's Escape - after every other check, and the bound process's start is compared once more after that read (no herdr call). A prompt found before the paste types nothing; before the Enter the paste is left uncertain for the user, as other late refusals are. ### Pi receipts `reconcilePiDeliveries` also uses `samePendingIdentity`. A start that reads as unknown for the same PID there is inconclusive: the receipt is read again later instead of failed (only a write is refused on it). A launch-bound first message whose agent's start reads as unknown for the same PID likewise waits for the next read instead of being held as replaced; a start read as another one still holds it. ### Window left herdr cannot compare and write in one step. A prompt or a process that changes during the last screen and start reads, or during herdr's own write call, is still not seen: the window is now those two reads plus the write itself, instead of every check after the screen read. OpenCode's skill-accept Tab is decided on its own screen read of the autocomplete row after these checks; that same screen is checked for a masked prompt, then (as it is the viewport, which may be scrolled) the live screen is read once more and the process start compared, right before the Tab (a `beforeKey` hook of `typeWithSkills`). ## Validation - `bun run check fast` on this head: workflow syntax, generated files, typecheck, build, unit tests (12705 pass, 0 fail, 50 skip). - Failing-first: `server/core/pending-birth-guard.test.ts`. The review round's 5 (a bound process gone from the front, a same-PID swap during Claude's last session check, Pi's last check after the anchor, an OpenCode skill key and a steering Escape into a late masked prompt) fail before its commit and pass after. On main, 4 of the first 5 tests fail: a same-PID process before the paste, an unreadable known start, and a masked prompt during the last process check before the paste and before the Enter. The fifth (a same-PID process after the paste) already held on main and guards that path. All 5 pass here. - `server/core/pending-identity-race.test.ts`: the "birth read before Enter" case now holds read 6, since the last checks before the paste read the start three times more. Same scenario, same expected outcome. - `bun test server/core server/pending-input.test.ts server/pending-input.contract.test.ts server/dispatch.test.ts server/opencode-skill-attach.test.ts`: 610 pass, plus the race test after its read count was updated (93 pass with the birth-guard and identity unit tests). `pending-input.contract.test.ts`'s macOS drift control passes again now that the start check goes through `samePendingIdentity`. - Second review round (Codex): the skill-accept Tab with a same-PID swap or a masked prompt on its own screen read, and a Pi receipt whose start reads as unknown for a moment - 3 more tests in `pending-birth-guard.test.ts`, each failing before its commit. - Final Codex pass: CLEAR (no HIGH). Its two MEDIUMs (a scrolled viewport before the skill Tab; a launch-bound first message held on one unknown start, a regression of this PR) are fixed with 2 more failing-first tests. - CodeRabbit round: the deadline is checked again after the last reads before the paste (failing-first test), steering's Escape compares the status from before its screen read, test starts sit outside macOS's leeway, and the changelog says a refused message with nothing typed stays queued. - `bun test server/pending-input.contract.test.ts` (isolated herdr test session): 122 pass. - `server/pending-input.test.ts`: a known start of the same PID that reads as unknown is refused; every other unknown part stays permitted. - Not run: a live herdr pane (the windows are only reachable with mocked reads).
|
Parked as a draft. The shared delivery fixes this PR needed landed in #3172 (same-PID swap, unreadable start, bound process gone, a late masked prompt before every write) and #3177 (masked prompts wrapped by display width), and this branch's own #3169 round plus the create() race fix are in (head a943473). It does not merge yet: a scheduled send types while nobody watches, so it comes back only after origin/main is merged in and a fresh Codex pass reproduces no HIGH in Send later's own path. Not before the 0.5 test builds. |
Fixes #2919
Fixes #2951
What
Send later: schedule a chat message to a pane's agent. The PC's server keeps it and sends it at its time while a device has the pane open; with none open it waits until one does (#2951).
server/scheduled-messages.ts): the messages live in<stateDir>/scheduled-messages.json(mode 0600). One timer covers the next due message, and a pane's events retry its due ones. Routes:GET/POST /api/scheduledandPOST /api/scheduled/{update,discard,send}. A watching device getsread_onlyfor all of them, and a Portal visitor or an open-LAN windowowner_only: a visitor's access can be withdrawn, and nothing would check it when the message goes. They are this PC's only: a connection server does not pass them to a remote PC, since it forwards with its own credential and a device revoked there could not stop what it scheduled. The composer offers Send later on the page's own PC (follow-up: carry the scheduling device across the proxy). New feature flagscheduled-send, and a content-freescheduled-changedframe tells connections to read the list again.Delivery (same rules as a queued next-turn message)
server/core/pending.tsused to type a pending message only through a connection's lease. It now takes aSender(a connection under its lease, or the PC itself for a scheduled message), andsendScheduledruns the existingdispatchPendingTextwithautomaticset. As a result, a scheduled message:pending_wait), behind the pane's queued messages and the turn gate the last send opened (turnBusy,committingUnrecorded), never goes over a Claude or Hermes draft (input_draft) or an open question (agent_blocked), and stays scheduled saying why;scheduled-sendthere;openView). That device's width is what a wrapped password prompt is read at, as for a queued message; with no device on the pane nothing is typed and the message waits (input_not_ready,attach_held), so no width is ever guessed;awaiting_status) until the status changes, then goes at that event;samePendingIdentity). If the pane is gone (pane_not_found) or now runs another session (pending_target_changed), it is held with the reason until the user sends or discards it;sending, written to disk) before anything is typed, so the timer and Send now never send it twice. One the PC stopped in the middle of loads asuncertainand is never sent again on its own, nor by Send now, and the pane's other scheduled messages wait behind it;Send now steers a working agent, as a queued row's Send now does, and goes to the session the message was scheduled for; only once its row says the pane now runs another session does it go to what runs there now. It is refused while another message to the pane may sit in its input (a scheduled one or a queued one): that one is checked and sent or discarded first. A refusal leaves the message as it was, its binding included, with the reason. A message whose pane is gone shows in every pane's list, an uncertain one included (still Discard only), so it can be read and discarded. A create or a claim whose write fails leaves nothing behind. An unreadable state file, or one with an entry that cannot be read, is kept aside (
scheduled-messages.json.unreadable-<ms>), never written over unseen.Known limit, same as queued next-turn messages today: only Claude Code's and Hermes' input boxes can be read for a draft, so for other agents a draft typed in the terminal is not detected.
The demo (
site/demo/transport.ts) supports the feature in memory, so it can be tried there.Design reference
This follows the Schedule send of mail and chat apps (Gmail, Slack): a few one-tap times, then a free date-time field. The coding-agent apps named in
src/AGENTS.mdhave no equivalent. When this plan limit resets comes from the issue.Out of scope
Repeating schedules (the issue's "Later" section).
One delivery rule (#2951)
Earlier review rounds kept finding new cases in the guards that let a scheduled message go with every device closed (a guessed width for a wrapped password prompt, a client leaving mid-send, a pane move dropping a mark, a restart dropping the queue's barrier). This round replaces those guesses with one rule: a scheduled message is typed only while a device has its pane open; anything the PC cannot rule out waits for the user's OK. Closed here, each with a failing-first test:
Review round (omo + Codex), fixed with failing-first tests: a resize while the screen is read makes the message wait; Send now keeps the bound session and is refused while another paste may sit in the input; a pane ending keeps an uncertain message non-resendable and settles one on its way as gone; an unreadable entry keeps a copy of the file; the restart barrier waits instead of holding for good (omo MEDIUM). Left as a LOW follow-up: each open pane fetches
GET /api/scheduledon every change (one shared store per PC would do).Known limits shared with queued next-turn messages on main (not changed here): drafts are read only in Claude Code's and Hermes' input boxes;
shared/secret-prompt.tsmeasures rows in string length, not terminal cells, and knows a fixed set of masked-input prompts. With Send later these apply while a device has the pane open, as they do for a queued message.Also kept from the last round: a pane move holds the pane's scheduled messages as Needs your OK; a queued submit on an idle pane waits behind an uncertain scheduled paste; a time more than a minute past is refused;
/api/scheduledanswersfeature_unavailableon Windows. The setting stays labelled experimental and off by default.Validation
bun run check faston this head (6e96312): workflow syntax, generated files, typecheck, build, unit tests (12746 pass, 0 fail, 38 skip).server/core/pending-scheduled.test.ts(no device on the pane, a held attach, the last device leaving mid-send, no status change since start, wrapped prompts read at the device's width),server/pending-input.test.ts(marks follow a pane move),server/scheduled-messages.test.ts(gone pane, discard releases the queue),src/components/ComposerSendLater.test.tsx(time passed while the dialog stood open).bun test server/core/pending-scheduled.test.ts server/pending-input.test.ts server/scheduled-messages.test.ts src/components/ComposerSendLater.test.tsx src/lib/sendLater.test.ts src/lib/i18n.test.ts src/conventions.test.ts server/conventions.test.ts: 173 pass.