mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-23 11:03:41 +00:00
b9dff0072ebba34a73c4f3704bb48ba4fb2cff0e
1443 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 xarmian
|
b9dff0072e |
test: close the coverage gaps codex round 3 named (BUG-2730)
Round 3 reviewed the added tests as production code and found four things, all real: - the atomicity test never asserted the replay CARRIED anything, so an implementation that always answered "cannot vouch" and delivered everything live would have satisfied "never both" by never replaying. It now seeds two events and resumes from the first. - both handler tests would have passed for a handler that emitted sync_required and then closed the stream. The activity one now proves an ordinary event still arrives afterwards; the watch one, which has no cheap ordinary event to publish, proves the handler did not return. - the refused-connection test's end state also holds on origin/main, so it argues for nothing about the new ordering. It is a regression test for the defect that ordering introduced, and the comment now says so. - whole paths had no test: the activity RedisBus's drop, coverage-drop and ID-space-reset signalling; the watch bus's epoch-change and counter-backward arms; the InstrumentedBus delegation; the metrics adapter; and the mid-stream counter increment. All covered now. The wrapper tests matter most of the three new files: it is the only implementation that does not originate a gap channel, and in production the bus IS wrapped, so a wrapper returning nil there would have disabled the whole fix while every bus-level test stayed green (CONVE-19). |
||
|
xarmian
|
b2277641cf |
fix(events): a refused connection is not a sync_required (BUG-2730, codex round 1)
Moving the Last-Event-ID parse above the subscribe — which is what makes the atomic subscribe-and-replay possible — put the unreadable-cursor increment on the wrong side of the per-workspace admission check. A connection refused with 429 is sent nothing, and was still counted in pad_event_resume_gaps_total, whose population is signals SENT. Counted at the emission site instead, which is where it effectively was before the parse moved. Pinned with both legs: refused must not count, admitted-with-an-unreadable-cursor must. |
||
|
xarmian
|
db8c5b76ed |
docs(deployment): sync_required is not only a resume answer (BUG-2730)
The signal's documented meaning was resume-shaped in every place it appeared, while the fix widens it to a live subscriber told mid-stream that it has a hole. A widened signal whose docs still state the narrow meaning is a half-shipped contract. Adds a subsection stating both situations and what a client does with each, and corrects the two resume-gap counters' descriptions: they count SIGNALS, not resumes, so a deploy with no reconnects at all can now move them. Documents the new pad_event_events_dropped_total, including that a deploy which starts reporting it may simply be the first that could. |
||
|
xarmian
|
1cee8e615c |
test(server): the quiet-control leg could not see a gap announced at connect (BUG-2730)
A mutation that made the activity handler announce a gap unconditionally at connect SURVIVED both tests. The control asserted the absence only AFTER publishing an ordinary event, and waitForFrameWithEvent reads past every frame that is not the one it wants — so the spurious sync_required went by unexamined and the window that followed was genuinely empty. The absence is now asserted first, before anything else is published, and the wait for the ordinary event refuses a sync_required instead of reading past it. Both legs fail on the mutation now. |
||
|
xarmian
|
af99762815 | test(server): the gap signal must reach the wire, not just the bus (BUG-2730, CONVE-19) | ||
|
xarmian
|
fa2ddf3f22 | test(events): pin the drop signal, its metric, and the subscribe-replay atomicity (BUG-2730) | ||
|
xarmian
|
9f23364b43 | test(watchevents): pin both gap-signal scopes with their counterfactuals (BUG-2730) | ||
|
xarmian
|
4269bdd4cc |
fix(watchevents): the same hole, told to the stream holding it open (BUG-2730)
fanOutLocally already DETECTED a gap in the received notification sequence, logged the exact id range, and raised knownFrom so a client RECONNECTING across it was honestly told sync_required. A client holding the stream OPEN across the same gap was told nothing: it stayed connected, kept receiving everything after the hole, and never saw what went missing. The instance knew; the one consumer whose correctness depended on it did not. Subscribers now carry the same capacity-1 coalescing gap channel the activity bus grew, raised from two different scopes: - per-INSTANCE, to every live subscriber, when this instance discovers a hole in what it received — a sequence gap, a counter that went backwards, an epoch change. Instance-scoped is the whole scope: the ids never arrived HERE, other instances may have them, and every subscriber registered at the moment of detection is exactly the set that was connected across the hole. - per-SUBSCRIBER, to one connection, when its channel was full. The watch SSE handler answers with sync_required mid-stream. The pad CLI monitor already clears its cursor on that event, so the client half needed no change. Refs BUG-2730. |
||
|
xarmian
|
b5615cc1b5 |
fix(events): a live subscriber is told when it has a hole (BUG-2730)
The activity bus dropped an event for a subscriber whose 64-deep channel was full, logged it, and continued. The subscriber was told nothing, so a later delivered event advanced its Last-Event-ID past the dropped ids — after which no replica would ever replay them, because every replica agrees that cursor is current. Only a full reconciliation corrected it. Every subscriber now carries a capacity-1 coalescing gap channel. It is raised when the bus cannot hand that connection an event, and when this instance's coverage of a workspace ends under it (a pub/sub flap, an undecodable message, an ID-space reset) — that second case is per- instance and reaches every subscriber of the affected workspace, because the gap is theirs collectively. The SSE handler selects on it and emits sync_required mid-stream, the same signal a resume we cannot serve gets: the web client already answers it with an incremental /changes delta, so the distinction a new event name would express is one the client would act on identically. Also folded in, because they are the same defect surface: - Subscribe-and-replay is now ONE critical section on both bus implementations (SubscribeAndReplaySince), closing the duplicate window where an event published between the two steps landed in both the replay set and the live channel. MemoryBus takes b.mu across the buffer append and the fan-out to get it; the lock order is b.mu then b.replayMu, everywhere. - internal/events gains the drop report its sibling has had since BUG-2699 (Observer.EventDropped, pad_event_events_dropped_total). The drops were log-only, so the condition this fix makes honest was not countable before it. Both resume-gap counters' help text now names what they actually count — sync_required signals, including the mid-stream ones — rather than resumes alone. Refs BUG-2730. |
||
|
xarmian
|
2a121c32d5 |
Merge pull request #1178 from PerpetualSoftware/fix/events-idspace-migration
fix(events): give every event ID space an identity, behind a two-phase flip (BUG-2736) |
||
|
xarmian
|
6e590b48ff |
docs(events): the straggler window closes per workspace, not globally (BUG-2736)
Codex round 21, correcting a claim I made in round 17 and asserted only in the direction that was convenient. Round 17 said the mixed-roll straggler window 'is one event wide and ends loudly', because the next event from the new space is lower than the straggler's id and trips the counter-backwards check. That check is PER WORKSPACE and the sequence counter is GLOBAL. If other workspaces consume ids past the straggler's value before this one publishes again, this workspace's next id is higher, nothing fires, and the dead-space id stays in the buffer — where a client resuming from just below it is served it as though it followed. My test asserted the closing case and stopped there, which is the shape my own record names: a partial verification stated without its boundary reads as a complete one. The boundary is now its own test, written as a characterization — it asserts that nothing detects this TODAY, so if someone adds the global high-water mark that would close it, the change announces itself there rather than in a deployment. Not closed here. A global comparison fires on interleaves across ANY pair of workspaces during the phase-2 roll, when un-flipped publishers interleave routinely — the storm round 9 armed this check against. It belongs with the other residuals the client cursor's missing epoch would close. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
db7a9317ed |
fix(events): two mechanisms that were each right alone (BUG-2736)
Codex round 20, asked to check the accumulated fixes in COMBINATION rather than one at a time. Both findings are interactions, which is what that angle is for. A STRAGGLER ERASED THE COUNTER-BACKWARDS FLOOR. The straggler drop reused the same argument an epoch change uses, which CLEARS the floor — but a lower generation inside the window is not proof of a new space; that is precisely the question the window exists to defer. The sequence: a counter-backwards reset raises the floor to 100, a straggler clears it, bare traffic repopulates from 51, and a client resuming from 99 is served later ids as though coverage were continuous, silently skipping 51..99. The boolean is now three named intents — clear, raise, keep — because the third had no way to spell itself and so took the wrong branch silently. Each call site now says which it means. A WRONG-TYPED EPOCH KEY DEFEATED THE RECOVERY WRITTEN FOR A CORRUPTED ONE. Round 11 rotated an epoch key holding a bad VALUE; a bare GET on a key holding a list raises WRONGTYPE and aborts the script before that branch can run, so every phase-2 publish failed until someone deleted the key by hand. The script checks TYPE first. THE MUTATION MATRIX EARNED ITS PLACE TWICE HERE. The first version of the wrong-type test passed with the TYPE check deleted: the script's id == 1 branch SETs the epoch unconditionally, and SET replaces a key of any type, so on a fresh counter the wrong-typed key was overwritten before the GET was reached and the branch under test was never entered. The test now publishes once first, and the mutation fails it. The same matrix then found a DEL in the recovery branch that no test could distinguish — redundant for the same reason SET replaces any type — so it is gone rather than left as a line nothing can justify. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
53afc2172c |
fix(events): a generation we cannot vouch for ends coverage, not just the message (BUG-2736)
Codex round 19, inside round 6's own fix. Round 6 made a LOWER generation inside the straggler window discard the message. It left the replay buffers valid — so a client reconnecting during that window was told it was caught up. Harmless if the message really was a straggler, and thirty seconds of silently missed events if the generation had regressed instead, because then the messages being discarded ARE the live stream. A bus that has just decided it cannot classify what it is seeing must not go on claiming it can answer for the span. Coverage now ends on the first lower generation. The CLASSIFICATION still waits out the window — the epoch is not adopted there — so a true straggler does not drag the bus into the dead space. Its cost is one extra drop next to a rotation that had already dropped the buffers, which is nearly free and loud either way. That changes what epoch_regressed means, so its documentation changed with it: it now reports that a lower generation was SEEN, and the two causes are told apart by count rather than at the moment it fires. One alongside an epoch_change is a message in flight during a rotation; a run of them is Redis losing writes. The test asserts both halves — the straggler still does not move the epoch and is still not buffered, AND coverage ends — plus the control that the live generation re-establishes coverage immediately, so this is a resync rather than a dead bus. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
f3aa86503a |
fix(events): assign the in-memory id under the lock that orders the buffer (BUG-2736)
Codex round 18, pointed away from the Redis bus that seventeen rounds had concentrated on. Three findings; one belonged in this unit, one is filed, one is documented. THE ID WAS ASSIGNED BEFORE THE REPLAY LOCK, so two concurrent publishes could take N and N+1 and append in the other order. replayBuffer.since computes oldest and newest by POSITION, so a buffer holding [N+1, N] reports N as its newest and answers a resume from N+1 with sync_required — a client told to resync at the moment it was exactly current. Pre-existing in shape, but it is the same invariant this unit buys for the Redis bus with an atomic publish script, on the same buffer, for the same reason: publish order must equal id order because the buffer's own ordering assumptions are otherwise false. Fixing one and leaving the other would be half an invariant. The test drives 300 concurrent publishes and asserts both the order and its consequence — that every id in the buffer is servable as a cursor, which under the race the newest ones were not. Verified to fail 5 of 5 with the assignment moved back out. FILED, NOT FOLDED IN: BUG-2737. Neither activity bus refuses a subscription after Close, so a handler that subscribes during shutdown holds a channel nobody will close and blocks for the full 30s deadline. It is a shutdown-lifecycle defect rather than an id-space one, it spans both implementations, and internal/watchevents already fixed the identical thing in BUG-2651 — so the fix is porting a decided question, not answering one. DOCUMENTED: the SSE data body carries the event id as a JSON NUMBER, which is now around 1.8e18 and past JavaScript's MAX_SAFE_INTEGER. It cannot be removed — the Redis bus's phase-1 wire form carries the id there and nowhere else — so the frame writer now says that the "id:" field is the one a client may use, and why web's ItemEvent deliberately declares no id. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
a0eb070b00 |
docs(events): name the mixed-roll straggler window, and assert what bounds it (BUG-2736)
Codex round 17. Once a replica has adopted an epoch, a message from an un-flipped instance carries none and is treated as belonging to the current space. It does — unless the sequence counter reset between that publisher assigning its id and publishing it, in which case an id from the dead space lands in a buffer describing the new one. NOT FIXED, because every alternative rule is worse and there is no discriminator. Refusing bare messages once an epoch is adopted would end coverage on every un-flipped publish for the length of the roll, which is a resync storm; delivering without buffering would put holes in the buffer that nothing records. An id from the dead space and an id from an un-flipped publisher are both 'above what we hold' and otherwise identical. What makes it acceptable is a mechanical property rather than an argument, so it is asserted rather than described: the next event from the new space is LOWER than the straggler, which trips the counter-backwards branch, drops the buffers and reports a reset. The exposure is one event wide and it ends loudly. The test also pins the other half — that nothing can detect the straggler ON ARRIVAL — because a reset there would mean the discriminator exists after all and the whole disposition was wrong. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
be47896e13 |
fix(events): hold both wire forms to the same id rule (BUG-2736)
Codex round 16. The positive-id check lived inside the PREFIXED branch of decodePayload, so a bare payload carrying id 0 or a negative was accepted: delivered with no SSE cursor for the client to advance to, and — once an epoch has been adopted — read as the sequence going backwards and used to discard every replay buffer. The check is now a function both branches call, which is the by-construction form rather than a second copy that can drift. Same route as an unreadable payload: this workspace's coverage ends and the next resume says so. The failing case that made this worth finding is the one my own round-11 test did not cover — it exercised the prefixed form only, because that is the form the guard was written next to. Symmetry asserted in the rejections table now, for both forms. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
19f29e1a67 |
fix(events): the channel is the authority on whose event a message is (BUG-2736)
Codex round 15. Two payloads decode without error and are not a usable event,
and both were skipped silently.
"null" and "{}" both unmarshal into a ZERO Event, whose workspace is the empty
string. Fan-out indexes subscriptions by the body's workspace, found none for
"", and returned early WITHOUT ending coverage — so the buffer went on looking
continuous across an event it had skipped, and a later id was replayed after
an earlier cursor as though nothing was missing.
A body naming a DIFFERENT workspace from the channel it arrived on is the same
shape one step further: fan-out would have appended it to that other
workspace's buffer, with an id from a stream those subscribers are not
reading.
The receive path now requires the body's workspace to match the channel's.
Both cases take the same route as an unparseable payload, because they are the
same fact about the world: something reached this channel that this
installation did not publish.
Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X
|
||
|
xarmian
|
1e40c972ae |
refactor(events): one home for the asymmetry, one assertion for the count (BUG-2736)
Codex round 14 was a prune pass rather than a defect hunt: 695 comment lines to 243 code lines in internal/events and internal/idspace is a ratio worth looking at, and my day-51 close records what happens when review prose grows under a diff. MOSTLY DECLINED, and the reason matters more than the decision. The list would have stripped this diff to terse definitions -- deleting bug archaeology, rollout tradeoffs and codex-round attributions -- in files whose EXISTING comments are dense narrative and carry exactly those things, "(codex round 6)" attributions included. It also named several blocks on code this diff never touched. Applying it would have made the new code inconsistent with the file it lives in, which is a style change to someone else's codebase wearing a review's clothes. APPLIED, because both match lessons this team already recorded rather than a reviewer's preference: - The numeric-base-vs-travelling-epoch argument existed in FOUR copies, in both packages. The lead's requirement was that the asymmetry declare itself in both buses' comments -- not that the argument be written four times. It now lives once, in internal/idspace's package comment (the shared code both halves depend on), and the four sites state the asymmetry in a sentence and point there. Four copies is four things to update when the deferred follow-on lands. - "Costs at most ONE drop per instance per roll" was a countable claim in prose with nothing asserting it. A comment asserting countable behaviour belongs in the suite: later messages in the same generation must leave the buffers alone, or "bounded" in that sentence is doing no work. Honest measurement, since the point of the round was volume: the net is +47 -41. The prune did not shrink the diff; it removed three copies of one argument and turned one claim into an assertion. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
9fe2839cc2 |
fix(events): an epoch that overflows int64 is corruption too (BUG-2736)
Codex round 13, plain pass, inside round 11's own fix. That fix made the publisher rotate an epoch key holding anything that is not a positive generation, so corruption self-heals instead of dropping every event forever. The pattern match it used accepts a value that is all digits and overflows int64 -- which the RECEIVER parses with strconv.ParseInt, so the payload is rejected on arrival and every event is dropped anyway. The same total, silent, unrecoverable failure by a different route, through the guard written to prevent it. Bounded by length rather than by value, because Lua has no int64 comparison to hand: any 18-digit number fits, and a generation counts an installation's id-space resets, so 18 digits is not a bound anything real approaches. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
5f8252ce88 |
test(events): cover the migration's central claim and three untested branches (BUG-2736)
Codex round 12 asked only "what has no test at all", which is a different question from "is each fix pinned" and found things twelve rounds of the second question had not. THE CENTRAL CLAIM HAD NO TEST. Parsing, publishing and fan-out were each exercised alone; that a phase-1 and a phase-2 instance on one Redis deliver each other's events -- the thing the whole two-phase design rests on -- needed both buses at once and nothing did that. Now both directions, with both receivers asserted, and the premise that the two payload forms really did differ. THE BACKWARDS GUARD IS `<=` AND EVERY TEST USED A STRICTLY LOWER ID. A `<` implementation passed all of them while letting a REPEATED id into the buffer -- a duplicate delivery and a replay that can serve the same id twice, with no reset reported. Mutation-checked. THE ROLLBACK PRECEDENCE HAD NO TEST. The procedure tells an operator to make the EFFECTIVE value false and warns that unsetting the env var is not the same thing; neither half was pinned, so a load order letting the file win over an explicit env-var false would have kept a deployment on phase 2 while its operator believed they had rolled back. AND THE RESIDUAL IS NOW EXECUTABLE. A bus with empty buffers adopts an epoch without dropping, so its first buffer starts at the first id it sees and a client holding the id one below is served. That was described in three places and asserted in none. It is now a characterization test that states the load trade in its own comment, so a future change to it is a decision rather than a side effect -- and the boundary is pinned exactly: adjacent is served, one lower is a gap. Declined with reasons: the startup phase log cannot be driven without standing up a server and its wrongness is visible on first read; the race between a phase-1 epoch delete and a concurrent phase-2 mint is documented, bounded at one extra buffer drop, and not deterministically reproducible; and the cross-Redis failover retry needs two Redises with controlled replication lag, which miniredis cannot represent honestly. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
f243540430 |
fix(events): three failure paths that lost events without saying so (BUG-2736)
Codex round 11 enumerated every Redis call, script step, parse and conversion the diff adds. Three of its findings were silent-loss paths. THE DEDUPE TOKEN WAS WRITTEN IN THE WRONG ORDER. Redis runs Lua atomically against interleaving, NOT with rollback: a script that errors part way through keeps whatever it already wrote. With the token written first, any later failure -- a wrong-typed key, an ACL denial -- left the token behind on a run that never published, and go-redis's retry then declined it. The event lost, permanently, with the caller told it succeeded. It is now CHECKED first and WRITTEN last. A script that dies early leaves no token and the retry does the right thing; a script that completed and merely lost its reply leaves one and the retry declines. The remaining window is an error on the final SET, whose key is a fresh uuid and so cannot be wrong-typed, and whose cost would be a duplicate rather than a loss. AN UNREADABLE MESSAGE WAS DROPPED AND FORGOTTEN. The buffer went on claiming a span that now had a hole in it: the event gone, the ids either side contiguous, and a later resume across it answered "caught up". It now ends that workspace's coverage, so the resume answers sync_required. The workspace comes from the CHANNEL rather than the body, which is what makes that possible when the body is the thing that would not parse. THE PUBLISHER TRUSTED WHATEVER THE EPOCH KEY HELD. Set to something that is not a positive generation -- corrupted, hand-edited, or written by another installation sharing the keyspace -- it was emitted into every prefix, every receiver rejected the payload, and every event was dropped for as long as the key stayed that way. The script now rotates instead: one generation change, one round of resyncs, and the space is identifiable again. Also: decodePayload refuses a non-positive id. The SSE handler omits the id: field for one, so such an event would be delivered with no cursor to advance to and the client would resume from the id before it forever. Both new conditions get their own reason label rather than being folded into an existing one, because an operator acts on undecodable_message differently from anything else here: it means something is publishing onto these channels that is not this installation. Mutation matrix: 4 applied, 4 caught -- but only after two survived the first pass. The dedupe order and the id check had no test that could tell the fixed code from the broken code; the tests that pin them now had to be written to make the mutations fail, which is the point of running the matrix rather than counting the tests. Declined with reasons: the phase-1 assign/publish eviction window is the legacy path this migration exists to replace, and the resume-gap counter's missing cause label is a pre-existing shape. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
6afe683389 |
fix(events): do not arm reset detection where interleave is ordinary traffic (BUG-2736)
Codex round 9, from the 3am-operator angle. Six findings; one of them was a regression this diff would have shipped in the DEFAULT configuration, and the review framed it as a log-volume problem. THE REGRESSION. Phase 1 publishes with a two-call INCR-then-PUBLISH, so on any multi-instance deployment two publishers interleave routinely and a lower ID arrives after a higher one as ordinary traffic. main has no counter-backwards detection at all; this diff added it. Armed unconditionally, it would have fired on that ordinary interleave, dropped EVERY workspace's replay buffer, and resynced every client -- in phase 1, which is where every deployment sits until an operator flips phase 2. The check is now armed only once an epoch has been adopted. What that costs is stated rather than hidden: a genuine counter reset on a never-flipped deployment goes undetected, which is exactly the behaviour before this change and precisely the case phase 2 exists to fix. The new test asserts the gate, and also asserts what is NOT claimed -- the interleaved workspace's own buffer still holds ids out of order, so a cursor at the higher one reads as foreign. That is pre-existing, unchanged here, and strictly less harmful than a global drop; it is asserted rather than described so a future change to since() surfaces there. THE REST ARE THE OPERATOR'S SIGNALS, which were unreadable: - The effective phase was invisible. pad_event_sequence_resets_total cannot be interpreted without it -- a counter_backward rate is expected on phase 1 and an anomaly on phase 2 -- and the setting can arrive from an env var, a TOML file, or neither. It is now on the startup line as id_space_phase. - An unparseable PAD_EVENTS_PUBLISH_EPOCH was silently ignored, so an operator who typed "yes" believed they had flipped. Ignoring it stays the right behaviour; being silent about it does not. - Both publish-failure logs said only "failed to publish". They now say what the operator needs, which differs by phase: phase 1 may or may not have reached subscribers, and phase 2's script is atomic so it did not half-execute, but a lost reply means it may have published anyway -- do not re-publish by hand. - Adopting an epoch with empty buffers is the moment the documented residual becomes possible on that replica, and it happened silently. It now logs at INFO -- not a reset count, deliberately, since counting it would give the reset metric a per-deploy baseline. Declined with reasons: a cause label on the resume-gap counter and a publish failure counter are both pre-existing shapes rather than anything this diff changed, and the straggler log is already bounded by the recovery window. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
378dec5244 |
docs(idspace): name the assumption the incarnation bound rests on (BUG-2736)
Codex round 8, on a fresh angle. The invariant was stated in terms of publish RATE -- an id can repeat across incarnations only if the earlier process published more than 2^20 events per millisecond of its life -- and quietly assumed the other half: that the next start lands in a LATER millisecond. The bases are separated by the clock at millisecond resolution, and the CAS separates only buses built inside one process. A second process starting inside the same millisecond as the first would take the same base and reissue its ids. Not closed, and the reason it is acceptable is physical rather than hopeful: reaching the constructor means the OS reaped the old process and the new one bound its listener, opened its database and ran migrations. Closing it for real needs persistence, which BUG-2736's body rules out for a separate and stronger reason. So it is accepted and NAMED -- in the package comment and in deployment.md -- rather than left for the next reader to find during an incident. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
e12cc5810e |
docs(events): say why each mechanism is here, after a scope review (BUG-2736)
Codex round 7 asked the question I do not reliably ask of my own work: should each of these mechanisms be in this change at all. Five findings, all DECLINED, and the reasons are worth having in the artifacts rather than only in a review log. Two were already the lead's explicit scope for this unit and are not mine to re-open: the two-phase rollout, and removing web's unread id?: number field while it is still unread. One I decline on the argument rather than the authority. The atomic publish script is not an ordering improvement bundled into an ID-space change: the interleave it closes is older than this diff and was merely wrong, but this diff makes it HARMFUL, because counter-backwards detection reads a descending ID as a reset and would fire on every ordinary interleave. And the dedupe token is required BY the script for the same kind of reason -- phase 1 retries a PUBLISH whose payload already carries its ID, so a duplicate arrives under the SAME ID; phase 2's retry re-runs the assignment, so it arrives under a SECOND one, ascending and indistinguishable. Moving assignment into the script is what makes retries worse. Cutting the token while keeping the script would ship a regression. That reasoning is now in the script's comment, where the next person asking this question will find it. One I decline as completing a fix rather than extending scope: the lower-generation recovery exists only because this diff's own straggler rule created a discard-forever state. Cutting it would leave a new unbounded silent failure in a unit whose entire subject is not failing silently. And one is a framing problem rather than a scope problem, which is the useful half of the round. The migration is a substantial MITIGATION and not a closure: it stops a replica mixing two ID spaces in one buffer, and it does not make a client's cursor say which space it came from. That was stated at the end of the deployment section, after the procedure; it is now stated before it, because a reader deciding whether to run the migration should meet the limit before the steps, not after. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
417776ce9b |
fix(events): recover when the generation counter goes backwards and stays (BUG-2736)
Codex round 6 walked four realistic scenarios through the code line by line. Three of its findings were already-filed or already-documented residuals; one was a hole my own round-3 fix had opened. THE HOLE. Round 3 made a LOWER generation mean "a straggler from a space we have left" and discarded the message. That is right for a message in flight at the instant of a rotation. It is wrong, and unrecoverable, for a Redis failover to a replica whose copy of the generation counter predates the rotation: every publisher then mints from the lower number, and this bus discarded every message forever -- nothing delivered, nothing buffered, and the only trace a log line per message. Silent and unbounded is the one outcome this family refuses, and round 3 had traded a loud bounded problem for it without noticing. A persistent regression is now ACCEPTED as a new space: buffers dropped, next resume answered sync_required, delivery resumes. Loud and recoverable. The discriminator is a physical quantity rather than a guess about intent -- a straggler is bounded by pub/sub delivery latency, so a lower generation arriving long after the adoption cannot be one. Both ways of being wrong are loud: too short costs an extra buffer drop, too long costs a few seconds of discards before recovery. It gets its own reason label, epoch_regressed, because an operator acts on it differently from every other reason here: the others are expected, this one means Redis lost writes. The metrics test now drives every reason with DIFFERENT counts, so an adapter that collapsed them onto one series fails there instead of in production. ALSO RECORDED RATHER THAN FIXED, because the review found the claim overstated rather than the code wrong: the publish dedupe token is as durable as Redis replication and no more. A retry that lands on a promoted replica which never received the token publishes a second copy under a second ID, and nothing downstream can tell the two apart. The comment said the token turns a retry into a no-op; it now says which retry. The other three scenario findings are pre-existing and filed: the subscribe-then-replay duplicate window is BUG-2730 and is documented at the site it happens; the empty-buffer replica that serves an adjacent cursor across a cutover is the residual this unit's own comment already names, with the numeric-base design that closes it on BUG-2736's trail; and a held-open SSE connection is not told about a gap detected under it, which is BUG-2730's family too. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
736a8c48f7 |
docs: thirteen claims about code that had moved under them (BUG-2736)
Codex round 5, cross-artifact consistency. Every one was a claim in a comment, help string, doc, or test name that the code no longer supported — and this diff created most of them by moving the code. The ones that would have misled an operator: - pad_event_sequence_resets_total documented ONE reason in both the Go doc comment and the Prometheus help text, and the deployment table said the same. It has emitted three since this branch. An operator reading the help string to build an alert would have alerted on a third of the signal. - deployment.md said every published message carries an epoch prefix. Phase 1 publishes bare JSON — which is the entire point of having two phases. - deployment.md said the first flipped message reaches each replica and every resuming client gets sync_required. A replica learns the epoch only from a message it RECEIVES, so only replicas subscribed to a workspace with traffic see it; and a replica with empty buffers adopts without dropping or counting, deliberately. - deployment.md said a restart's IDs cannot collide. internal/idspace documents a bounded case — the earlier process publishing more than 2^20 events per millisecond of its life. Stated as the bound it is, with the backwards-clock direction named as the safe one. - cmd_watch.go described sync_required as eviction-only. It has had four other causes since BUG-2731 and gained a fifth here. The ones that would have misled the next person editing this code: - bus.go said the Redis half was unwritten and a reset counter could still merge two ID spaces. It is written, three commits back on this branch. - bus.go and watchevents.go said in-memory IDs restart from 1. They count from an incarnation base. - redis_bus.go described this bus's epoch as an opaque uuid equivalent to the watch bus's, twice, after round 3 made it a Redis-minted generation. Only the watch bus still uses uuids. - observer.go said counter_backward happens only during mixed-version rolls. Phase 1's two-call publish produces it in steady state too. - redisns.go said the publish script spans four keys (it is five here now, plus a two-key assign script), and its hand-kept reserved-name inventory never gained event_epoch or event_epoch_gen — so a namespace equal to either would have nested one installation inside another's keyspace unrefused. - A test comment referenced idIncarnationShift, which moved to internal/idspace.Shift when the package was extracted. - Two tests called themselves process-restart tests while constructing successive buses in one process. They test bus incarnations; the comment now says so and says why that is the equivalent thing. And one reasoning error rather than a stale fact: the counter-backwards branch justified raising the floor by asserting the arriving ID is necessarily in the SAME numeric space. It is not — a phase-1 counter reset publishes low IDs with no epoch to explain them, which is a NEW space we cannot see. The behaviour is unchanged and still correct (the lead's day-52 ruling: raise unconditionally, prefer a loud bounded resync loop to a silent skip), but it now says what it actually knows, which is nothing, and names the cost on a real phase-1 reset. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
d393126d80 |
test(events): close the gaps a tests-as-production-code pass found (BUG-2736)
Codex round 4, on the tests themselves. Eight findings, all real; three of them were behaviours in this diff with no test at all. NO TEST AT ALL: - The real receive path. Every reconciliation test drove fanOutFromRedis directly and the publish tests read the wire with a raw subscriber, so a regression that decoded the epoch correctly and then handed 0 to the fan-out would have passed all of them -- reconciliation silently never running in production. Now driven through Subscribe/Publish and back through Redis, with the mutation checked. - The atomic script's ordering claim. Every phase-2 test published once or ran the script sequentially, so a two-call INCR-then-PUBLISH implementation passed them all -- and that ordering is load-bearing, because the receive path reads a descending id as a counter reset. 300 concurrent publishes now assert arrival order equals id order; verified to FAIL 5 of 5 against a two-call implementation and pass 3 of 3 against the script, so the instrument discriminates rather than merely being green. - The TOML tag. The env-var test proved PAD_EVENTS_PUBLISH_EPOCH reaches the field and said nothing about the toml:"events_publish_epoch" tag -- the exact form the rollback procedure warns about, since a file value outlives an unset env var. PASSING FOR THE WRONG REASON: - The production config wiring was still unexecuted: passing an empty config.Config at both RunE call sites compiled and passed everything. The source-text guard that already counts those call sites now also requires them to pass the loaded config. - The phase-2 wire assertions accepted a well-formed payload with an empty event body. They now assert the body survives. - The Redis metrics subtest had no served-resume control, so a bus that refused every resume would have passed. It now round-trips a publish through Redis first. - TestResumeGapIsReportedForBothWaysOfNotServing never proved ws-warm HAD a buffer, so its second half could silently duplicate its first. - internal/server's cold-resume tests still sent a literal 4200, which the incarnation guard now answers before the handler's no-buffer path is reached. My own round-1 sweep of this class stopped at four packages and never looked at internal/server: reviewer-named instances are a sample (team CONVE-18), and so, evidently, are self-named ones. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
c3d485d136 |
fix(events): make the ID space's epoch a monotonic generation (BUG-2736)
Codex round 3, on concurrency. Two findings, and the first says the epoch's TYPE was wrong. AN OPAQUE EPOCH CANNOT BE ORDERED. Each workspace has its own Redis subscription and its own receive goroutine, and Redis orders messages within a channel but not across them. So a message published BEFORE a rotation, on workspace A's channel, can arrive AFTER the rotation was already learned from workspace B's -- and with a uuid there is no way to tell that straggler from a second rotation. The bus flipped back into the dead space, dropped every buffer again, and the "at most one drop per instance per roll" property this unit claimed was simply false. The epoch is now a generation number minted by Redis (INCR on a counter that Pad never deletes), so the two spaces are comparable. A HIGHER generation is adopted; an EQUAL one is steady state; a LOWER one is a straggler from a space we have left, and its message is DISCARDED rather than delivered -- its id belongs to the dead sequence, so buffering it would put two spaces in one buffer, and its subscribers were already told to resync across the change. A wall clock was the other way to order them and is the wrong one: instances have different clocks, so a rotation minted on a lagging machine could carry a lower stamp than the space it replaces and be ignored forever. That is a silent failure where this is a loud one. Minting inside the script also removes the propose-then-SET-NX race: two publishers can no longer both believe they minted the space. THE SECOND FINDING was a TOCTOU in yesterday's phase-1 stale-epoch clear: INCR and DEL as two commands leave a window in which a concurrent flipped publisher mints an epoch between them, and we delete a LIVE one. Phase-1 assignment is now a two-line script, so the restart and the clear are one atomic step. The wire form it publishes is unchanged -- still bare JSON with the id inside, which is the whole point of phase 1. decodePayload now refuses a zero or negative generation. Zero is this package's sentinel for "no ID-space information", so a malformed publisher carrying it would make every receiver stop reconciling while looking healthy. Mutation matrix: 6 applied, 6 caught -- straggler adopted, adoption weakened to any-difference, straggler ignored but still buffered, the phase-1 clear removed, the generation minted as a constant, and the zero-generation guard removed. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
94cc2492fc |
fix(events): a phase-1 counter restart must not leave a live epoch behind (BUG-2736)
Codex round 2, on the rollout angle. Four findings; one was a real silent-loss hole and three were claims in the docs and config comment that the code does not support. THE HOLE. Phase 2 mints an epoch and the counter climbs; the deployment rolls back to phase 1; the seq key is then evicted or deleted; phase-1 publishers climb from 1 again; phase 2 is re-enabled and its SET NX finds the OLD epoch still there. A receiver that had adopted it sees no change, and if its high-water mark is below the new sequence -- a replica that just started, or one whose buffers were empty -- the numeric check does not see the reset either. Two ID spaces merge in one buffer silently, which is the outcome this whole unit exists to prevent. Phase 2's rotation cannot cover it: that rotation fires when the SCRIPT's own INCR returns 1, and by then the counter has climbed past 1 under the phase-1 path. So phase 1 now deletes the epoch when its own INCR returns 1. Deleting rather than rotating, because that path publishes no epoch and has none to propose, and an absent key is what phase 2's SET NX expects. The cost is one extra buffer drop if a phase-1 publisher deletes an epoch a flipped publisher just minted during the phase-2 roll -- loud and bounded, which is the direction this family always chooses over a silent merge. THE THREE CLAIMS. - "Rolling back is symmetric: unset the variable and roll" was true only of the roll back to PHASE 1. Downgrading past it is a second step in reverse order, because a pre-phase-1 binary still cannot parse the prefix, and introducing one while any flipped instance publishes drops events on it. - Unsetting the environment variable is not the same as setting the value false: events_publish_epoch can come from config.toml, whose value stands when the variable is absent. - counter_backward was documented as expected during mixed-version rolls and near zero between them. On phase 1 it can be non-zero at any time: that path keeps the two-call INCR-then-PUBLISH, so instances can interleave. The expectation is now stated per phase. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
a9544a57ba |
test(events): repair the resume tests the base guard made vacuous (BUG-2736)
Codex round 1 named three sites; the class was six, across four packages. Every MemoryBus test that spelled out a cursor as a small literal now passes through the incarnation guard before reaching the branch it is named for. The worst were the two that exist precisely to distinguish branches: the both-ways-of-not-serving observer test would have gone green with BOTH of its branches deleted, and the watch bus's eviction test would have gone green with eviction deleted. Cursors are now base-relative or read back from what the bus issued. Where the test has only the EventBus interface and no access to the base, the cursor is derived from a published event's id instead. Mutation-checked in the direction that matters: with the no-buffer branch, the coverage check, and the eviction check each made inert in turn, the tests named for them fail. Before this commit they did not. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
4a6a748c85 |
feat(events): identify the shared Redis ID space, behind a two-phase flip (BUG-2736)
The activity event counter lives in Redis and is shared by every instance, so no instance can compute an identity for it the way MemoryBus computes its own incarnation base. If that counter is ever reset -- evicted under maxmemory, deleted by hand, a fresh Redis after a restore -- IDs start again from 1, and a replica buffering the old sequence cannot tell the new 101 from the old 101. It merges two ID spaces into one replay buffer and answers a resume across the boundary as though nothing was missed. Numeric detection alone cannot see it. By the time the new sequence passes the replica's high-water mark it looks like ordinary progress -- which is the case the epoch exists for, and the high-water check is what catches the OTHER case (a publisher that never learned the epoch), so both are kept. So the identity travels WITH each message, as an opaque token in a "<epoch>|<id>|<json>" prefix. A prefix rather than an envelope field: an older instance would unmarshal an envelope object SILENTLY -- no matching keys, no error, a zero-valued Event delivered to its clients -- and fails loudly on the prefix instead. TWO PHASES, because the failure is asymmetric. Every instance ACCEPTS both wire forms from this release; only emission is gated, on PAD_EVENTS_PUBLISH_EPOCH. Phase 1 rolls the binary everywhere publishing the historical bare JSON; phase 2 sets the flag and rolls again. Flipping before every instance is upgraded is the one direction that LOSES events rather than resyncing: a pre-phase-1 binary cannot parse the prefix at all. Rollback is symmetric and safe. docs/deployment.md carries the procedure both ways, what the reset counters should read during each roll, and what remains unfixed. Phase 2 also moves ID assignment into one atomic script. The two-call INCR-then-PUBLISH lets two instances interleave, so a receiving instance can append 6 before 5 -- a window older than this change, and already wrong, but load-bearing here because counter-backwards detection reads a descending ID as a reset. The script carries a dedupe token for the same reason internal/watchevents' does: go-redis retries a command whose REPLY was lost, so a publish can happen AND return an error, and the retry would deliver a second copy that looks perfectly valid. THE COUNTER-BACKWARDS FLOOR STAYS, and the earlier hope that this unit would delete it was wrong. Its trigger is mixed-VERSION ordering -- an older binary assigning and publishing in two calls -- not mixed-FORMAT payloads, so publish-old-until-flip removes the format window only. It lives for as long as a deployment can run two publisher versions at once, which is every rolling upgrade, and the code now says so where it fires. THE ASYMMETRY WITH MemoryBus IS DECLARED IN BOTH BUSES, in both packages: an opaque epoch where the counter is shared, a numeric base where one process owns it. They are not two spellings of one idea and must not be symmetrized. A numeric base for Redis would close more -- it would refuse cross-incarnation cursors, which the epoch cannot -- and is deferred rather than rejected: at the flip, IDs would jump to ~1.8e18 in one step and every un-flipped publisher's message would read as a massive backwards jump, dropping every buffer across the whole roll. It is a candidate follow-on once the flip has soaked. What this does NOT fix is stated in the code and the docs rather than implied: the client cursor is still a bare integer with no epoch, so an old and a new ID of the same value remain indistinguishable TO A RESUME even though the buffers can no longer mix them. The flip is read inside newObservedEventBus, which now takes the whole Config. As a hand-picked argument at the two RunE call sites it was untested wiring: replacing it with `false` compiled, passed the entire tree, and left the deployment silently on phase 1 -- indistinguishable from a correct phase-1 deployment, since phase 1 is the default. Mutation-checked in both directions, because a helper that ignores its config and hardcodes either value would pass a one-directional test. Also: the epoch and dedupe keys join the namespace assertions (an epoch shared between two installations is a cross-feed with teeth -- each would read the other's ID-space changes as its own), and this package's four-key EVAL is now recorded on BUG-2724's cluster deferral, which had one call site and now has two. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
c017ad359d |
fix(events): give each in-memory bus incarnation its own ID space (BUG-2736)
Both in-process buses assigned Last-Event-ID values from a counter that restarted at 1 on every process start. A client holding cursor 2 from a previous incarnation could reconnect to a restarted server, pass every coverage check BUG-2731 added, and be replayed the NEW space's 3, 4, 5 as though they followed the OLD space's 2 -- silently missing everything the dead space held above 2. Nothing local could tell the two 2s apart. The cursor carries no epoch, and in internal/events per-workspace IDs are non-consecutive by construction, so "did we issue this ID?" was numerically undecidable. The four adjacent levers were checked rather than assumed: comparing in memory has nothing to compare against; persisting the counter makes single-process Pad carry durable event-bus state and still resets on data loss; refusing cursors we did not issue is the undecidable one; and a nonce on a second channel is unavailable because EventSource echoes Last-Event-ID and nothing else, and cannot rewrite its URL on an automatic reconnect. So the ID space's identity goes in the ID's VALUE while its FORMAT is unchanged: still a bare int64, still ParseInt on the way back. internal/idspace mints a base of processStartUnixMilli<<20 and each bus counts up from it. Two incarnations can only collide if the earlier process published more than 2^20 events per millisecond of its own lifetime -- a deterministic bound, not the probabilistic one BUG-2736's body rules out. A CAS makes bases strictly increasing within a process too, which the clock alone does not do for two buses constructed in the same millisecond. A backwards clock step degrades in the SAFE direction: a lower base puts old cursors ABOVE the new buffer's newest ID, so they are refused rather than answered wrongly. The overflow bound is computed, not estimated: the last start instant that fits is 2248-09-26T15:10:22Z. Each bus then answers the resume question exactly instead of inferring it: a non-zero cursor at or below this incarnation's base was issued by a dead space. That is strictly stronger than the coverage check alone, which serves the ADJACENT cursor on reasoning that only holds within one ID space. In internal/watchevents the check lives in one helper both entry points call. Written inline in EventsSince it was absent from SubscribeAndReplaySince -- the path the SSE handler actually uses -- so the component was fixed and its wiring was not (team CONVE-19). A test now drives both. web's ItemEvent no longer declares `id?: number`. Nothing read it, which is the only reason it was harmless; a base of ~1.8e18 is past JavaScript's MAX_SAFE_INTEGER, so the first reader would have silently got a rounded number. Defused while still unread. Tests that spelled out IDs now read back what the bus assigned -- a literal 1 is a cursor from a dead space, which turned two negative controls into their own opposite. The two watchevents guards (cold buffer, dead incarnation) are tested separately, because a single test covering both would keep passing with either deleted. The Redis half is not here. Its counter is shared across processes, so identifying its ID space needs an epoch travelling with each message; that is the next commit on this branch. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
bc422d0e31 |
Merge pull request #1177 from PerpetualSoftware/fix/events-resume-coverage
fix(events): a resume must not be answered from coverage we never had (BUG-2731) |
||
|
xarmian
|
86b0f7508f |
docs(server): name the subscribe-then-replay window where it lives (BUG-2730)
Codex round 18 was asked to assume exactly one defect survived sixteen rounds and to find it rather than survey. What it returned is the subscribe-then-replay duplicate window — a REAL defect, and one already filed on BUG-2730 by two earlier rounds. That it went that deep and surfaced a known residual rather than a new defect is the useful result. But the code said nothing at the site, so a successor reading handleSSE would have to re-derive it, exactly as three review rounds did. Now stated where it happens: the window, what it costs (a duplicate toast and duplicate work, never a lost event), how internal/watchevents closed the same window with SubscribeAndReplaySince, and why closing it here is its own unit — a new method on events.EventBus across three implementations, folded together with the admission check SubscribeIfAllowed already performs. That is a change about DELIVERY, and this one is about COVERAGE. No behaviour change. Refs BUG-2730, BUG-2731 |
||
|
xarmian
|
f2a037e393 |
docs: nine claims about other people's code that I had not checked (BUG-2731)
Codex round 16, aimed at every factual assertion this diff makes about
code OUTSIDE it — go-redis, the SSE spec, HTTP header handling, the web
client, internal/watchevents, Prometheus. The angle was chosen because
this diff had already been caught twice asserting library behaviour that
was false, and claims about other people's code are the one class no test
in this repo can falsify.
It found nine. Every one is mine, and every one claimed more than I had
verified.
- "no reconnect in 24 seconds of probing" cited an experiment that is
not in the tree — the probe was deleted with the test it belonged to.
The MECHANISM is checkable from the library source and now says so
with the call named; the unretained number is gone.
- "the SSE `id:` field has no room for an ID-space identity" is wrong.
The spec allows an arbitrary UTF-8 event ID. What excludes it is PAD's
own contract — an int64 every deployed client already parses — which
is a stronger and more honest statement of the constraint, and it is
the one BUG-2736 has to argue against.
- "the spec defines an empty header as no position" overstated it. The
spec governs what a client SENDS. What a server does with a value it
cannot use is our policy, and the test now says so.
- "HTTP strips optional whitespace from header values" is too broad: Go
trims on the way OUT, while the incoming MIME parser only TrimLefts.
What I measured was the round trip, and the comment now claims exactly
that.
- "every gap is a full resync / full re-fetch" is wrong in three places.
The web client answers sync_required with an incremental /changes
delta and only falls back to a full refresh after a long absence or a
failure. This one matters beyond wording: the load argument for the
whole fix rests on what a gap costs a client.
- "a wrapper cannot see that a resume gap occurred" — it can see the nil;
what it cannot see is WHY. I had already corrected this in the metrics
adapter and left the overbroad version in the seam it describes.
- internal/watchevents' `since` no longer "mirrors internal/events
exactly" — that stopped being true when knownFrom went into the
latter's `since`. Now states where the two differ and why.
- "the counter returns to baseline" — a Prometheus counter only
increases; its RATE returns to baseline. Two places.
- "the only case where INCR fails while PUBLISH still reaches
subscribers" — an ACL permitting one and denying the other is another.
The test now names the SHAPE as what matters and its arrangement as
one route to it.
No behaviour changes; comments, docs and test prose only.
Separately verified while waiting on this round, and now cited rather than
asserted: the three WHATWG steps that make the empty `id:` cursor
retirement work. That claim was the one thing in the diff I had taken from
memory of a spec rather than read, and it is load-bearing — if wrong, the
feature is theatre.
Refs BUG-2731
|
||
|
xarmian
|
3c6b412b0f |
refactor(events): let go-redis own the redial, and record what neither form detects (BUG-2731)
Codex round 15 raised a P1 against the reconnect handling: a bare pubsub.Receive loop does not start go-redis's health check, which only the Channel* constructors do, so a HALF-OPEN connection — no FIN, no RST, just a route that stopped working — blocks forever with no error, no retry and no coverage drop. That is the exact failure the reconnect handling exists to prevent, reintroduced one layer down by the mechanism meant to fix it. The diagnosis is right. The remedy is not, and the difference is measured rather than argued. PubSub.Ping ONLY WRITES the command — writeCmd, then return; it never reads a reply (go-redis v9.22.0, pubsub.go). So the health check's pingErr is nil for as long as the socket accepts writes, which a half-open socket does until its send buffer fills, and the channel path sets no read deadline. Probed with a TCP proxy that silently stopped forwarding, with ChannelWithSubscriptions in use and the health check running: NO RECONNECT IN 24 SECONDS. A test asserting detection was written, failed, and was deleted rather than left in a shape that cannot pass. So this switches to ChannelWithSubscriptions on the grounds that survive the measurement — it is the supported API, go-redis owns the redial and backoff, and a hand-rolled retry loop is machinery this package does not need to maintain — while the function's comment now states plainly that NEITHER form detects a half-open connection, with the mechanism and the probe result, so nobody re-derives the health check as a solution. The residual is filed on BUG-2730, which already owns "this bus is under- delivering and cannot say so", together with what would actually work (application-level idle tracking) and why it is a decision rather than a patch: it needs a threshold, and too low a threshold resyncs quiet workspaces for no reason — the load-posture inversion this family keeps having to avoid. Mutation: removing the coverage drop on resubscription fails two tests, so the detection still discriminates through the new API. Refs BUG-2731, BUG-2730 |
||
|
xarmian
|
5cbeb52784 |
docs(events): three claims the split left describing code that is gone (BUG-2731)
Codex round 14, a post-split damage review. Two findings, both stale
prose, which is the failure mode a SUBTRACTION has: the code is correct and
the comments describe the version that was removed.
- fanOut still credited "publishScript" for the ID. The activity bus does
INCR then PUBLISH again; the script went to the migration.
- replayBuffer.knownFrom and handleSSE both listed an ID-space reset among
the things that invalidate coverage. That detector went with the
migration, so a reset Redis counter can still leave two incarnations'
IDs in one buffer.
The second matters more than a wording slip: it claimed a guarantee the
remaining code does not provide, which is the shape that gets a successor
to trust a boundary that is not there. Both now NAME the omission and point
at BUG-2736 instead of implying coverage they do not have — the same
boundary-declaration the knownFrom comment already carries.
Codex's verdict on the rest, worth recording because it is what the gate
was for: "the replay-coverage logic and corrected tests are otherwise
coherent on their own."
Refs BUG-2731, BUG-2736
|
||
|
xarmian
|
8ef40691b9 |
test(events): correct an arrangement the split made vacuous (BUG-2731)
Re-running the mutation battery on the split tree — which is the point of re-running it, since a subtraction hides its defects as absences — found one survivor. TestAFailedPublishDeliversNothing killed Redis outright to force the publish-failure branch. That discriminated while an atomic script assigned the ID; after the split, Publish does INCR then PUBLISH, and with Redis down BOTH fail — so "returns early" and "continues with a fallback ID and fails at PUBLISH" look identical from the outside. Restoring the local-counter fallback left the whole suite green. The arrangement now reproduces the PARTIAL failure the branch exists for: the sequence key holds a non-integer, so INCR fails while PUBLISH keeps working. That is a realistic shape — a key-type collision with another tenant of the same Redis — and it is the only one where a fallback ID would actually reach subscribers. Re-run against the mutation, it fails immediately. The replay-buffer assertion comes back with it. It had been dropped because killing Redis also kills the subscription, so an empty buffer was the reconnect handling working rather than evidence about the publish; under a healthy subscription it is evidence again. Post-split mutation battery: 6 applied, 6 caught after this fix (5 of 6 before it). Also moves the flap-vs-keyspace justification for the reset counter's label into the metric's own doc, where an operator reading the metric will find it. Refs BUG-2731 |
||
|
xarmian
|
b4989aa2f0 |
fix(server): retire a cursor we just refused, and stop trusting one we cannot read (BUG-2731)
Three handler changes and the documentation the coverage fix made wrong. sync_required NOW RETIRES THE CLIENT'S CURSOR, carrying an empty `id:` which per the EventSource spec clears the last event ID. Without it the client keeps the cursor that was just declared unservable, so every later reconnect on a quiet workspace is answered sync_required again and re-runs a full delta sync — a loop that only ends when a live event happens to arrive. Survivable while the response was rare (buffer eviction only); the coverage check makes it common, so this is a load consequence of that fix and belongs to it. AN UNREADABLE Last-Event-ID IS A GAP, not a fresh connection. Only a parseable positive value reached the replay path, so "-1", "not-a-number", a quoted number, or an integer too large for int64 silently dropped everything published before that subscription. The same lie this fix exists to end, arriving through the parser rather than the buffer. A genuinely fresh client sends no header and is unaffected — asserted, because the fix is one `if` away from resyncing everyone on connect. Not a case, and the test says why rather than omitting it silently: a whitespace-only value. HTTP strips optional whitespace from header values, so the handler sees an empty string, which the spec defines as "no position". Measured, not assumed. HANDLER-LEVEL GAPS ARE COUNTED. A cursor no one can parse never reaches a bus, so without Server.countResumeGap the counters would undercount exactly the resyncs an operator is most likely to be asked about: a client looping on a cursor nobody can read. BOTH SSE HANDLERS GET ALL THREE, because introducing them on one stream is how parallel surfaces silently diverge. The pad CLI masks the cursor difference by clearing its own — verified by reading its parser, which handles the empty-value form — so the consumer this would bite is a generic SSE client, the one nobody tests. DOCS. Two comments described mechanisms that had changed: the handler's own "gap too large — buffer evicted" (eviction is now one of several) and internal/config's claim that the activity stream silently misses a namespace cutover. And docs/deployment.md's cutover note said resync is honest on the watch stream and silent on the activity one; it is now honest on both, with the edge that a cursor exactly one below a replica's first-seen ID is served rather than refused, tracked as BUG-2736. The sync_required reason text changes from "Event buffer exceeded" to what actually happened. Keeping it was defended earlier BECAUSE the client never reads it, which is the same reason correcting it is free. Refs BUG-2731 |
||
|
xarmian
|
3ba5dcc836 |
feat(events): report unservable resumes and coverage resets (BUG-2731)
The conditions the coverage fix added are invisible from outside the bus,
so nothing could tell an operator they were happening. A resume the bus
refused is a nil return — a wrapper can SEE that nil but not why, or that
it happened at all, without reimplementing the coverage rules it wraps —
and a coverage reset is detected on the receive path, which no caller
invokes.
That is why this is an Observer seam and not more of
metrics.InstrumentedBus, which already wraps this bus. The split is by what
each can see, and the package doc says so, because "why is this
instrumented twice" has an obvious wrong answer.
Three counters:
- pad_event_resume_gaps_total. Expect a step around a deploy and a return
to baseline; each instance starts with no coverage. It counts RESUMES,
not clients, so a deploy nobody reconnects through does not move it. A
rate that does NOT settle is the evidence against this fix's central
claim — that the syncs it adds are only the warranted ones — which is
what makes the claim falsifiable in production rather than only in
tests.
- pad_event_sequence_resets_total{reason}. One reason today,
subscription_resumed. Labelled from the start rather than shipped bare,
because BUG-2736's ID-space work adds reasons an operator diagnoses
completely differently, and an alert built on an unlabelled total would
have to be rewritten when they arrive.
- pad_event_receive_loop_exits_total. Unlike the watch stream's twin this
does NOT stay at zero — one loop per workspace, so it fires whenever a
workspace's last local subscriber leaves. Read as a rate.
The resume counter is reported in ONE place, on the way out of EventsSince,
so both ways of failing to serve — no buffer at all, and a buffer refusing
the span — reach it structurally rather than by remembering to increment at
each return. That is the exact shape the watch stream's own gap counter got
wrong. internal/watchevents.MemoryBus also gains the reporting its RedisBus
already had, so that counter stops reading zero on single-process
deployments for a path that genuinely resyncs clients.
Wired on MemoryBus too, not only RedisBus: a single-process deployment
restarts, and the cold-buffer gap is as real there.
newObservedEventBus exists so the WIRING is testable (CONVE-19). Inline in
the command's RunE, the SetObserver call was a claim no test could reach —
the events test attaches its own observer and so does the metrics test, so
both pass with the production line deleted. Extracting it also had to be
done without hollowing out the BUG-2724 keyspace guard, which reads
cmd_server.go: it now checks both that the constructor takes the shared
value and that every call to the helper passes it, with the helper's own
declaration excluded from the call count.
Refs BUG-2731
|
||
|
xarmian
|
9f88e94832 |
fix(events): a resume must not be answered from coverage we never had (BUG-2731)
internal/events answered a Last-Event-ID resume with an empty-but-non-nil
slice whenever the workspace's replay buffer could not speak to the span
being asked about. The SSE handler reads that as "caught up", so the client
sat on a live stream believing it was current while everything between its
cursor and now was silently gone.
COVERAGE. replayBuffer gains knownFrom: the lowest event ID from which this
instance's coverage of a workspace can be vouched for. A resume from below
it answers nil, which the handler already turns into sync_required. Covers
a buffer that does not exist (cold start, restart, scale-up, or simply the
first connection to a workspace on this instance), a buffer that exists but
starts above the cursor — NOT full and NOT empty, reachable on any
multi-instance deployment with no eviction and no restart — and a non-zero
cursor from a previous incarnation of a single process.
knownFrom here means RECEIVING-continuity, never ID-contiguity, and the
defining comment says so with the measurement attached.
internal/watchevents has a field of the same name that ALSO detects holes
by noticing a non-consecutive ID; porting that would have been a serious
regression, because this bus has a global counter and per-workspace
buffers, so a workspace's buffer holds non-consecutive IDs by construction
(four publishes alternating across two workspaces measure as W=[1 4],
X=[2 3]). An ID-contiguity check would fire on nearly every append and turn
every resume into sync_required — the false-positive inversion of this bug.
LIFECYCLE. Coverage now ends where it really ends:
- a stopped workspace subscription drops its replay buffer. Keeping it
"in case they come back" looks like a free win and is the bug: events
published elsewhere never enter it while it goes on looking complete.
- subscriptions are generation-numbered, so a straggler from an ended
subscription cannot re-create a buffer and vouch for coverage that
ended with it — including the case where the workspace has already been
resubscribed under the stale goroutine.
- a pub/sub reconnect ends that workspace's coverage. PubSub.Channel
resubscribes transparently, so a Redis failover left a hole the buffer
had no idea about; the loop reads pubsub.Receive instead. It must
RECOVER rather than exit — returning on a transient error would leave
an instance publishing fine and receiving nothing — and it drops ONE
workspace's buffer, since a dropped subscription says nothing about any
other channel.
Subscribers are indexed by workspace because the replay buffers moved under
the same mutex (necessary for the straggler race): scanning every local
subscriber under that lock would make one hot workspace the serialization
point for every other workspace's fan-out and every resume.
Also removes Publish's local-counter fallback on a failed INCR, which
minted an ID from a process-local space and published it — every receiving
instance reads that as the counter having been reset. It bought nothing:
this bus has no local fan-out path, so an event that does not reach Redis
reaches no subscriber here either.
SIBLING. internal/watchevents had the identical cold-resume defect on its
MemoryBus — its RedisBus guards it, MemoryBus reached the buffer directly —
so a single-process instance answered a post-restart resume as caught up.
Found by a cross-artifact review pass; the guard goes in `since` so both
implementations inherit it, and is tested through SubscribeAndReplaySince
as well as EventsSince because that is the path the handler uses.
Refs BUG-2731
|
||
|
xarmian
|
4f0cd65ca3 |
Merge pull request #1176 from PerpetualSoftware/fix/redis-operability
fix(redis): namespace, connection limits, and operational visibility (BUG-2724, BUG-2726, BUG-2727) |
||
|
xarmian
|
d6e153a583 |
fix(test): the new config test needed a writable HOME (Nix CI)
`config.Load()` resolves and CREATES ~/.pad, so TestStreamAndRedisEnvMapping passed on any machine with a writable HOME and failed in the Nix check sandbox, which sets HOME to an unwritable path: "Load: mkdir /homeless-shelter: permission denied". Every other test in this package already sets HOME to a t.TempDir() for exactly this reason. Mine did not, and no local gate could tell me — build, lint, `go test ./...` and `-race` all run with my HOME writable. Only the Nix leg exercises the sandboxed shape, which is the second time that leg has been the sole instrument for a class the whole local matrix is blind to. Verified both directions locally by compiling the test binary and running it under a read-only HOME rather than trusting CI to re-run: fixed passes, unfixed fails with the same message the sandbox reported. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
bb003dd6bb |
fix: five claims the final comment-truth round found (BUG-2724, BUG-2726)
The bounded process the lead set: N rounds, an author prune pass, one final comment-truth round. This is that round's output, and the loop stops here. Two were mechanisms I had wrong, and both are the kind a reader would reuse without re-deriving: - "Different Redis DB numbers do not help" was half true. Ordinary keys ARE DB-scoped, so two installations on different DBs keep separate presence registries; it is pub/sub that ignores DBs entirely, which is why the buses cross-feed regardless. Stating it as "does not help" made the namespace look like the only fix for a problem it only half is. - A namespace cutover's client resync was attributed to the epoch check. That check needs an OLD epoch to compare against and a freshly namespaced bus has none — the resync comes from the cold replay-buffer coverage check instead (knownFrom is zero, so every resume falls below it). Same honest outcome, different mechanism, and the mechanism is what someone reasoning about a cutover would use. Three were stale or over-general after earlier changes: the admission comment still said the global limit is passed to the bus as 0 (that parameter is gone), `pad watch --help` and the plugin monitor description lumped a missing .pad.toml's hourly retry in with the 5s-to-5min backoff, and CLAUDE.md said clients must back off without the browser exception docs/deployment.md spells out. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
461c5a3e3d |
refactor: prune the claim surface, and turn one prose claim into a test
The review could not converge on this diff's comments because each round of corrections re-expanded the surface it was reviewing — rounds 16 and 17 found errors inside 15 and 16's fixes. That is a production rate being measured, not a backlog being drained, so the treatment is to write fewer claims rather than review the same ones again. PRUNED, ~135 comment lines: process narration. "An earlier version said X", "found by mutation testing", "codex round N caught this", the scoreboards. Every one of those is already in a commit message, which is where the archaeology belongs; in the source they are claims a future reader has to verify, about a past that no longer exists. KEPT, because they earn it and a reader would otherwise re-derive them: metric semantics, reachability boundaries, what a test does and does not discriminate, why the obvious alternative was rejected, and the hazards that cannot be enforced in code. MOVED TO A TEST, per the rule this run earned the hard way: a comment asserting countable behaviour belongs in the suite. Two test comments in internal/watchevents relied on "this constructor waits for its SUBSCRIBE to be confirmed" — prose, and the same assumption applied to the OTHER bus (which subscribes asynchronously) is what made a namespace test flake. It is now asserted with no polling and no sleep, and the mutation that removes the wait fails it. That rule generalises and is why round 17's find mattered: "counts every unservable resume" was prose, so its falseness could hide a real metric gap. Prose is for claims that cannot be asserted. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
35e564298b |
fix: seven more prose claims, one real metric gap, and a flaky test of my own (codex round 17)
The prose angle again, and it is still finding things — which is itself the finding: this diff's comment density is generating wrong beliefs faster than the review is removing them, in the one dimension where the defect is a reader's understanding rather than the program's behaviour. Everything below was a claim I wrote. ONE WAS A REAL GAP, not just wording. pad_watchevents_resume_gaps_total was documented as counting every unservable resume, and counted only the half decided by the shared counter. The LOCAL half — a cursor below what this instance can vouch for, from a hole or a cold start — returns nil from replaySince, becomes sync_required for the client, and reported nothing. Now counted, on the deferred path so it fires with the lock released. Its test needed a second pass to be an instrument: the first version arranged a hole and asserted the counter moved, but the shared counter disagreed too, so resumeOutrunsLocalView reported and the mutation survived. It now sets the counter to AGREE with what the instance has seen, which is the only arrangement that isolates the local path. The prose corrections, swept by grep rather than by instance this time: - MemoryBus's comment said a single-process deployment never wires an observer. cmd_server wires one, deliberately — that is what makes the drop counter meaningful there, which is a claim I had just added elsewhere. - "Every write path works with Redis down" was too strong in three places. Push answers 503 for an unresolvable targeted push and 502 push_unconfirmed on publish failure — the paths whose job IS cross-instance delivery. - Presence-failure consequences were stated as certainties in four more places after round 16 fixed one. A failure means an error was REPORTED; Redis can fail a pipeline after applying it. - The deployment metrics table still described pad_eventbus_publish_total as "Events published" after the Help string had been corrected to attempts. - The reserved-namespace rationale called prefix nesting a "collision". It is nesting; an exact collision would need the namespace to match a workspace UUID. Refused anyway, and now for the reason that is true. - A presence cutover was described as stranding one renewal interval of stale entries. It is the full 90s TTL — three intervals. AND A FLAKE OF MY OWN, caught by the full suite rather than by the targeted runs: the activity-bus namespace test asserted subscription state immediately, but that bus subscribes ASYNCHRONOUSLY (the watch bus waits for confirmation; the two differ). It now polls, and the asymmetry is named in both tests so the next reader does not assume symmetry the way I did. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
2fa1316853 |
fix: three claims round 15's corrections got wrong or missed (codex round 16)
Reviewing the corrections found three more, which is the honest shape of this: the prose angle keeps paying because the errors are in prose. - My round-15 correction said the Redis counters "stay at zero" on a single-process binary. That is wrong for one of them: pad_watchevents_notifications_dropped_total moves there, because MemoryBus has the same slow-subscriber drop and is wired to the same observer. So the comment was wrong before AND after, in opposite directions. It now says which counters are Redis-only by construction (everything sequence-related — MemoryBus assigns contiguous ids and has no subscription to lose) and which are not, and a test pins both halves. - "The three keyspaces cannot drift" survived in cmd_server.go. Round 15 fixed the copy in redisns.go and not this one — a two-member class, fixed one member, which is team CONVE-18 for the second time in this branch. - The presence-failure consequences were stated as certainties. Redis can fail a pipeline or a script AFTER it applied, so a failure means the operation reported an error, not that it did not happen. Now phrased as what a failure risks. Codex's reserved-namespace audit came back complete: the set covers every current suffix root (watchevents:pub: is covered by watchevents), every configuration path goes through Parse, all three production constructors receive the parsed value, and the exact-match controls do not reject names that merely contain a reserved word. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
7c8ed3c815 |
fix: nine false or overstated claims in this diff's own prose (codex round 15)
An angle worth naming, because it found more than several code-shaped
ones did: check the COMMENTS against the CODE. This diff is
comment-heavy and its comments make specific factual claims. Nine were
wrong.
The one that mattered most was a false argument for a correct rule.
redisns.Parse rejects colons, and justified it with a collision example
that does not happen: ns "a:events" builds pad:a:events:events:<ws>, not
pad:a:events:<ws>, because the suffix is appended too. The rule stands on
its own grounds (a colon spans segments and makes the keyspace ambiguous
to read back) — but a false example is worse than none, because the next
reader trusts it.
Chasing that turned up a REAL collision needing no colon: a namespace
equal to one of Pad's own first segments nests this installation inside
the default one's keyspace. Namespace "events" puts every key under
pad:events:*, which is the default installation's activity channel space
— the exact cross-feed the namespace exists to prevent, arriving through
the namespace. Now rejected, with a control leg asserting that names
merely CONTAINING a reserved word ("events-eu", "prod-session") stay
valid.
The other eight:
- "The three keyspaces cannot drift" — overstated. Each constructor takes
its own Keys; a source-reading test is what enforces it, which is
weaker than a compiler and now says so.
- Two docs claimed both SSE endpoints incur a presence registration. Only
the watch stream registers.
- The Redis metrics section said they "stay at zero" without Redis, while
pad_redis_up is deliberately unregistered — the section contradicted
the field three lines below it.
- The presence-failure metric's HELP string still carried the blanket
"leaves sessions unlisted and untargetable" that the field comment had
already been corrected away from. Two of the four ops fail in the
opposite direction.
- A nil from MGET was described as proof the process died. Eviction, a
restart and a manual DEL produce the same nil, and this file's own doc
says eviction is indistinguishable from expiry.
- A test comment claimed to cover both corrupt-entry shapes; the second
is unreachable and the subtest is deliberately absent, as the note ten
lines down already said.
- "Enumerates every refusal path" covered per-instance and per-workspace
and not per-user — the same undercount as round 13's, one round later.
Both per-user paths added.
- The Observer contract said a go-redis drop is reported as a sequence
gap. Only if a LATER notification arrives to expose the hole: drop the
newest message on a bus that then goes quiet and nothing is reported.
Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X
|
||
|
xarmian
|
9d54f24626 |
fix(server,cli): the half of round 12's fix I missed (BUG-2726)
Codex round 13, unanchored, found that my previous commit fixed one of the two refusal paths on /api/v1/events. The admission check moved above the SSE headers; the PER-WORKSPACE check stayed below them, so half the 429s on that endpoint still carried the JSON error envelope under Content-Type: text/event-stream — the exact defect the commit said it fixed. Team CONVE-18 in its own shape: the reviewer named one instance, I fixed that instance, and the class had two members. The enumeration I owed was "how many ways can this handler refuse", and it takes ten seconds to read. Every refusal is now above the header block, with a line saying nothing below it refuses. The contract test made the same omission and is the reason this reached another round: it drove the admission bound on both endpoints and never the per-workspace one, so it agreed with a handler that was half fixed. It now enumerates all three refusal paths, and the mutation that reintroduces the defect fails it by name. Also from round 13: `pad project watch`'s 429 message named the two knobs that cover both streams and omitted PAD_SSE_MAX_PER_WORKSPACE, which is the one most likely to be the cause on a busy workspace — true as far as it went, and pointing the reader away from the answer. Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X |
||
|
xarmian
|
3e3170e915 |
fix(server,cli,docs): the consumer contract, per codex round 12 (BUG-2726)
An angle no earlier round took: what does a CLIENT see. Two of the five
findings were about consumers I had never opened.
- `pad project watch` returned "event stream returned 429: {json}" and
exited, which sends the reader looking for a bug rather than at a
limit. It now says what happened and which knobs govern it, and names
the fact that those knobs cover this stream and the agent watch stream
together. It still exits rather than backing off — it is interactive,
and a human can decide — unlike the unattended monitor, which already
folds 429 into its ladder.
- Both endpoints now answer a refusal through one helper: same status,
same code, same message, plus `Retry-After`. `/api/v1/events` was
setting `Content-Type: text/event-stream` BEFORE the admission check,
so its 429 carried the JSON error envelope under an SSE content type —
a different contract from its sibling's for the same refusal. Admission
moved above the headers, which is where it belonged anyway.
- The anonymous-caller rule was documented as if it applied to both
endpoints. It applies to `/api/v1/events` only; the watch stream
requires a resolved user and answers 401 without one.
- docs/architecture.md described one SSE endpoint and one bus. It now has
the table: two streams, two buses, different scopes and consumers, one
shared connection budget, one Redis namespace.
FILED, not fixed: the web UI's `EventSource` cannot see a 429 or a
`Retry-After` — the spec exposes neither to the page — so a refused
browser tab reconnects at a constant rate while the CLI backs off. That
asymmetry means reaching the limit sheds load from the population that
respects it and not from the one that grows fastest under it. No
server-side change closes it; the fix is a client-side reconnect wrapper.
BUG-2733, and docs/deployment.md warns operators to size the limit with
it in mind.
Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X
|