chore: promote dev to main for 1.2.0 - #910
Merged
Merged
Conversation
feat(gr2): add native add, commit, and push verbs
fix: make PR merge refusals observable
Manual link application now fetches every referenced branch-backed gripspace and refuses before writing when behind upstream. Materialization records the requested revision so a detached HEAD is accepted only when it matches an explicit tag or commit pin. Rev-less or branch-configured detachment refuses rather than claiming freshness from an unproven state. Tests cover source enumeration, stale refusal and recovery, detached refusal, and explicit-SHA acceptance.
…eporting fix(link): refuse stale gripspace sources
Prove the propagation state contract before any daemon touches a real clone: observed -> fetched -> planned -> applied -> verified -> acknowledged, with refused, partial, and unverifiable reachable from any state. Every transition names the observation that established it and every receipt names the exact source and destination revisions. All identifiers are opaque; the allowed directions are a required input with no default. The tests build a bare source remote and three destinations (clean replica, dirty authoring clone, diverged authoring clone) and prove, each as its own witness: kill-between-states then replay applies exactly once (reflog and journal both measured); the cursor advances only on acknowledged; a moved expected base refuses with the observed base recorded and nothing merged or forced; dirty and diverged authoring clones are refused and left byte-for-byte untouched; an unreadable destination after the apply verb is unverifiable, never collapsed into a neighbour, and resolves on replay; acknowledged replays as a no-op returning the original outcome while refused starts a new attempt. Also carries the born-red outbox witness: read_events() advances the consumer cursor before the caller performs its effect, so a consumer that fails after reading loses the event. Marked xfail(strict=True) so the marker fails the suite the moment acknowledgment moves after the effect. Co-Authored-By: Claude <noreply@anthropic.com>
The first cut of the propagation prototype keyed journal rows and cursor files by joining the six coordinate fields with "|". The fields are opaque, so no delimiter can be assumed absent from them, and review proved the collision: (source="source|dest", destination="target") and (source="source", destination="dest|target") produced one key, so two distinct coordinates would have shared cursor and replay state. That contradicts the prototype's own opaque-identifier contract. The key is now the canonical JSON of the coordinate (sorted keys, compact separators), the same encoding operation_id_for already uses, and Coordinate.from_key decodes it. Injectivity is therefore STRUCTURAL, and the witness asserts it that way: every key round-trips to the coordinate that produced it, over the proven pair, over shifts across every field boundary including the enum-guarded one (an opaque field may itself contain the enum words), and over the bytes JSON uses for structure. The consequence is witnessed too: advancing one coordinate's cursor and writing its note leaves the other's cursor absent and its notes at zero. Each collision pair asserts its own control first, that the old scheme DID collide on it, so a green says the fix discriminates rather than that the pair was harmless. Mutation: restoring the pipe-join reds exactly the new witnesses (three pairs, eleven round-trips, the cursor-and-journal isolation) and nothing else, 22 of the original tests staying green; module restored and hash-verified against the pre-mutation bytes. 37 passed, 1 expected xfail, ruff clean. Co-Authored-By: Claude <noreply@anthropic.com>
…ain the outbox xfail to its cause Two defects found in second review, both in how the prototype REPORTS rather than in how it propagates. run_all dropped declared targets whose cursor was already at the source revision. run() returns None for "no new source revision" (not an operation, and that contract stands), and run_all took None to mean "not part of this invocation", so a replayed partial outcome reclassified itself: first run partial with reached=(clean,), second run -- nothing changed -- refused with reached=(). The aggregate must distinguish "already reached" from "never declared", and the only way to do that is to keep reporting the reached target. run_all now drives each target through a private path that, when the cursor is at the source revision, returns the terminal receipt from the journal (replayed=True) instead of None; a cursor at a revision the journal never acknowledged is a corrupted sink state and raises rather than guessing. The public run() is unchanged. Witnessed across three invocations: partial, then partial again with the reached target enumerated as a replayed terminal receipt and not applied twice (reflog count held at 1) while the refused target starts attempt 2, then acknowledged with both reached once the author cleans the refusal; plus an empty-targets control. The born-red outbox witness accepted any failure. xfail(strict=True) marks a test expected to fail for ANY reason, so a broken premise -- nothing emitted, nothing offered on the first read -- would have satisfied the marker forever while its reason text kept claiming cursor loss; replacing the first read's result with [] still reported the same expected xfail. The marker is now xfail(strict=True, raises=EventLost), where EventLost is raised only by the final survival check; every premise assertion is a plain assert, which is not the expected type and is reported as a real failure. The premise itself is also an ordinary test of its own, so an upstream outbox regression reds the suite instead of hiding inside the envelope. Evidence: 40 passed, 1 xfailed, ruff clean. Mutations, each restored from a saved copy and hash-verified: run_all back to the first cut's run() reds exactly the new replay witness; the witness's own first read returning [] reds the premise test AND reports the witness as FAILED rather than xfailed; an outbox that offers the event again reports XPASS(strict) as a failure, which is the marker-off signal the witness exists to give. Co-Authored-By: Claude <noreply@anthropic.com>
Found in third review: the branch that refuses a cursor sitting at the source revision with no acknowledged journal attempt behind it was promised by the previous fix and by the PR body, and nothing witnessed it. Replacing the RuntimeError with a silent return None kept the whole file green -- and a silent None there is the exact outcome class the previous fix closed, a declared target dropped from the aggregate, now with a corrupted sink state underneath it instead of a healthy one. The check now runs on both driver paths, not only the aggregate one: run() and run_all() both consult the journal when the cursor is already at the source revision, so a cursor the journal never acknowledged raises from either entry point instead of run() reporting "nothing new" over it. The witness forges the one state the journal can never produce (cursor advanced with no rows), asserts both entry points raise with the missing- acknowledgement detail, asserts the destination is byte-for-byte untouched, and carries a healthy control: an acknowledged cursor is the ordinary current state, None from run() and an already-reached replayed receipt from run_all(). Evidence: 41 passed, 1 xfailed, ruff clean. Mutation, restored from a saved copy and hash-verified against the pre-mutation bytes: the raise replaced by return None reds exactly the new witness and nothing else. Co-Authored-By: Claude <noreply@anthropic.com>
feat(gr2): propagation state machine prototype on synthetic repos
…lica Run the Prototype 0 state machine on a loop against a single destination that a declaration names as a managed replica of one branch of one source. One tick observes the source with ls-remote; when the cursor already names the source revision there is no operation and nothing is written; otherwise the machine runs once and its receipt, whatever its state, is written as its own JSON file with per-state latency derived from the receipt's own transition timestamps, and announced as one propagation.receipt event on the gr2 outbox. A refusal is a receipt too, and a refused replica is left untouched. The declaration can only name a replica: an authoring kind, an unknown git environment, a non-positive interval, or a missing field is refused before any git call. ensure_replica clones the declared branch single-branch when the path is absent and refuses a checkout whose origin or branch differ, or a non-git directory, before the machine ever reads it. Adds EventType.PROPAGATION_RECEIPT (documented in HOOK-EVENT-CONTRACT.md 3.2 and 7.2, with receipt_path declared as an explicit exception to the relative-path rule) and bumps the exhaustive EventType count test to 36. Tests: 35 new witnesses on synthetic repositories (declaration refusals, replica ensure and refuse, first tick, current tick, pushed change, refused then applied, receipt round trip with recomputable latency, loop and CLI). run_loop resolves stdout at call time so a redirecting caller gets the lines. Co-Authored-By: Claude <noreply@anthropic.com>
…keeps inherit as inherit
The first dogfood run against a real private remote hung on a username prompt:
the machine collapsed an explicit empty git environment (the daemon's "inherit"
mode) into its isolated default because {} is falsy, so the clone made with the
host's credential helper was followed by an ls-remote without it. Three changes:
- the machine distinguishes None (the isolated default) from {} (inherit), and
names a failed or empty ls-remote SourceUnobservable (a RuntimeError subclass,
raised before any state is touched) instead of a bare RuntimeError
- the daemon sets GIT_TERMINAL_PROMPT=0 in both git environment modes, so a
missing credential fails the tick instead of hanging the loop on a tty
- run_loop prints and counts a tick whose git call fails (SourceUnobservable,
CalledProcessError, OSError) and goes on; the next tick replays whatever the
machine left pending, which is the machine's own kill-and-replay contract
Witnesses: inherit reaches the machine as inherit (asserted at both ends), both
modes carry the prompt override, a source moved away fails two ticks with
tick-failed lines and no receipt, and the tick after it returns acknowledges.
Four mutations (restore the `or`, drop the prompt override, stop catching
SourceUnobservable, raise the bare RuntimeError again) each kill exactly their
witnesses.
Co-Authored-By: Claude <noreply@anthropic.com>
… the journal escape The second reviewer of this branch enumerated every raise site reachable from a tick and ran the one the loop still let escape: the machine wraps a failed destination read in DestinationUnreadable, which is raised at observe, plan, and verify (each a point the machine replays from), and the loop caught the two exceptions it wraps but not the wrapper, so a checkout removed or a volume unmounted mid-loop crashed the daemon on the next operation. - run_loop now catches DestinationUnreadable with the other environmental failures, prints it as a tick-failed line, counts it, and goes on; the catch list is derived from the machine's raise sites and says so - the machine's deliberate escape for a cursor the journal cannot account for is named JournalInconsistent (a RuntimeError subclass) so an escape that is meant is distinguishable from one that is not - the docstring lists what propagates by design Witness: the first tick acknowledges, the checkout is removed and a new source revision pushed, two ticks fail with "destination unreadable" lines and no new receipt, and a fresh loop re-ensures the replica and acknowledges. Mutations: dropping DestinationUnreadable from the catch list kills exactly that witness; raising the bare RuntimeError again kills exactly the corrupted-cursor witness. Co-Authored-By: Claude <noreply@anthropic.com>
…ursor; witness what propagates Reviewing commit 3, the second reviewer widened the loop's catch list to swallow RuntimeError and LookupError and the suite stayed green: the docstring's claim about what propagates by design was unwitnessed. Writing the witness found a real gap behind it. The daemon's tick observed the source itself and, when the cursor already named the source revision, returned "current; not an operation" without asking the machine, so the machine's corrupted-sink check (a cursor at a revision the journal never acknowledged raises JournalInconsistent) never ran on the daemon path and "not an operation" would have hidden corrupted sink state on every tick forever. Found by the witness, not by the dogfood. - Propagator.run accepts an optional observation so a caller that already observed this tick can hand it over instead of asking the source twice; the cursor check runs on it all the same - the daemon hands its observation to the machine on both paths, so a corrupted cursor leaves the loop by name with no tick line and no receipt - witnesses: a forged cursor makes run_loop raise JournalInconsistent with nothing written; LookupError, a bare RuntimeError, and ValueError from a tick are not swallowed (the loop is bounded in the test so a wrong loop fails rather than hangs) Mutations, eight rows in one run, each killing exactly its witnesses: the six from before, plus restoring the not-new shortcut and widening the catch list to RuntimeError and LookupError. Co-Authored-By: Claude <noreply@anthropic.com>
fix(link): verify gripspace pins before applying
feat(gr2): Prototype 1 propagation daemon on one declared managed replica
…lease-guarded fast-forward push The contribution protocol on the Prototype 0 machine: one new destination kind and no new state. DestinationKind.CANONICAL is a bare remote that owns the branch; an operation with direction=up observes the child's branch, fetches it into the sink's mirror, plans against the owner's branch as read now, and lands by `git push --force-with-lease=<branch>:<expected_base>` from the mirror. The receiving repository enforces the compare-and-swap; the plan's fast-forward gate guarantees the lease never forces; a rejected lease is a REFUSAL carrying the revision the owner holds. Replanning is the author's act: the machine never rebases, merges, or forces on anyone's behalf. Machine changes: read_head reads the branch ref (never HEAD) for a canonical; the cleanliness gate is recorded NOT RUN for a bare destination rather than omitted; ahead/behind fetches the branch for a canonical; apply has a lease-push verb with a `before_apply_verb` test seam; verify names the canonical postcondition (branch-is-intended-after-and-tree-matches-digest, no worktree term). Six witnesses on a scratch parent + two subspace clones: the happy path with exact revisions and the not-run gate; the manufactured collision refused at plan with the observed base and nothing touched; the compare-and-swap race (plan, sink dies, the other child lands, resume) refused at apply; the lease refusing a move inside the check→push window; replan-by-the-author landing as a fresh attempt with both attempts in the journal; policy refusing `up` before any verb. Five mutations each kill their own witness (bare --force instead of the lease → the window witness; dropped apply-step head check → the race witness; fast-forward gate forced to pass → the collision witness, because a held lease would then force; porcelain on the bare remote → every landing; clean gate as pass instead of not-run → the happy path). Existing suites: 83 passed, 1 xfailed unchanged; with this file 89 + 1. Co-Authored-By: Claude <noreply@anthropic.com>
…ibution sets, retire refusal, append surfaces The machine lands ONE contribution. This adds the protocol around it, each piece the smallest shape its witness needs: - the state machine's run() takes stop_after=PLANNED: drive an operation through its gates and stop BEFORE any verb, journaled at planned; a later run resumes it. This is how a set prepares every member against its owner's current base before landing any of them - ResolvedManifest / ResolvedEntry: every resolved entry carries declared_by and overridden_by; owner is the override or the declaration; classify() answers by longest declared prefix and refuses a path under no entry; two layers declaring the same entry without override is a NAMED ResolutionCollision, never a silent precedence. Today's resolver flattens this away, so the dataclass is the field the resolver must grow and the witnesses run against the stub - ContributionSet: prepare() every member (no verb), land() in declared order, STOP at the first member that does not acknowledge; the set receipt names the landed, the refusing, and the not-attempted members; nothing is rolled back (history is forward-only; a rollback would be a new forward operation) - Subspace.retire() refuses while any contribution is open (not acknowledged and not explicitly abandoned) and lists their operation ids; abandon() is a note in the contribution's own journal, so a change can never simply evaporate - AppendSurface: one guarded append point (exclusive lock across write, flush, fsync), arrival-ordered sequence numbers, no expected base because appends commute BY DECLARATION; file-level only, never a git-tracked path Eleven witnesses in gr2/tests/test_contribution_protocol.py (W1 x3, W4 x4, W5 x2, W6 x2). Eight mutations, each killed by its own witness: refused-is-terminal, retire-ignores-open, land-never-stops, prepare-lands, collision-by-precedence, owner-ignores-override, append-caches-seq, classify-first-match. Two of those needed the harness tightened first: one mutation had not actually applied (the harness now asserts the mutated file differs from the saved copy before running), and the classify fixture's declaration order made first-match and longest-prefix agree, so it was reordered until they disagree. All four propagation suites: 100 passed, 1 xfailed. Co-Authored-By: Claude <noreply@anthropic.com>
…s, printed for one landing and a two-member set state_latencies(receipt) returns (state, seconds since the previous transition) rows from the receipt's timestamps — the number comes from the artifact, not from a stopwatch around it. One test prints the table (visible with -s, written to a file) for a single contribution and both members of a set. On this host a single contribution lands in ~0.23 s end to end (fetch ~0.09, plan ~0.05, lease push ~0.07, verify ~0.02); for set members the applied row spans prepare -> land by construction, so it reads as the time the prepared base sat, not the push alone. Co-Authored-By: Claude <noreply@anthropic.com>
… neighbour The journal writer appends and fsyncs, so a kill between the two leaves a partial line. Three consequences, each with its own witness. A tear used to cost TWO rows, not one. Appending onto an unterminated line glues the new row to the remnant, so a row that was written correctly and fsynced becomes unreadable. _write now terminates a remnant before appending, confining the damage to the line actually interrupted. _rows raised a bare JSONDecodeError on any unparseable line. The daemon lets anything outside its named failure list propagate, so one bad moment exited the loop on its first tick and on every restart after it. Lines that cannot be parsed are now dropped and counted, and MalformedLine keeps the drop from being silent. Position does not discriminate cause, which is why the drop is unconditional rather than trailing-only: a tear is trailing only until the daemon restarts and appends again, after which the same orphan sits mid-file with intact rows on both sides. The question that does discriminate is whether the cursor can still be accounted for, and replay already asks it and already raises JournalInconsistent by name. Dropping here lets that check decide instead of being pre-empted by a parse error. Prototype 1 asks the machine to account for an observation on every tick, including idle ones, which moved this parse from once-per-change to once-per-tick. Rows are now cached against (size, mtime_ns); the Propagator is built once per loop and holds one Journal, so the cache spans ticks. The one state it cannot see - an in-place mutation changing neither size nor mtime_ns - is named in the code rather than assumed away. 86 passed, 1 xfailed, up from 83 by exactly the three witnesses added. Each guard is mutation-proven: removing the newline repair reddens the neighbour witness, restoring the bare parse reddens that one and the skip witness both, and disabling the cache reddens the idle-tick witness. Ref #893 - closes at promotion Co-Authored-By: Claude <noreply@anthropic.com>
fix(gr2): a torn journal line stops being fatal, and stops eating its neighbour
feat(gr2): Prototype 2 contribution protocol — canonical destinations, lease-guarded up, sets, retire refusal, append surfaces
A writer killed between its record and the terminator leaves a remnant line.
Three consequences, all measured before this change:
1. append() raised on the remnant and kept raising: the guarded append point
was BRICKED permanently, because every later writer re-reads the whole file
to compute the next sequence number and dies on the same line.
2. The next record GLUED onto the remnant, producing one unparseable line and
losing a record that had itself completed and fsynced.
3. records() raised, so every reader of the surface died too.
Both scans now skip a line they cannot read AND count it (malformed_lines,
reflecting the most recent full scan). Skipping alone is silent, and silence is
the defect: the remnant sits in the file while a caller sees a healthy-looking
surface with a contiguous sequence. append() also repairs a missing terminator
before writing, so a completed record is never swallowed by an incomplete one.
MalformedLine is reused from the state machine rather than re-declared, so the
two append-only surfaces in this tree report a torn line in one vocabulary.
Four witnesses, five mutations, each mutation killing witnesses whose failure
TYPE matches it: removing the terminator repair kills by AssertionError (glue is
wrong content, not an exception); removing either scan's guard kills by
JSONDecodeError and KeyError, exactly the types the guard catches; silencing
either count kills only that path's count assertions. Tear-fixture count for
this surface: 4, previously 0.
Ref #897 - closes at promotion
Co-Authored-By: Claude <noreply@anthropic.com>
Two review blocks on this PR, both correct, and both about the SHAPE of the guard rather than a case it was missing. FIRST: an except tuple over untrusted file content is a DENYLIST, and a denylist leaks by construction. Valid JSON seq 1e999 parses to inf, int(inf) raises OverflowError, and no list written from the parse side would have predicted it, because the COERCION AFTER the parse is what invents the new failure. Asking what else escaped the same guard found a sibling: deeply nested JSON raises RecursionError from json.loads itself and bricks identically. So the coercion is gone. Everything after the parse validates types rather than converting them, and inf is refused because a float is not a sequence number, with no exception existing to catch. SECOND: the decode is a raise site BELOW the parse. The exception type was never the problem, since UnicodeDecodeError subclasses ValueError which was already caught; the OPERATION that raises it sat outside the guarded region. In text mode the decode happens while the iterator MANUFACTURES the line, so bytes are converted before any try block can see them. The fix guarded the transformation and left the ACQUISITION unguarded. Operationally that was worse than a brick: a whole-file decode means ONE bad byte anywhere destroys every record in the surface, including the thousands written correctly around it. The file is now read as bytes and decoded one line at a time, which is what a JSONL file actually is, so a bad byte costs its own line. A line passes through four layers - read, decode, parse, shape-check - and the last three are each guarded where they happen. Naming the enumeration rather than claiming completeness, because two earlier claims that the raise-site set was closed were each broken by the next reviewer: unbounded line length is a resource limit on the READ layer and is deliberately not defended, stated in the class docstring and the PR body rather than left to be discovered. Also pinned, found when a new witness exposed a wrong expectation in a fixture of mine rather than a defect in the code: a line can be unreadable as a RECORD while its sequence number is perfectly readable, and numbering honours it anyway. No writer is ever issued a number a reader can already see on disk. Seventeen hostile-content and undecodable-byte rows plus a confinement witness proving ten good records survive a bad byte written between them. Ten mutation rows, measured with an extractor that reads only E-prefixed traceback lines and is proven against a green control - an earlier extractor read exception names out of parametrized TEST IDS and reported types that were never raised. One row was discarded as contaminated, its kills coming from a sloppy edit rather than the defect, and re-run clean. Ref #897 Co-Authored-By: Claude <noreply@anthropic.com>
fix: AppendSurface survives a torn write, and counts what it skips
Both lane prototypes carried a BYTE-IDENTICAL append_jsonl and a reader with the same body under two names. That is one defect with two addresses, not a class with two instances, so it gets one fix rather than two: a shared jsonl_store module both consume. Fixing them separately would have written the same fix and the same witnesses twice and left the copies free to diverge again, which is how the situation arose. The defect, in both copies: the writer appended with no terminator repair and no fsync, so a writer killed mid-write left a remnant and the NEXT record glued onto it - a record that had itself completed became part of one unparseable line and was lost. The reader parsed with a bare json.loads over read_text().splitlines(), so the remnant raised, and the whole-file decode meant one invalid byte anywhere destroyed every record in the file rather than its own line. WHERE THE COUNT LIVES, which was the open design question: these are module-level functions with no instance to hang health on, and a module-level accumulator would be hidden state and wrong under concurrent readers. So the count is a RETURN VALUE - read_jsonl returns rows AND the lines it could not read. The question dissolves rather than getting answered. And the count reaches a CONSUMER: both CLI callers report it on stderr, so a --json caller's stdout stays machine-readable. A count nobody surfaces is the same silence as no count at all, which is the defect this fix exists to close. The reporter lives in the shared module too. The first version of this change put a helper in one consumer and an inline copy of the same three lines in the other - byte-adjacent duplication of a reporting rule across two files, which is the exact shape being eliminated, committed inside the fix. Caught before the gate. Structure is validated; schema is not. A generic reader has no fields to check, so a line that is well-formed JSON and a well-formed object IS a row even when its values are odd. That boundary is pinned by its own witness, added after a witness failed against a wrong expectation of mine rather than against the code - the hostile-content table had been copied from a surface that does have a schema. 21 witnesses, 7 mutation rows, each mutation killing witnesses whose failure TYPE matches it: removing the decode guard kills seven by real UnicodeDecodeError; removing the parse guard kills six by real JSONDecodeError, RecursionError and the integer-literal ValueError; silencing the count kills fifteen; silencing the reporter kills exactly one, which is what proves the count reaches a consumer. The fsync witness is deliberately weaker than it sounds and says so: it pins that fsync is CALLED, not that durability holds, because a real crash is unwitnessable in-process. It exists because without it, deleting the fsync killed nothing at all - and a guard nothing checks is not a guard. Unbounded line length is a resource limit on the READ layer and is deliberately not defended, named in the module docstring rather than left to be discovered. Ref #897 Co-Authored-By: Claude <noreply@anthropic.com>
fix: one torn-line-safe JSONL home for both lane prototypes
Fix 4 of the torn-line sweep, and the two-part contract it was ruled as. TERMINATOR REPAIR. emit() appended with "a" and wrote json + "\n". A previous write that died between write() and fsync() leaves a last line with no terminator, so the next append GLUES two records into one. The damage runs FORWARD from the tear: the torn record and THE NEXT HEALTHY APPEND fuse into one unparseable line, while the record before the tear is untouched. A torn write therefore costs that record and the next one written after it, permanently, because every later append builds on the glued line. emit() now probes the last byte under the existing write lock and heals the seam first. THE COUNT. Both readers skipped unusable lines in silence. For the channel bridge an unreadable line is a message that never reaches a channel. It is reported on EVERY read, not once: the cursor filter applies only to lines that parse, so a line with no usable seq can never be advanced past, wherever it sits -- position is irrelevant and mid-file lines repeat exactly as trailing ones do. Deliberate, not incidental, and its cost is named in the residuals. read_events_detailed() now returns the events AND the lines it could not read, from the SAME read. read_events() stays list-shaped for the ELEVEN call sites that index and len() it -- all tests, and ZERO production callers remain once the bridge moves to read_events_detailed(), so the wrapper is a test-compatibility surface rather than a load-bearing API. An earlier draft of this message said seventeen; that was a substring artifact counting a def, two prose mentions inside strings, and four hits on an unrelated _read_events helper in another test file. The bridge reports on stderr so stdout stays parseable. Reported from the read path ONLY. _current_seq() runs once per emit inside the write lock, where a count would be per-APPEND and aimed at whoever happened to be writing. Its docstring says so, because an omission and a decision look identical in code. Also guarded, each with witnesses: the decode moved to bytes-per-line so a single invalid byte cannot escape from outside every guard; the parse guard's exception tuple is now derived from what json.loads can raise rather than from what had been seen -- the old (JSONDecodeError, TypeError) caught syntax errors but missed RecursionError, which is not a ValueError, so deep nesting escaped; structure is checked separately, since a valid JSON array parses and is still not an event; seq values are type-validated, since bool is an int subclass and a float becomes inf and serializes as Infinity; and a corrupt cursor no longer bricks reads. THREE THINGS THIS FIX GOT WRONG FIRST, none found by its own witnesses: - An early version swallowed OSError in the line iterator. That is exactly the OSError-to-zero fallback an earlier fix removed on purpose: swallow it and _current_seq returns 0, then emit allocates sequence numbers that duplicate live ones. A pre-existing test stood guard and caught it. Content errors are data and get skipped-and-counted; an I/O error is not knowing what the file holds and must fail closed. The reader tolerates FileNotFoundError only, because rotation renames the file, and propagates every other OSError. - MalformedLine and EventRead were dataclasses. This module is loaded out-of-tree by spawned workers via spec_from_file_location + exec_module, which does not register it in sys.modules, and dataclass resolves field types through exactly that. Every worker died at import. Now plain classes, with a witness that fails in 0.06s naming the cause instead of after a 10s timeout that reads like flakiness. - The default report echoed raw line content to stderr, verbatim, including an API-key-shaped string in a measured probe. stderr is copied into CI logs and transcripts. Redacting was rejected: matching secret-shaped patterns in arbitrary bytes is a denylist over untrusted input, the same defect the parse guard exists to avoid. The excerpt stays on the data object; the default report prints ordinal and reason, both structural, with show_content opt-in. One existing test's monkeypatch target moved read_text -> read_bytes because the read verb changed; its contract and every assertion are unchanged, and it would otherwise have gone green by missing its target. Both it and the new witness now undo the patch before reading the file back, since a verification must not travel the path the test deliberately sabotaged. A reviewer measured both of those statements against an earlier version of this message and of the description, where the glue direction was backwards and the count was claimed to be reported once. The suite was green while the prose said the opposite of the code, because nothing pinned that behavior. Three witnesses now do, including the mid-file case, which shows the rule is broader than the trailing-line framing that surfaced it. A third review then caught a further claim, in the description only: that a torn last line self-heals at the next emit. That is true of one tear and false of the other, and the bare word "torn" hides the difference. An UNTERMINATED record -- complete, only its newline lost -- DOES recover fully once the seam is healed. A TRUNCATED record -- the write stopped mid-record -- never does: its bytes were never written, so it stays unreadable and every read reports it until the file is repaired. A FOURTH review then caught the opposite overcorrection, which a later draft of this message had made -- claiming a torn record never becomes readable -- and the unterminated-tear control disproves it. Both cases are now named separately and the bare word is not used to claim anything about recovery. That prose was wrong only because every tear fixture until then dropped the trailing newline and left the record COMPLETE -- the lucky case. Three witnesses now cover the realistic tear, with the UNTERMINATED tear kept as a discriminating control so the finding cannot be mistaken for the norm. What the repair buys, stated exactly: it cannot recover a record that was never fully written, and it saves the NEXT one. Correcting the message and the description was not enough, and a second review caught that: the same backwards claim was still in the production repair comment and in the test module docstring. Fixing the two cited surfaces and stopping there left the code asserting one direction while the description asserted the other -- an artifact contradicting itself, which is worse than being uniformly wrong. A sweep for the whole class rather than the cited instances found a third live instance neither review named: the reported-once claim, still standing in that same docstring. Those particular corrections changed comments only and left the executable syntax tree identical, verified by parsing both revisions and comparing with docstrings stripped. A FIFTH review then caught the same overclaim surviving in the test module comment -- "A TORN RECORD IS NEVER REPAIRED BY A LATER EMIT" -- which the unterminated control disproves. The class query that was supposed to have swept it missed it because the query was CASE-SENSITIVE and the comment is upper case: a false negative from my own instrument, of the kind noted one revision earlier and then committed in the next query. That round also renamed the control from "benign" to "unterminated" so the tests and the prose use one vocabulary, so unlike the previous round the syntax tree DID change here and the suite was re-run rather than reasoned about. A SIXTH review, from the other reviewer, found three defects and all three were PROSE -- the code it gated was clean. The first is the one worth the round: the reported-once claim was still live in the channel-bridge comment, in the bridge hunk of ALL FIVE versions, and every class sweep I ran missed it because it PARAPHRASES the sentence the earlier reviews cited rather than repeating it. A query built from a cited instance finds COPIES, not paraphrases; the class is the CLAIM, and the only instrument that finds a paraphrase is reading the hunk. The second: the parse-guard sentence read as if the new tuple were open-ended. It is enumerated -- (ValueError, RecursionError). What changed is where the enumeration comes FROM: what json.loads can raise, rather than what had been seen. The third: "seventeen call sites" was an artifact of a substring query that counted a def, two prose mentions inside strings, and four hits on an unrelated _read_events helper in another test file. The real count is eleven, all tests, zero production callers. BOTH reviewers co-signed seventeen at v1 from that same defective query, which is why it needed re-deriving rather than re-reading -- and the class sweep for it then found three more copies of the count in docstrings that no review had cited. A SEVENTH review found two more, and one of them was wrong in the CODE rather than in the prose about it. The bridge comment said an unterminated record is reported and then heals at the next emit. Measured false: an unterminated record whose bytes are COMPLETE is never reported at all, because _iter_outbox() splits on b"\n" and a complete final chunk parses on the spot -- before any emit, repair or no repair. The sentence implied a window of unreadability that does not exist. The gap that let it survive is now pinned: every tear fixture in this file emitted AFTER tearing, so nothing had ever asked what the READER alone does with a torn file. W11 is that pair -- the unterminated case reporting nothing across two reads, the truncated case reporting one as its discriminating control. The reviewer's own probe became the witness. Its two siblings, the W10 header and the description's table, are scoped to tear -> emit -> read and are true there, so they are deliberately unchanged. The second finding was stale suite totals, and the mechanism differs from the one proposed: the +3 is not the earlier merge, which added 21 tests that are inside both baselines, but a test directory the narrow invocation does not collect. Both figures were correct measurements of different scopes and the defect was publishing one without naming which -- which is also how the three mutation rows above went stale, unasked about by any review. RESIDUALS, named rather than presented as clean: this is a third copy of these primitives, and consolidating them is outside this fix's scope; the same content-echo property exists in the prototype reporter merged earlier; unbounded line length remains a read-layer resource limit and is undefended; and a TRUNCATED record -- an UNTERMINATED one is readable throughout and never reaches the report at all -- is permanently unreadable and reported on every read, so a consumer polling in a loop warns every cycle until the file is repaired -- suppressing that would need persisted already-reported state and would hide a re-occurring fault, so it is the lesser cost but it is a cost. Evidence: 37 witnesses, 0 before. Nine mutation rows, RE-MEASURED for this version over the two events spec files (71 tests), each killing witnesses whose failure TYPE matches the mutation, restores hash-verified with the unmutated pair as a control: decode 11, exception tuple 1, terminator repair 5, seq validation 4, count silenced 19, reporter silenced 1, OSError swallowed 3, FileNotFoundError widened 1, cursor guard 1. Three of those rows were STALE -- published as 8, 3 and 12 where the true figures are 11, 5 and 19 -- because they were measured early and never re-derived while five review rounds added witnesses to the files they count. Full suite at BOTH scopes, base pinned by SHA, since omitting the scope is what made the last version's totals unverifiable. On origin/dev@93675677 in an isolated worktree: pytest gr2/tests 1032 passed / 5 failed, pytest gr2 1035 / 5. On this head: 1069 / 5 and 1072 / 5. Delta exactly +37 in both, matching the witness count, failure sets identical in both directions. The scopes differ by gr2/gr2_overlay/tests/test_overlay_refs_namespace.py -- 3 tests, measured passing on both sides, which the narrow invocation does not collect. Lint unchanged: production 6 = 6, edited test file 23 = 23, new file 0. Premium boundary: grip is OSS; this is local file mechanics over opaque paths and carries no identity, org, or policy content. Co-Authored-By: Claude <noreply@anthropic.com>
fix(events): repair torn outbox lines and report the ones that cannot be read
gr checkout reported every per-repo failure to the terminal and incremented no counter, so a run in which every repo failed printed "Switched 0/N repos to <branch>" and exited 0. The printed ratio is something a caller has to read and interpret; the exit code is the only failure signal a script sees, and it reported the batch as done. Adopt cli/repo_iter::for_each_repo -- which is what gives this command an error count at all -- and make a nonzero exit disjoin the per-repo failures. Two changes fall out of the adoption: - for_each_repo's cloned-check called path_exists on the repo directory while its own docstring promised to skip repos that "aren't cloned". RepoInfo::exists tests for .git, which is what cloned means; the two diverge on a directory that exists and is not a clone. Aligning the code with the docstring preserves checkout's existing behavior exactly rather than reclassifying a skip as an error. - for_each_repo_path had no callers and gains none here, so it is removed rather than left relying on pub visibility to keep its own dead-code warning quiet. The plan step prescribing that suppression is struck in the same change. An unused private item warns and an unused pub item does not, so following the step as written left the module with zero callers and nothing anywhere going red for as long as it existed. A warning is a detector; silencing one to reach a clean build is not a fix. Tests pin both sides of one property. The discriminator is an ABSENT .git versus a PRESENT BUT UNOPENABLE one: absent is a skip and still exits 0, present but unopenable is an error and exits nonzero. Without both sides, a witness asserting only the failure case could pass while the skip path had silently become an error too. These are two separate tests with their own fixtures, and those fixtures differ in more than the discriminating property -- one corrupts a single repo and the other both, one creates a branch first, and they target different branches. The claim is about which property discriminates, not about the fixtures being otherwise identical.
…o-failures fix(checkout): count repo failures and disjoin them into the exit code
…topology feat(workspace): initialize spec from declared topology
gr pr merge reported success when every merge in a run had failed. The per-repo diagnostics were printed and truthful; the exit code was not, so a script driving the command was told a batch had merged when none of it had. Three exits returned Ok while carrying failures: - an empty candidate list that was empty because the PR lookups FAILED. "We looked and found nothing" and "we could not look" are different answers, and only the first is an absence of PRs. They are now tracked separately rather than arriving at the summary as one state. - the --auto path, where every attempt to enable auto-merge could fail. - the final exit, after a mixed or wholly failed run. Each now fails with a count of what failed against what was attempted. The test fixtures needed correcting first. mock_get_pr mounts a single invariant response, so a read issued after a successful merge still reported merged: false, which the real API cannot produce. Three groups of tests were affected, by three different causes, and they should not be counted together. FIVE tests began failing for the fixture reason once the exit code became truthful. The new mock_pr_lifecycle couples the GET and the merge PUT through shared state so the sequence behaves as the live API does; all five then passed with NO ASSERTION CHANGED, which is what distinguishes a fixture defect from a contract change. A SIXTH test, repo_filter_excludes_non_target, carried the same fixture defect and never went red at all: its PR was mocked as state "open" and merged: true simultaneously, making the command's own post-merge verification vacuous, so it passed whether or not the merge did anything. It now starts unmerged and GAINS an assertion that the merge actually fired. ONE test, branch_behind_suggests_update, was red for an unrelated reason and is the only assertion this change edits. It required is_ok() while its own failure message said "handled without crashing". Those are different claims. The merge did not happen, so graceful handling means a useful error rather than a success. Its fixture is untouched. Every added guard is mutation-proved: neutering each of the three exits independently turns a named test red. The --auto path had no test of any kind before this change, so its guard arrived with the first one. Ref #884 -- closes at promotion Ref #886 -- closes at promotion
…ilures fix(pr): count failed merges and disjoin them into the exit code
Bumps the crate to 1.2.0, adds the 1.2.0 changelog entry, corrects a false [Unreleased] heading, and fixes formatter drift in one test file. SCOPE. The work promoted is v1.1.0..6192e8c -- the 1.1.0 tag to the dev tip at freeze -- which is 42 commits / 14 first-parent units. This release-prep commit sits one beyond that; counting it makes the range 43 / 15. Units are classified by diffing each against its FIRST PARENT. A merge commit has no canonical diff and git show --name-only returns nothing for these, so the method is part of the claim. FIVE of those 14 units are in the published crate. Nine are gr2, a separate Python surface at 0.1.0 that is not distributed. An entry claiming fourteen units of work would describe a release nobody can install. Two claims are deliberately narrow. The exit-code work fixes two instances of a class with 9+ known members, and the most consequential member is untouched. The freshness work ships with two measured holes open and no user-facing documentation. Both are stated as limits in the entry. A commit-level statistic was removed twice. First as unsourced -- "roughly a third of this repository's active commit volume," no range, no metric. Then as mis-sourced: "22 of 42 commits, measured by first-parent diff" cited a method that did not produce it, since 22 comes from a path-limited rev-list whose history simplification drops eight merges that touch gr2 against their first parent. Under the named method the figure is 30 of 42. A number citing the wrong method is worse than an unsourced one, because the citation invites trust it has not earned. The unit measure, 9 of 14, needs no footnote and is what remains. The [Unreleased] heading was false: that work shipped in v1.1.0 on 2026-08-13 and is on crates.io. Verified by checking the feature's own symbols into the v1.1.0 tree with a negative control at v1.0.2, not by commit archaeology. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
chore: release gr 1.2.0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ceremony promotion of
devtomainfor the 1.2.0 release.Promoted via a throwaway
promote/branch rather than makingdevthe PR head, becausedelete_branch_on_mergeis enabled and merging a dev-headed PR would delete the integration branch. Ancestry is preserved exactly: this branch is dev's tip commit.Content is already gated. The release commit passed a four-version public-push gate with two reviewers bound to head
2eca904and body72deb4d4. This PR adds no new bytes.Premium boundary: grip is OSS — multi-repo workspace orchestration, no identity or org semantics.