Bulletproof the background wait, batch delivery, and the editor - #42
Merged
Conversation
The skill told agents to run poll --timeout 600 in the foreground and re-run it on every timeout. Now it starts one open-ended poll in the background and ends the turn; the harness wakes the agent when the command exits, which only happens on Send or when the review is closed. To make an open-ended wait safe, the CLI reconnects (restarting the server if needed) when the connection drops instead of giving up after three tries, and treats a socket close without end as a drop so the promise cannot be left unsettled.
From community PR #37 by @dacoldest.
From community PR #30 by @ulysses0604.
From community PR #36 by @NuclearManatee.
…eading-label bugs - Port community PR #7 by @noammigdali-hio onto the invocation(run) signature: a bin under any node_modules segment is transient, not just npm's _npx cache. - Issue #12: clicks inside the compose textarea place the caret instead of being swallowed by the card's focus handler. - Issue #25: <summary> keeps its native toggle so collapsed content stays reachable. - Issue #26: a heading labels itself instead of being named after the heading before it.
…ds, one server per state dir - A tab that unloads tells the server; unless it reconnects within a short grace the review ends with reason window_closed and the waiting poll is released. A tab that silently vanishes ends the review after the 30-minute TTL instead of being deleted with the poll left hanging. - A poll with no review open waits a short grace for one, then closes with reason no_review_open. Closed answers carry the unsent counts. - A new poll for a target supersedes an older one, so stale background waits cannot double-apply a batch. - delivered is persisted, so a server replaced between delivery and --ack honors the ack instead of re-shipping the batch. A batch written by an overlapping server is read from disk. - Sending while the agent is applying a batch queues only the new items and carries the old cleanup forward; the agent's next --ack clears both. - edits_saved and markdown flags per page; edit text capped at 200k with an explicit truncated marker instead of a silent 4k cut; Markdown and self-rendering pages keep unsent edit rows when the file changes on disk. - Artifact URLs carry a per-run secret segment; a page key derived from the file path is no longer enough to read sibling files. - Localhost reviews proxy the app's own requests (API calls, prefetches, srcset images) to the app origin instead of 404ing. - One server per state directory: a lock file plus a health-checked takeover of an older-protocol server. Protocol bumped to 9. - New routes: DELETE edit (undo), POST mode (dynamic), POST discard, POST away. - Comment rewording updates an undelivered batch in place, or takes a fresh id after delivery so the rewrite ships next time.
…er banner, multi-block edits - The chrome tells the server when the tab unloads (keepalive fetch on pagehide) and re-opens a session when the server has forgotten it, instead of turning into a dead tab that still looks alive. - Undo for the two irreversible gestures: a toast with an Undo button after a block delete or move restores the block and drops its edit row. - Feedback left over from a review that ended without a Send shows a Keep/Discard banner on the next open, noting that HTML text edits are already in the file either way. - A second Send while the agent works shows as queued, not as nobody listening. Comments get an Edit button and an edited badge; rewording after delivery explains it ships with the next Send. - Typing over a selection that spans blocks reports every block it touched, including a deleted row for one that vanished; a paragraph turned into a list keeps its label and captured original. - CLI: an open-ended wait ends after 12 hours with a timeout status that says to check status and start again; superseded polls just print. - SKILL.md: closed/superseded handling, edits_saved, deleted and truncated rows, rendered quotes with anchors, staged assets before ack, note-only batches. Codex block updated to match. - srcset URLs are absolutized for localhost reviews.
…oad order
- After the server ended a session, the tab's event stream reconnected, got
a 404, and the new re-bootstrap path opened a fresh session with no tab
behind it, which kept the review open and the poll waiting. The chrome
now never re-bootstraps once the review ended or the tab is unloading,
and the server drops a session no browser connected to within a minute.
- Label ordinals ("p 3") come from sibling order at load, so deleting a
paragraph does not hand two blocks the same label within one review.
- A list command now retracts the row the fresh <li> emitted about itself
and reports the change under the paragraph's own label with its original
text.
petergyang
changed the title
…meout On a slow Windows runner the poll's reconnect could spawn its own server before the test's replacement announced itself, and then wait forever for a batch sent elsewhere. The first reconnect now waits 3s, the test races the poll against a 30s timer, and every test gets a 60s timeout so a hang fails the file instead of the whole job.
…the poll in the foreground Codex returns a detached session id and nothing wakes the agent when that session finishes, so a Send sat unread until the user nudged. The skill and the AGENTS.md block now say which harness gets which wait, and how to cope with a shell tool that caps command duration.
…he ended-turn limit plainly
A project set up months ago kept the instructions from that day forever, because setup skipped any AGENTS.md that already mentioned human-review. The block is now written between marker comments and rewritten on every setup; the unmarked block an older setup wrote is recognized by its heading and replaced; a project that wrote its own human-review guidance is left alone.
Every edit flush rewrote state.json in full, and it embedded the complete HTML of every page reviewed in the last month. The copy now lives in pristine/<key>.html, is written only when it changes, and is removed with its page. A state file that still embeds copies loads as before and moves them out on its next save.
Three tests boot the SDK against a jsdom document: block delete with undo, typing over a selection that spans blocks, and stable labels after a deletion. The last one caught that the load-order snapshot skipped the body element, so top-level paragraphs still renumbered.
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
Builds on the background-poll change so every wait terminates cleanly, then lands the top items from the audit and the community PRs. One branch, phased commits.
The wait (why #42 exists)
human-review poll <target>in the background and ends its turn. The command exits only on Send, End review, tab close, or when no review is open — never on a timer. An open-ended wait reconnects across server restarts and caps itself at 12 hours.status: closed, areason, and the unsent counts, and the skill tells the agent to report those in one line.Batch delivery
deliveredis persisted, so a server replaced between delivery and--ackhonors the ack instead of re-shipping. A batch written by an overlapping server is read from disk.--ackclears both. The UI shows "queued" rather than "nobody is listening".edits_savedandmarkdownflags. Edit text is capped at 200k with an explicittruncatedmarker instead of a silent 4k cut. Markdown and self-rendering pages keep unsent edit rows when the file changes on disk.Hardening
srcset) to the app origin.srcsetis absolutized too.Editor
Community
SKILL.md
edits_saved,deletedandtruncatedrows, quotes are rendered text withanchorcontext, copy staged assets before ack, note-only batches. Codex block matches.Test plan
npm test— 117 passing; newtest/lifecycle.test.jscovers tab close, reload grace, no-review grace, supersede, queued sends, delivery across restart, truncation, external writes, undo/discard, rewording,edits_saved, artifact secret, and the localhost proxypollwithreason: window_closed; Undo toast removed the row; leftover banner shows the right counts; no console errorsProduction cleanup (added)
setupnow owns the AGENTS.md block with marker comments and rewrites it on every run, replacing the unmarked block older setups wrote. Projects set up months ago pick up current instructions instead of keeping stale ones forever.pristine/<key>.htmlinstead of inside state.json, written only when it changes. Edit flushes no longer rewrite whole documents.