fix(protocol): decide a moot claim from the record, not the label - #167
Merged
Conversation
`claim_is_moot` and `issue_is_finished` read the held labels outright. That disagreed with §6's rule 1, which has asked the record since protocol/9: a delivered task the operator requeued by commenting reads as still-held by its stale badge and as free by its record. The worker called it resolved and dropped a task the queue had just handed back, with nobody left holding it. Both now go through `holding_state`, which is §3.1 itself — where a record exists it decides, and only a task with no record falls back to its badge. Reusing the predicate that already backs `Task.holding` is what makes this free: a legacy no-record queue behaves exactly as it did before. `open_claim_of` needed one record it did not have. `lease_view` now also returns the state shas the matching-refs read already carried — a pure parse of a payload in hand, no request — and the caller resolves the single record it turns out to need via `States.at`. A worker holding nothing still pays nothing, and §5.3's "no GraphQL on a named claim" bound stands. `TaskState.requeued` reads an unreadable comment total as requeued, the same fail-open direction `_comment_total` already floors a lost GraphQL field to 0: offering a task whose worker then finds nothing to do escalates (§6), while the other direction buries a delivery the operator asked to reopen. Also splits the terminal transitions (escalate / deliver / release) out of `claim.py` into `terminal.py`. claim.py is about taking a task and holding it, terminal.py about putting it down; neither imports the other, since `read_body_file` moved to `comments.py`, which both already depend on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`build_parser` was 165 lines of repeated `add_parser`/`add_argument` sequences — a table written as procedure, and not by accident the largest function in the package. The 16 subcommands are now a `COMMANDS` tuple of `Command` records and the function is a 16-line walk over it. Positionals are a signature string (`"repo issue worker result_file pr_url?"`, a trailing `?` marking one that may be omitted); the two positionals carrying argparse keywords of their own (`reap worker`, `contract field`) ride `opts` alongside the flags. No behavior change, and not by inspection: every subcommand's full argparse surface — option strings, dest, nargs, default, required, choices, type, action class and help — was compared against the previous parser and matches exactly. The prose cuts are deliberately narrow. Most `protocol/N` mentions in the package describe ref shapes the code still accepts and stay, because they are the only explanation of why those branches exist. What went was autobiography already written down in HISTORY.md: `state.py` carried 26 lines restating §3.1 nearly verbatim, and `PROTOCOL_VERSION` carried a paragraph restating the spec it points at. Roughly 27 lines across seven modules. The package stays about 45% prose. That is not slack — the invariants it records (a projection is never read by a decision; which absence fails open) are ones neither the type system nor a unit test can hold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The hook asked the logged-in `claude` CLI to review every push touching `skills/**` or `README.md`, and ran the agent behavior suite as a second stage. Both are real model runs that spend tokens, on a path where a contributor expects a mechanical check. Nothing replaces it. The agent suite is now wired into no hook and no CI — `make test-agent` runs it by hand when `skills/` or `tests/agent/` change. The docs that described the hook and the `.githooks/*` LF rule in `.gitattributes` go with it.
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
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.
Three commits on
fix/moot-claim-reads-record. The first changes behavior, thesecond changes none, and the third is unrelated to either — see the note at the
end.
decb72f— the moot predicate reads the record (§3.1)claim_is_mootandissue_is_finishedreadatdecb72f— the moot predicate reads the record (§3.1)claim_is_mootandissue_is_finishedreadatdisagreed with §6's rule 1, which has asked the record since protocol/9:
The worker called it resolved and dropped a task the queue had just handed back,
leaving nobody holding it. Two answers to "did the task leave my claim" is one
too many.
Both predicates now go through
holding_state, which is §3.1 itself — where arecord exists it decides, and only a task wis badge.
Reusing the predicate that already backs `Tas cheap:
a legacy no-record queue behaves exactly as it did before.
Cost
open_claim_ofneeded a record it did not have, and that path is the one with noGraphQL call in it. Rather than add records mit read
(which would put a commit read on the one path that had none),
lease_viewnowalso returns the state shas the matching-refs read already carried — a pure
parse of a payload in hand, costing no requethe
single record it turns out to need via
States.at. A worker holding nothing paysnothing, and §5.3's "no GraphQL on a named claim" bound still holds.
Fail direction
TaskState.requeuedreads an unreadable coming thedirection
_comment_totalalready floors a lost GraphQL field to 0. Offering atask whose worker then finds nothing to do escalates (§6); the other direction
buries a delivery the operator asked to reop
Module split
Terminal transitions (escalate / deliver / release) moved out of
claim.pyintoterminal.py. claim.py is about taking a task and holding it, terminal.py aboutputting it down. Neither imports the other —
comments.py, which both already depend on.claim.py761 → 603 lines.16f8c53— table-drive the CLI, cut prosbuild_parserwas 165 lines of repeatedadd_parser/add_argumentsequences —a table written as procedure, and the largese 16
subcommands are now a
COMMANDStuple and the function is a 16-line walk overit. Positionals are a signature string (
"repo issue worker result_file pr_url?",a trailing
?marking one that may be omittyingargparse keywords of their own (
reap worker,contract field) rideopts.**No behavior change, and not by inspection.gparse
surface — option strings, dest, nargs, defauaction
class and help — was compared against the pr
The prose cuts are deliberately narrow. Most package
describe ref shapes the code still accepts and stay, because they are the only
explanation of why those branches exist. What went was autobiography already
written down in HISTORY.md —
state.pycarrearlyverbatim, and
PROTOCOL_VERSIONcarried a paragraph restating the spec it pointsat. Roughly 27 lines across seven modules.
The package stays about 45% prose. That is not slack: the invariants it records
(a projection is never read by a decision; which absence fails open) are ones
neither the type system nor a unit test can
630396b— drop the pre-push semantic-review hookUnrelated to the two commits above. The ude
CLI to review every push touchingskills/**orREADME.md, and ran the agent behavior suite as a second stage — real model runs that spend tokens, on a path where a contributor expects a mechanical cheagent suite is wired into no hook and no CI, andmake test-agent` runs it by hand.Happy to split this into its own PR if it mu
Verification
make checkon the branch HEAD:New coverage:
ClaimIsMootTests(6 cases) and 5 cases inResumeVerdictTests,pinning that a requeued delivery is not moot, that a stale badge the record
contradicts is not moot, that a task with nots
badge, and that
in-progressis never moot. Verified these actually pin thechange by restoring the old label-reading predicate: 3 fail, and the 2
backward-compatibility cases pass on both si
tests/COVERAGE.md§3 was corrected — it claimed the write-only clause waspinned when it was pinned only for the startable filter and the console, not for
claim_is_moot/issue_is_finished.🤖 Generated with Claude Code