Repository navigation
Sky: a haiku that was not written leaves the one before showing - #680
Conversation
A claim cleared the kept haiku before the copy wrote one, so a reply that was empty, refused or failed saved `"lines": []`: every sky opened blank, and the same holder could not claim again during its hold. The claim is now kept apart from the haiku shown; a write that fails is a typed `Failed` (unreached, unanswered, not a haiku) recorded under `unwritten`, and only three lines can be published. A file an earlier claim had emptied shows the last of the day's haikus. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e30209778
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e claim The 0.8.0 `haiku.json` (no `claim`, a cleared haiku) is rewritten once by a registered `upgrade::once` migration, retiring at 0.10.0, so the reader knows only the current form. A sky opened while a claim's haiku is missing restores that claim's holder and hold, not the last writer's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
|
CI Generated by Claude Code |
# Conflicts: # sonny/FEATURES.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 071f7790c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let Some(_lock) = store.lock() else { | ||
| return false; | ||
| }; | ||
| let mut all = store.all(); |
There was a problem hiding this comment.
If an existing haiku.json cannot be read or parsed, Store::all() silently returns an empty map. This migration then overwrites the file with {} and records haiku-claims as complete, permanently discarding every daemon's poems and claim counts merely by opening a sky. Read and validate the migration input fallibly, returning false without rewriting or marking completion when conversion is impossible. The root migration guidance explicitly requires rejecting unconvertible data rather than silently resetting it.
AGENTS.md reference: AGENTS.md:L27-L30
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real. Fixed in the commit before eaf1c07: upgrade now reads haiku.json strictly under the lock. If it can't be read or isn't a JSON object, the migration returns false without writing and without recording itself done, so the next sky tries again. The test a_file_that_cannot_be_read_is_left_as_it_is_and_upgraded_once_it_can checks that a truncated file is left byte for byte, and is upgraded once it reads.
Generated by Claude Code
| self.title.rule = rule; | ||
| } | ||
| self.heed(kept, now); | ||
| self.heed(*kept, now); |
There was a problem hiding this comment.
If A's displayed poem is 20 minutes old and B claimed the title but failed one minute ago, returning from replay replaces B with A as holder, allowing a busy challenger to spend another claim during B's ten-minute hold. Unlike the corrected View::with initialization, View::replay(None) clears shown, and the next Heard::Read calls heed, which still restores the title from kept.by and kept.since_ms rather than kept.claim. This replay-to-live path is fresh evidence distinct from the previously fixed opening path. Update heed to synchronize the claim holder independently of the displayed poem.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real. Fixed in the same commit: heed no longer takes the title from the haiku shown. The view keeps the last claim it heard (claimed). A read whose kept.claim differs moves the title to kept.holder(now, HOUR) (set directly if the agent is here, or as heir otherwise), whatever haiku is new or shown. A replay doesn't touch the title, so coming back keeps B. The test a_haiku_another_sky_wrote_shows_here_and_its_writer_holds_the_title now has another sky claim and write nothing, then checks that B holds the title with A's haiku still up, both on the next read and after shown is reset as replay(None) resets it.
Generated by Claude Code
…ku file is not upgraded A read of haiku.json moved the title only with a new haiku, and from the haiku's writer, so a sky back from a replay handed the title back to the writer of the haiku shown over the claim that wrote nothing. Now a claim that differs from the last one heard moves the title, whatever is shown. The haiku-claims migration read the file leniently and wrote an empty one over a file it could not parse, recording itself done. Now it leaves such a file as it is and tries again at the next sky. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE
# Conflicts: # sonny/FEATURES.md
`a_view_that_opens_reads_where_the_windows_sit_now` sent a job that closes and opens windows, then read the places kept from before it. The worker thread could run that job and refresh the kept places first, so the test failed now and then (seen on #675 and #680). A job ahead of it now holds the worker until the read is done, as the naming test beside it does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE (cherry picked from commit f38b3c0)
`a_view_that_opens_reads_where_the_windows_sit_now` sent a job that closes and opens windows, then read the places kept from before it. The worker thread could run that job and refresh the kept places first, so the test failed now and then (seen on #675 and #680). A job ahead of it now holds the worker until the read is done, as the naming test beside it does. Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE Co-authored-by: Claude <noreply@anthropic.com>
…e until work resumes (#675) * Sonny tool host: a call that ran is never run again because its result was not recorded The host dropped a call from its active map whatever came of handing in its result, so when the daemon did not take the result (a lost connection, a store that could not write), the next sweep found the call still waiting and ran it again: a second write, message or task. A call answered is now kept with its result until the daemon takes it or says the call wants none (answered already, out of time, its turn over), handed in again every two seconds, and never run again meanwhile. The host's own state is in memory: a host that restarts between running a call and recording it still runs it again, which the execution records of the next release cover. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE * Sonny tool host: a pause stops what Sonny's calls run, and starts none until work resumes A pause parks every turn and stops the daemon's own processes, but the tool host heard only of calls started and turns finished: an exec script ran on after a pause and could make further MCP calls, ones that need no approval among them. The host now hears the pause: what each call still runs is stopped, so a script makes no call after it (one already made may have had effects, which the daemon's daemon_paused result tells its agent), and no waiting call starts while it holds, also for a host that starts or connects again while paused. Resuming lets them start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE * Sonny tool host: a result the daemon did not take wakes the host to hand it in again A call that ended after a sweep, whose result was then lost, waited for the 30 s sweep. Its task now wakes the serving loop. The declared-tools row records the guarantee and its limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE * Sonny tool host: a listing read across a stop starts none of its calls A pause (or a turn's end) that stopped a call while the waiting calls were listed left that call on the listing with no job to say it ran, so the sweep started it again after the resume. The host counts its stops, and a sweep whose listing a stop came during admits nothing from it and looks again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE * Sonny tool host: only a stop that let a call go makes a listing stale Every turn's end counted, so on a busy store a sweep could keep finding its listing stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE * Tests: the approvals test change belongs to the judge handover branch, not this one Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE * Sonny tool host: every pause invalidates a listing read across it A sweep admitted none from a listing read across a pause only when the pause stopped a running call. With nothing running yet, a call named on an earlier page could start after the resume though the daemon had already answered it `daemon_paused`. Each pause now counts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE * Tool host: a call a pause stops is answered, and losing the events stops calls too A call the host stopped for a pause was forgotten, so a turn the daemon had parked on it (past the answer hold) listed it again after the resume and it ran twice. Now a stopped call keeps an answer saying why (`daemon_paused`) and is handed in like any result. Losing the connection that reads the events stops the calls too (`events_lost`) and holds until the next greeting, since a pause while it is down reaches no event. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE * Sonny tests: hold the mux worker until the kept places are read `a_view_that_opens_reads_where_the_windows_sit_now` sent a job that closes and opens windows, then read the places kept from before it. The worker thread could run that job and refresh the kept places first, so the test failed now and then (seen on #675 and #680). A job ahead of it now holds the worker until the read is done, as the naming test beside it does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013jkdGG873rfqV9CdHz7KrE (cherry picked from commit f38b3c0) --------- Co-authored-by: Claude <noreply@anthropic.com>
Wave 1 item 9 (D1, #669): an empty haiku left Sky blank.
Cause
Kept::claimcleared the kept haiku (lines,at_ms) when an agent took the title, before its copy wrote anything. If the copy's reply was empty, refused, failed or ran long,writereturnedNoneand nothing else touched the file.haiku.jsonthen held"lines": []: every sky that opened showed no haiku (it needs three lines), and the holder could not claim again for the rest of its hold.Change
Kept.claimis(by, since_ms)and the hold the cap and the "once a hold" rule count.by/lines/at_ms/since_msare the last haiku written, and onlypublishchanges them.haiku::writereturnsResult<[String; 3], Failed>, withFailed::Unreached(detail)(daemon or fork),Failed::Unanswered(detail)(still running past the wait, failed, or cut short) andFailed::NotAHaiku(an empty reply among them). The sky records it underunwritten: {by, at_ms, why, detail}inhaiku.json, but only while that claim is still the current one.cleanandpublishtake[String; 3], so a haiku of the wrong length can't be built.pasthaikus, and the claim it held stays spent.Heard::ReadboxesKept(clippylarge_enum_variant).Restart and replay
haiku.jsongainsclaimandunwritten. Older files load as described above. Replays readpast, which never held an empty haiku.Tests (fail before, pass after)
a_claim_that_writes_no_haiku_leaves_the_one_before_showing: a writes a haiku, b claims and fails withNotAHaiku, and every load still shows a's haiku. The count stays spent, b is not tried again for the same hold, and a late failure for a claim that has been taken over records nothing.a_file_whose_claim_cleared_the_haiku_shows_the_last_one_written: a 0.8.0 file with"lines": []shows the lastpasthaiku.clean("")and whitespace-only replies are not haikus.cargo test -p sonny, clippy and fmt pass.There is no change on screen when a haiku is written, so there are no screenshots. The only visible effect is that the previous haiku stays up where there used to be a blank. FEATURES.md
/skyrow updated.Generated by Claude Code