kind was accepted at CREATE on both doors and dropped at UPDATE on both, so a task filed under the wrong kind could never be corrected.
The frontend made it worse by looking like it worked. TaskEditorView binds a Kind select, marks the form dirty, and has always sent kind in the update payload — the store even types it. The route ignored it, returned 200, the view optimistically updated, the toast said "Task saved", and the old value came back on reload. Silent success, the same class as #2709.
Found by trying to re-file a task as spike after deploying 0091. It could not be done; the task had to be recreated and the original cancelled.
What changed
One seam, not two doors.minted_kind() lives in services/notes.py because the REST route cannot import an MCP tool module, and a second spelling of the list is how the two doors would come to disagree. Both create and update route through it — so a bogus kind is now a readable error instead of a CheckViolationError surfacing as a 500. That was a second bug the issue did not name, and it existed on the create path all along.
TaskKind joins TaskStatus and TaskPriority as a real enum, and update_note validates task_kind exactly as it already validated those two. The field had been reaching setattr through the hasattr guard with no validation at all — invisible only because no door ever offered it.
The → plan question is answered in code, not left implicit. MINTABLE_KINDS is work/issue/spike, deliberately NARROWER than the column's CHECK. plan stays a valid stored value because historical plan-tasks carry it and must stay writable; no door hands out a new one, and the refusal names start_planning, because a caller reaching for it wants a plan. The whitelist and the door policy answer different questions and are not the same list.
Verification
CI green on 2e39dca — all six jobs, image built.
Every new test reads the value BACK. One asserting only that the call succeeded would have passed against the broken code: the route returned 200 while dropping the field, which is exactly how this survived until it was found by hand.
CI 4682 failed first, and the failure was correct. test_create_task_issue_sets_kind_provenance_and_systems asserted task_kind == "issue" and got a MagicMock — that test patches the whole notes_svc module to keep the database out, so reaching validation through notes_svc.minted_kind(...) returned a mock and the guard approved anything. The product was wrong, not the test: minted_kind is now imported by name, so a test that stubs the service to avoid I/O keeps the guard intact. That test now exercises the real validation and doubles as the regression guard.
Generalisable lesson worth keeping: a pure guard reached through a mockable module handle is not a guard.
Context
Milestone 312's steps 1–5 are already on main (PR #132) — the commit list below is longer than the diff because the rebase-merge rewrote those SHAs. The content diff here is this fix alone: 7 files, +170/−2.
Milestone 312 is now complete. Step 6 (the rulebook curation) went straight to the live instance as data and needed no code, and the sweep has been run for real: five rules carry checks, four never-verified sorting above the one verified today.
`kind` was accepted at CREATE on both doors and dropped at UPDATE on both, so a task filed under the wrong kind could never be corrected.
The frontend made it worse by looking like it worked. `TaskEditorView` binds a Kind select, marks the form dirty, and **has always sent `kind`** in the update payload — the store even types it. The route ignored it, returned 200, the view optimistically updated, the toast said "Task saved", and the old value came back on reload. Silent success, the same class as #2709.
Found by trying to re-file a task as `spike` after deploying 0091. It could not be done; the task had to be recreated and the original cancelled.
## What changed
**One seam, not two doors.** `minted_kind()` lives in `services/notes.py` because the REST route cannot import an MCP tool module, and a second spelling of the list is how the two doors would come to disagree. Both create and update route through it — so a bogus kind is now a readable error instead of a `CheckViolationError` surfacing as a 500. That was a **second bug the issue did not name**, and it existed on the create path all along.
**`TaskKind` joins `TaskStatus` and `TaskPriority` as a real enum**, and `update_note` validates `task_kind` exactly as it already validated those two. The field had been reaching `setattr` through the `hasattr` guard with no validation at all — invisible only because no door ever offered it.
**The `→ plan` question is answered in code**, not left implicit. `MINTABLE_KINDS` is work/issue/spike, deliberately NARROWER than the column's CHECK. `plan` stays a valid stored value because historical plan-tasks carry it and must stay writable; no door hands out a new one, and the refusal names `start_planning`, because a caller reaching for it wants a plan. The whitelist and the door policy answer different questions and are not the same list.
## Verification
CI green on `2e39dca` — all six jobs, image built.
**Every new test reads the value BACK.** One asserting only that the call succeeded would have passed against the broken code: the route returned 200 while dropping the field, which is exactly how this survived until it was found by hand.
CI 4682 failed first, and the failure was correct. `test_create_task_issue_sets_kind_provenance_and_systems` asserted `task_kind == "issue"` and got a MagicMock — that test patches the whole `notes_svc` module to keep the database out, so reaching validation through `notes_svc.minted_kind(...)` returned a mock and the guard approved anything. The product was wrong, not the test: `minted_kind` is now imported by name, so a test that stubs the service to avoid I/O keeps the guard intact. That test now exercises the real validation and doubles as the regression guard.
**Generalisable lesson worth keeping:** a pure guard reached through a mockable module handle is not a guard.
## Context
Milestone 312's steps 1–5 are already on `main` (PR #132) — the commit list below is longer than the diff because the rebase-merge rewrote those SHAs. The content diff here is this fix alone: 7 files, +170/−2.
Milestone 312 is now complete. Step 6 (the rulebook curation) went straight to the live instance as data and needed no code, and the sweep has been run for real: five rules carry checks, four never-verified sorting above the one verified today.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
A rulebook holds two kinds of row in one table. A NORM is a decision: no
truth value, changes only when its author changes it, and they know they
did. A CONSTRAINT asserts a fact about someone else's software, and goes
false with nobody present. Milestone 307's audit found nine stale sites;
every one was a constraint, and not one norm had rotted.
Three nullable columns so a rule can say how to check itself. expires_when
is a STATE, not a date — constraints expire when the ground moves, not on a
schedule. verified_at NULL means never checked and sorts FIRST in the sweep
to come: unexamined outranks examined-long-ago. Most rules set none of the
three; a null verify_with is the marker for "this is a decision, there is
nothing to go and check," and it only reads that way while it stays honest.
Nothing is backfilled and nothing is indexed. A migration cannot invent a
check any more than 0088 could invent a trigger, and the sweep reads a whole
rulebook — hundreds of rows, on operator demand, never on a request path.
Also, in the backup service the fields had to pass through:
- Restore now remaps arose_from_id through note_id_map. It has been exported
since 0088 and silently dropped on the way back in ever since, so every
restore lost every rule's provenance link.
- _dt_or_none, because _dt substitutes now() for an absent value. That is
right for created_at/updated_at and wrong here: a rule nobody ever checked
would restore looking freshly checked and fall to the bottom of the sweep
it should top.
Column additions do not move BACKUP_VERSION; only new sections do, as when
0088 added when_to_apply/tier/arose_from_id to the same helper.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
verify_with / expires_when now reach a rule through both doors and come back
on every read. The open question this step existed to settle was how to
UNSET a nullable field, and the answer is one convention per door:
- MCP: "" still means "leave unchanged" — an agent filling three fields must
not wipe the other five — so clearing is explicit, clear_fields=["..."].
Naming the field is the one form that cannot happen by accident.
- REST: a cleared form input arrives as "", and the service normalises "" to
NULL for every nullable rule column, so an emptied input does what it looks
like it does.
Two idioms, one outcome, and the normalisation is what makes the step-3 sweep
correct: `verify_with IS NOT NULL` would otherwise be true for every rule ever
touched through the UI, and the sweep would list the whole rulebook and mean
nothing. to_dict renders "" and NULL identically, so this is only visible
against a real column — hence the integration module rather than a mock.
Editing verify_with drops verified_at. A stamp certifies A CHECK, not a rule;
reword the check and the old stamp vouches for something that no longer
exists. Safe direction, same asymmetry as _valid_tier: a rule wrongly listed
as due costs one look, a rule wrongly vouched for costs the thing the sweep
exists to catch. Editing anything else leaves the stamp alone, or a rulebook
tidy-up would reset every constraint and the ordering would carry nothing.
Reads: rule_brief attaches `last_verified` ONLY to a rule that carries a
check — its presence is the signal, and it says both "this asserts a fact
that can go false" and "here is how long ago anyone confirmed it". "never"
rather than null, per #2483. The check text itself stays in get_rule; a
listing needs to know which rules can rot, not how to test them. Search hits
carry the full trio, since a hit is exactly the moment someone is about to
act on a rule.
Also folds in the #3078 finding, which had been sitting as a note: create_rule
now teaches that when_to_apply is the retrieval surface and must carry the
SYMPTOM — the words you would type while stuck — not just the situation.
fake_rule gains the three fields as None for the reason the helper already
documents one line up: unnamed, verify_with is a truthy MagicMock and every
stand-in rule would claim a check it does not have.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_update_rule_only_sends_non_default_fields pins that the MCP door
forwards only what the caller actually gave. `clear` is now always
forwarded — an empty tuple is "clear nothing", a value rather than an
absent argument — so the expected kwargs gained it. The property under
test is unchanged: everything left at its default still stays out.
Two tests added beside it while the shape is in view: naming a field for
clearing reaches the service as `clear`, and the check fields are
forwarded when given.
CI 4630 otherwise green — the integration lane ran all six of the new
real-Postgres cases (72 selected, was 66) and applied 0089 -> 0090.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The query the last two steps were storage for. `rules_due_for_verification`
returns every rule carrying a `verify_with`, ordered by `verified_at` ASC
NULLS FIRST, each row carrying the check IN FULL — the opposite call from
rule_brief, because the reader is about to go and run it.
NULLS FIRST is the ordering this turns on. Postgres sorts NULLs last on an
ASC ordering, which would put the rules nobody has ever confirmed BEHIND
every rule someone once looked at. Exactly backwards: a claim with no
evidence at all outranks an old one.
Rules with no check never appear, and that is the property that keeps the
list worth reading. Most rules are decisions — no truth value, nothing to go
and check. If they appeared here the sweep would be the rulebook.
`mark_rule_verified(rule_id, still_true)` closes the loop, asymmetrically:
passing writes a stamp, FAILING WRITES NOTHING. There is no "verified false"
state because a rule whose check failed is not in a special condition, it is
wrong — and recording the failure as a flag would let it sit there being
false with the sweep satisfied that someone had looked. So it stays at the
top until someone corrects or retires it, and the response says so.
An unrecognised `tier` filter raises rather than falling back. _valid_tier's
silent always_on default is right for a WRITE — a typo should leave a rule
binding — and wrong for a FILTER, where the same fallback quietly answers a
different question and returns a short list that reads as good news.
Deliberately NOT filterable by project: a project reaches rules through
project scope, subscriptions, always-on rulebooks and exclusions, and a
filter missing one of those paths would UNDER-report — the exact failure
this surface exists to prevent. Said so in the docstring rather than
shipping a half-correct filter.
Ownership-scoped like every other rule read (owned rulebook, or owned
project), in ONE statement with an OR across the XOR rather than two queries
merged in Python, so the ordering is the database's and cannot disagree with
itself. Note that rules have no sharing ACL in this schema — no rule_shares,
no rulebook_shares — so there is no wider set for access.py to consult here.
Also fixes a test title that had been lying for ten tools: "all sixteen
tools" asserted 26. The number now lives only in the assertion.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three surfaces already agree on WHERE a rule goes — the using-scribe
skill's "Where a new rule goes" section and both tool docstrings frame it
as one question, who should this bind. What they did not say is that the
two homes want differently SHAPED rules, and one deferral was actively
misleading.
`create_project_rule` said `tier: "always_on" or "conditional" — see
create_rule`. That imports a bar calibrated for a different blast radius.
On a rulebook rule always_on means every session in every project, so the
test is severe: the trigger must be nameless. A project rule is already
scoped by construction, so always_on costs only that project's sessions —
and being specific, which the family test treats as the signal for
conditional, is what project rules are FOR. The instance's own data says
so: rules 78, 115 and 119 are all project rules and all always_on.
Not zero bar, a different one: conditional is right when the rule is about
one AREA of a large project, because forty always-on rules on one project
reproduces locally the preload bloat milestone 307 fixed globally.
Also:
- create_rule now says to write the general form WITHOUT hedging for
exceptions — a project needing to narrow it writes its own and links
with overrides/elaborates. A rulebook rule padded with "unless…" for two
projects is two project rules that were never written. Only the project
side mentioned that relationship; the side that benefits from it did not.
- arose_from_id: reach for it harder on a project rule, which usually comes
from one traceable incident in the repo, where a family rule is more
often a standing preference with no single origin.
- system_ids is worth setting on a project rule too — it is what lets a
conditional one arrive with its area.
- when_to_apply no longer claims to "decide" the tier here, which stopped
being true one entry down.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rule 27 — the milestone was backend-only until this. Four surfaces:
RULE EDITOR — verify_with and expires_when under a legend that asks the
actual question ("Can this rule go stale?") and says empty is the normal
answer, because most rules are decisions and a form that implies a missing
field would get them filled in out of tidiness. When the SAVED rule carries
a check, the stamp shows with Still true / No longer true beside it. The
stamp reads the stored value, not the draft: an unsaved edit to the textarea
has not been run against anything.
SWEEP PANE — its own surface, not a filter on the rule list. That list can
only ever show one topic of one rulebook, and a rule that has gone false
belongs to no one rulebook; filtering it would under-report, which is the
failure this whole surface exists to catch. Reached from the rulebook list,
below the rulebooks, because that is where you go to look at rules.
RULE ROWS — a chip only on rules carrying a check, so its presence is the
signal. PROJECT RULES TAB — the check shows beside `why` when a rule has
one, read-only: that tab is the project's view of what binds it.
NO AGE-GRADED COLOUR anywhere, deliberately. The sweep is already ordered by
urgency, so a red/amber ramp would restate the ordering AND require an
invented "stale after N days" threshold — a magic number nobody could defend
and the first thing to go out of date. --fs-overdue is error red and reserved
for a broken promise like a missed due date; a verification age is not one,
and colouring it that way makes a rule someone just wrote look broken. Only
"never" is marked, because it is categorically different from a date rather
than a worse one — and it is marked by weight, not hue.
An empty sweep says "Nothing to check", not nothing: good news must not read
as a broken page.
Two chips (tier, then verification) turned out byte-identical, so .rule-chip
moves to rules-shared.css and snippet #2906 is updated to match rather than
left describing a file that has moved on. Its header comment counted the
panes it served; that count went stale the moment a fourth arrived, so it no
longer counts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A spike is a shape the other kinds cannot hold. `work` ships a change;
`issue` fixes something broken. A spike is time-boxed and its output is
KNOWLEDGE — it succeeds by producing an answer, and nothing ships at the end
of it. Filing one as `work` makes a finished investigation look like an
abandoned change, which is why the distinction earns a value rather than a
convention.
It is also the record a failed check asks for. This milestone gave rules a
verify_with; when one fails the rule is wrong, and the next move is often to
go and find out what replaced it. notes.arose_from_id already exists (0065),
so constraint -> spike provenance needed no schema at all — only a docstring
saying it is there.
Rule 36: the value and the widened CHECK land in the same migration, DROP
then ADD, exactly as 0065 did for 'issue'. The two whitelists live in one
tuple each so upgrade and downgrade cannot disagree about what the list was
on either side. The downgrade demotes existing spikes to 'work' first —
lossy, deliberately, because the alternative is a downgrade that fails on
real data, and one that says what it did beats one that cannot run.
'plan' stays whitelisted though retired: historical plan-tasks carry it, and
a row that cannot be rewritten cannot be edited, restored or migrated.
The integration test asserts both halves. A test that only proved 'spike' is
accepted would pass just as happily against a table whose CHECK had been
dropped and never re-added — which is the other way rule 36's failure
happens — so an unknown kind is asserted to still raise.
Not in scope, deliberately: any special lifecycle, time-box enforcement, or
gating relationship. It is a kind, not a workflow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The spike CHECK tests constructed Note(is_task=True). `is_task` is a derived
read-only property — `status is not None` — so SQLAlchemy raised
"property 'is_task' of 'Note' object has no setter" before any row reached
the database. All three failed for that, not for anything about migration
0091; the other 80 integration tests passed, including 0090's.
status="todo" is what makes a note a task. Noted inline, since the field
appears in to_dict output and reads like an ordinary column from there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`kind` was accepted at CREATE on both doors and dropped at UPDATE on both:
update_task had no such parameter, and the REST PATCH allow-list never read
the field. So a task filed under the wrong kind could never be corrected.
The frontend made it worse by looking like it worked. TaskEditorView binds a
Kind select, marks the form dirty, and HAS ALWAYS SENT `kind` in the update
payload — the store even types it. The route ignored it, returned 200, the
view optimistically updated, the toast said "Task saved", and the old value
came back on reload. Silent success, same class as #2709.
Found by trying to re-file #3126 as a spike after deploying 0091. It could
not be done; the task had to be recreated as #3128 and the original
cancelled.
One seam, not two doors. `minted_kind()` lives in services/notes.py because
the REST route cannot import an MCP tool module and a second spelling of the
list is how the doors would come to disagree. Both create and update route
through it, so a bogus kind is now a readable error rather than a
CheckViolationError surfacing as a 500.
TaskKind joins TaskStatus and TaskPriority as a real enum, and update_note
validates task_kind exactly as it already validated those two — the field
had been reaching setattr through the hasattr guard with no validation at
all, unnoticed only because no door ever offered it.
The `-> plan` question #3129 raised is answered in code rather than left
implicit: MINTABLE_KINDS is work/issue/spike, deliberately NARROWER than the
column's CHECK. `plan` stays a valid stored value because historical
plan-tasks carry it and must stay writable; it is simply not a value any
door hands out, and the refusal names start_planning because a caller
reaching for it wants a plan. The whitelist and the policy answer different
questions and are not the same list.
Every new test reads the value BACK. One that only asserted the call
succeeded would have passed against the broken code — the route returned 200
while dropping the field, which is how this survived long enough to be found
by hand.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI 4682: test_create_task_issue_sets_kind_provenance_and_systems asserted
task_kind == "issue" and got a MagicMock. That test patches the whole
notes_svc module to keep the database out, so reaching validation through
`notes_svc.minted_kind(...)` handed back a mock — the guard approved
anything and returned nothing real.
The product was wrong, not the test. minted_kind is pure validation, not a
service call, so it is imported by name. A test that stubs the service to
avoid I/O now keeps the guard intact, which is the behaviour you want from
a guard: the only way to disable it should be to say so explicitly.
That test now exercises the real validation, so it doubles as the guard
against this recurring.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR #132 was rebase-merged, so main carries rewritten copies of dev's
milestone-312 commits under new SHAs. The merge base stayed at 02c1e37, so
a dev->main merge tried to replay all eight already-landed commits and
collided in the two test files both sides had touched.
Resolved by taking dev's version of each: dev's copy is main's content plus
the #3129 additions, verified as a strict superset before resolving rather
than assumed. The merged tree is identical to dev's — asserted below, not
eyeballed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts:
# tests/test_integration_task_kind_spike.py
# tests/test_mcp_tool_tasks_kind.py
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
kindwas accepted at CREATE on both doors and dropped at UPDATE on both, so a task filed under the wrong kind could never be corrected.The frontend made it worse by looking like it worked.
TaskEditorViewbinds a Kind select, marks the form dirty, and has always sentkindin the update payload — the store even types it. The route ignored it, returned 200, the view optimistically updated, the toast said "Task saved", and the old value came back on reload. Silent success, the same class as #2709.Found by trying to re-file a task as
spikeafter deploying 0091. It could not be done; the task had to be recreated and the original cancelled.What changed
One seam, not two doors.
minted_kind()lives inservices/notes.pybecause the REST route cannot import an MCP tool module, and a second spelling of the list is how the two doors would come to disagree. Both create and update route through it — so a bogus kind is now a readable error instead of aCheckViolationErrorsurfacing as a 500. That was a second bug the issue did not name, and it existed on the create path all along.TaskKindjoinsTaskStatusandTaskPriorityas a real enum, andupdate_notevalidatestask_kindexactly as it already validated those two. The field had been reachingsetattrthrough thehasattrguard with no validation at all — invisible only because no door ever offered it.The
→ planquestion is answered in code, not left implicit.MINTABLE_KINDSis work/issue/spike, deliberately NARROWER than the column's CHECK.planstays a valid stored value because historical plan-tasks carry it and must stay writable; no door hands out a new one, and the refusal namesstart_planning, because a caller reaching for it wants a plan. The whitelist and the door policy answer different questions and are not the same list.Verification
CI green on
2e39dca— all six jobs, image built.Every new test reads the value BACK. One asserting only that the call succeeded would have passed against the broken code: the route returned 200 while dropping the field, which is exactly how this survived until it was found by hand.
CI 4682 failed first, and the failure was correct.
test_create_task_issue_sets_kind_provenance_and_systemsassertedtask_kind == "issue"and got a MagicMock — that test patches the wholenotes_svcmodule to keep the database out, so reaching validation throughnotes_svc.minted_kind(...)returned a mock and the guard approved anything. The product was wrong, not the test:minted_kindis now imported by name, so a test that stubs the service to avoid I/O keeps the guard intact. That test now exercises the real validation and doubles as the regression guard.Generalisable lesson worth keeping: a pure guard reached through a mockable module handle is not a guard.
Context
Milestone 312's steps 1–5 are already on
main(PR #132) — the commit list below is longer than the diff because the rebase-merge rewrote those SHAs. The content diff here is this fix alone: 7 files, +170/−2.Milestone 312 is now complete. Step 6 (the rulebook curation) went straight to the live instance as data and needed no code, and the sweep has been run for real: five rules carry checks, four never-verified sorting above the one verified today.
🤖 Generated with Claude Code
clear(#3096, milestone 312 step 2)Rule 27 — the milestone was backend-only until this. Four surfaces: RULE EDITOR — verify_with and expires_when under a legend that asks the actual question ("Can this rule go stale?") and says empty is the normal answer, because most rules are decisions and a form that implies a missing field would get them filled in out of tidiness. When the SAVED rule carries a check, the stamp shows with Still true / No longer true beside it. The stamp reads the stored value, not the draft: an unsaved edit to the textarea has not been run against anything. SWEEP PANE — its own surface, not a filter on the rule list. That list can only ever show one topic of one rulebook, and a rule that has gone false belongs to no one rulebook; filtering it would under-report, which is the failure this whole surface exists to catch. Reached from the rulebook list, below the rulebooks, because that is where you go to look at rules. RULE ROWS — a chip only on rules carrying a check, so its presence is the signal. PROJECT RULES TAB — the check shows beside `why` when a rule has one, read-only: that tab is the project's view of what binds it. NO AGE-GRADED COLOUR anywhere, deliberately. The sweep is already ordered by urgency, so a red/amber ramp would restate the ordering AND require an invented "stale after N days" threshold — a magic number nobody could defend and the first thing to go out of date. --fs-overdue is error red and reserved for a broken promise like a missed due date; a verification age is not one, and colouring it that way makes a rule someone just wrote look broken. Only "never" is marked, because it is categorically different from a date rather than a worse one — and it is marked by weight, not hue. An empty sweep says "Nothing to check", not nothing: good news must not read as a broken page. Two chips (tier, then verification) turned out byte-identical, so .rule-chip moves to rules-shared.css and snippet #2906 is updated to match rather than left describing a file that has moved on. Its header comment counted the panes it served; that count went stale the moment a fourth arrived, so it no longer counts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>