feat: evidence bound to its inputs, gates that fire, and a second reviewer (ADR-017 to ADR-022) - #9
Merged
Merged
Conversation
Found by wiring MirroringBoard to a caller for the first time (the ADR-017 slice). `resync` of a real
item failed: `projected 1, failed 1`, with the mirror saying
state pr_open not projected onto mirror-fake: illegal board transition ready -> pr_open:
allowed from ready: claimed
Every mirror item was created at `ready` and then WALKED to the state being projected. That walk does
not exist — the state table allows `ready -> claimed` and nothing else out of `ready` — so a projection
of any item past `claimed` (pr_open, fix_round, merged) could never be built, and `resync`, whose whole
promise is "a projection is rebuildable", could not rebuild in-flight work at all.
The port already had the answer: `BoardWorkItemSpec.state` ("the state the new item starts in, default
ready"), honoured by all six adapters. The mirror simply never passed it.
- the mirror item is created IN the projected state, so no walk is needed
- a comment arriving FIRST for an item the mirror has never seen reads the state from the authority
rather than assuming `ready` (creating the copy at `ready` would state something false about work
that is already merged)
- the test double stopped hardcoding `state: 'ready'` and now honours `spec.state` like every real
adapter — that hardcoding is why core's suite never saw this: the double answered more kindly than
the host (the ledger's class list, #13), and only a real caller could expose it
Verified with a real resync over items in ready/pr_open/merged: projected 3, failed 0, and the mirror's
items read ready/pr_open/merged. Core 244/241/0, CLI 47/47.
Fix is 9bc6f90. Found the first time the decorator had a caller — which is the argument for wiring a capability in the same slice that builds it, and for a test double that answers exactly as unkindly as the host.
… (`pilot --resync`)
ADR-017 built MirroringBoard and never had a caller: the "capability with no reader" smell this project
keeps removing. This is the wiring, and wiring it found a real bug (fix `9bc6f90`, ledger #21).
- `pilot.boardMirrors: [{ id, provider, options, idMapFile }]` in takumi.yaml — the board OTHER people
look at, built through the same provider factory `takumi board` uses, so a mirror can never be a
configuration the CLI cannot also express or check
- the composite is constructed where the board is built, with the event log, so mirror writes and
failures are EVENTS (`mirror.written` / `mirror.failed`) rather than whatever the mirror felt like
- what is projected WHERE is printed on every tick (`mirror notion: canCreateWork=true,
editableComment=false`): the check ADR-017 asks for, and the reason a read-only projection is never
mistaken for an authority
- `takumi pilot --resync`: rebuild every projection from the primary WITHOUT running a tick, then save
the id map. Asked for with no mirrors configured, it says so instead of printing nothing.
- the id map is persisted next to the slot dir: loaded tolerantly (a damaged cache costs API calls, not
correctness), written after the work, and a write failure is REPORTED rather than thrown — a cache
must not be able to fail a tick that already happened
- the primary's `pr_open` note now carries the MR/PR URL: "PR #7" without a link is a reference a reader
(in particular a mirror's reader) cannot follow
Tests drive the COMMAND, not the class: a configured mirror is projected onto and its id map lands on
disk, `--resync` rebuilds without a tick, and `--resync` with no mirrors is an error rather than a
silence. Core 244/241/0, CLI 47/47, 0 TS errors.
Live Notion check next, pending a token: the database already exists with the right property names and
the seven states as select options.
Found by projecting real work into a real Notion database (ADR-017): `resync` reported `projected 1, failed 0` where the GitLab project holds seven items. Five of them were merged — and merged items are CLOSED, while GitLab's `listWork` hardcoded `state=opened` (its own `issuesUrl` has taken `'opened' | 'closed' | 'all'` all along). GitHub had the same shape with `state: 'open'`. So every finished item was INVISIBLE to any caller that asked for it: a mirror rebuilding itself, an audit, a report. The pilot never noticed because it only ever asks for open work, which is exactly why the bug could live this long. - the host's open/closed axis now follows the REQUEST: `merged` in `query.states` asks the host for `all` (GitLab) / `state=all` (GitHub), and the local state filter still decides what the caller sees - the shared contract suite asserts the rule as a rule: after driving an item to `merged`, the board must list it when `merged` is asked for. A state that can be set but not listed is invisible. - verified against the REAL GitLab project: `resync` went from `projected 1` to `projected 7, failed 0`, and a raw REST read of the Notion database now holds all seven rows with the right Status (six `merged`, one `blocked`), with no duplicates across two resync runs HONEST LIMIT: the board suites' doubles answer list queries without honouring the host's open/closed axis, so the new contract assertion passed against GitLab and GitHub BEFORE the fix — the live run is what caught it. The doubles are kinder than the hosts again (the ledger's #13 class), and making them model the axis is the next thing this rule needs in order to be defended by tests. Core 244/241/0, boards 147/147/0, 0 TS errors.
Fix is 5e9b1fd. Found the first time a MIRROR asked a board for finished work — the pilot never asked, so the defect was invisible for as long as the only caller wanted open items.
A page has no body FIELD, so the adapter documented the loss and dropped `spec.body`. That was honest, and still wrong for the first real user of the projection: eight pages, eight titles, eight statuses, and not one word about what the work was. - `createWork` writes the body as paragraph blocks: one block per LINE, runs of at most Notion's 2000-character ceiling, at most 100 blocks per request (the first batch rides the create, the rest are appended). Nothing is truncated and nothing is reflowed: `blocksToText(bodyBlocks(b)) === b` is an invariant with its own test, blank lines and a 2500-character line included. - reads read it back (`readContent`, default true): one request per page, plus one per 100 blocks, which `listWork` pays once per row. An item whose text silently reads as empty is how an agent gets sent to work on a description nobody gave it, so the opt-out is documented as the trade it is. - an idempotent re-create REPAIRS: content that is a strict PREFIX of the spec's text is completed by APPENDING the missing lines, and a page whose content is anything else is returned untouched. Append is the only operation this adapter ever uses on content it did not write. - a content write that fails is classified and says what it is: the page was filed, its content is incomplete, and re-running the same key appends the missing lines. And a 400 is no longer reported as a missing column option — Notion answers 400 for refused CONTENT as well. - `plainText` now reads `text.content` as well as `plain_text`: it returned '' for the runs this adapter had just written, which the round-trip test caught on its first run. `canTextSearch` still means TITLES ONLY: reading a body back is not searching it, and the tests assert both halves.
…c completes a copy The projection sent `labels: []` (hardcoded, so Notion's Labels column could never fill) and trusted its id map — which knows THAT a copy exists and never whether the copy is complete. - the authority's labels ride the create. A mirror board that cannot record them refuses it (fail-closed, which is what the Notion adapter does), and the operator opts out once with `labels: false` on that mirror; nothing here guesses which failure was about labels. - `resync` RE-ASSERTS the create for every item instead of trusting the map, so a copy filed before text and labels were carried is completed by the rebuild that was going to happen anyway. `mirror.written` says "created the mirror item" only when one was actually created. - `mirrorsList()` and the pilot's per-tick line report `labels`, so a projection filed without them is visible configuration rather than an always-empty column. - the core double now records `spec.labels` and completes a copy on an idempotent hit, like the real adapters do: a double that ignores a field cannot tell you the field is being dropped.
…efused `labelsProperty: Labels` sat in takumi.yaml, reached the provider factory, and vanished: the factory forwarded a hand-written subset (two or three options per adapter). A live projection filed eight pages with an empty Labels column and no error anywhere. The same class swallowed `trustedAuthors`, `statusMap`, `issueType`, `readContent`, `titleProperty` — every option an adapter documents and the wiring never passed. - one typed mapper per provider forwards every documented option, parsed as what it is (string, comma list, boolean, number, JSON object) and with no coercion: `readContent: yes` is refused, not read as false. - `assertKnownProviderOptions` refuses a key the adapter does not read and names the ones it does; a credential in configuration is refused as env-only, and a transport or a clock as code-only. A typo like `database:` for `databaseId:` is now a startup error instead of a silent no-op. - `--option key=value` (repeatable) reaches any documented option without this parser having to know every provider by heart. - a stray Chinese character in an English comment is gone: the repository is English-only.
…is closed - ADR-023 records the decision, its costs and its limits: content in batches of 100 blocks and runs of 2000 characters, the read-back that keeps `body` truthful, append-only repair, and the rule that a capability difference is a decision to make rather than a footnote to write. - ADR-017: rule 5 now says what is mirrored (state, title, text, labels, comments) and `labels: false` is the configuration for a board that cannot hold them; the live verification and the re-asserting resync are recorded. - ADR-022: the sentence claiming ADR-017's wiring was still open is corrected — it is closed, and the live-host report surface is the gap that remains. - README: the claim that no adapter had ever run against a live board or host was false (GitLab and the Notion mirror both have), the Notion row no longer says it has no labels, and the new capability is in the status table.
Both were found on a LIVE run and neither was visible in the suite: - #23: the projection carried a title and a status and nothing else — `labels: []` hardcoded, an id map trusted for completeness, and no way to hand Notion a body. - #24: `labelsProperty: Labels` reached the provider factory and vanished, because the factory forwarded a hand-written SUBSET of every adapter's options. The same seam was swallowing `trustedAuthors`, `statusMap`, `issueType`, `readContent`. The two class notes that came out of them are the durable half: a value that parses and then disappears at a seam looks exactly like a feature that does not work, and a placeholder filled with an empty literal can make a fail-closed guard unreachable.
ADR-023 listed "content is not edited, only appended" among the things not done. It is now a decision: rewriting a Notion page's content means deleting blocks this adapter did not write (a person's note beside the description, a correction, a checklist), because the host replaces content by removing and re-adding it. What a rebuild guarantees is therefore COMPLETENESS — nothing the authority carries is missing — and not currency of prose. The rejected alternative (takumi-owned and human-owned sections) is recorded with its reason.
… the path nobody deploys (#26, #27)
…ow-gates feat: mirrors closed out, and the deterministic rules gate a workflow (ADR-023, ADR-024)
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.
Carries this block into dev. The first PR for this block was opened against the previous branch of the stack, so it merged there; same commits, no content change.