update_lesson did not take system_ids while create_lesson did, so a lesson could be filed under a System exactly once — at the moment of least information. A lesson is usually written at the end of a piece of work, which is precisely when that argument gets dropped, and afterwards the record could never be filed at all. A System tag is how list_system_records gathers an area's pile, so an untagged lesson is reachable by search and by nothing else.
Filling in the rest of the table found three more gaps and showed the issue's own generalisation was wrong. It read as "the REST door can do something the MCP door cannot", on three instances in a row:
kind
MCP create
MCP update
REST create
REST update
lesson
✅
❌
✅
✅
preference
❌
✅
—
—
note
✅
✅
❌
❌
process
❌
❌
—
—
create_preference is the exact mirror — the same capability missing from the opposite end of the same lifecycle. routes/notes.py handled system_idsnowhere, which runs the opposite way round from the premise. And processes took it at neither door despite being notes, and therefore taggable in the data model all along.
The real pattern is that whichever door nobody exercised for a kind is the one that never grew the parameter — not something a reviewer reliably catches, because each door is only ever read on its own.
Milestones are not a fifth gap.RecordSystem.note_id is a ForeignKey("notes.id") and milestones are their own table, so they cannot be tagged at any door by construction. Pinned in the test so the next pass doesn't re-open it.
Then: tagging ran as the wrong user in four places
set_record_systems is not a dumb setter — it runs its own can_write_note and links only the Systems the given user can read. Four of twenty call sites handed it the record's owner, which did two quiet things at once: the access check became trivially true (an owner can always write their own record), and the System filter used the owner's visibility instead of the actor's.
On a single-user install neither is observable. With a share it is an editor acting with the owner's reach — the shape rule 47 exists to prevent, and the same reasoning routes/notes.py already spells out for set_supersedes two lines away.
Not a permission change. Every one of the four establishes the caller's write access first, so nobody gains or loses the ability to edit anything. What changed is whose reach the tagging runs with.
The last was written earlier in the same session as the sweep that found it, with a comment confidently explaining why the owner was correct.
The guards are the deliverable
Fixing update_lesson alone would have left the shape that produced it intact. tests/test_system_tagging_door_parity.py carries both:
test_the_mcp_door_takes_system_ids_at_both_ends / test_the_rest_door_handles_system_ids assert the table: every kind taggable anywhere is taggable everywhere it is written.
test_every_tool_module_that_tags_is_in_the_registry keys on the signature via AST, not a grep, so a fifth kind is dragged into the parity checks by its own first use of the parameter. (A grep also hits projects.py, search.py and systems.py, which name the argument only in prose — the fix for that is a structural check, not a skip list that would itself go stale.)
test_every_tagging_write_acts_as_the_caller walks every set_record_systems call in src/ and asserts the first argument is a bare local, since an attribute access is an owner by construction.
Verification
Each guard was run against the working tree and against git show HEAD:, so it is known that they can fail and on what:
parity guard — PASS on the fix; 5 failures pre-fix (update_lesson, routes/notes.py, create_process, update_process, create_preference).
caller guard — PASS on the fix; 4 offenders pre-fix, named above.
CI 7205 and 7206 green on dev, all six jobs.
Known and deliberately not done here
Reads are untouched. list_record_systems(owner_uid, …) gates on reading the note, which the caller can do anyway, so it returns the same list either way — a different operation, and churning it would add noise without changing behaviour.
Two commits: `e2e1ea3` and `43ed7f4`.
## The issue as filed, and what it turned out to be
`update_lesson` did not take `system_ids` while `create_lesson` did, so a lesson could be filed under a System exactly once — at the moment of least information. A lesson is usually written at the end of a piece of work, which is precisely when that argument gets dropped, and afterwards the record could never be filed at all. A System tag is how `list_system_records` gathers an area's pile, so an untagged lesson is reachable by search and by nothing else.
Filling in the rest of the table found three more gaps and showed the issue's own generalisation was wrong. It read as "the REST door can do something the MCP door cannot", on three instances in a row:
| kind | MCP create | MCP update | REST create | REST update |
|---|---|---|---|---|
| lesson | ✅ | ❌ | ✅ | ✅ |
| preference | ❌ | ✅ | — | — |
| note | ✅ | ✅ | ❌ | ❌ |
| process | ❌ | ❌ | — | — |
`create_preference` is the exact **mirror** — the same capability missing from the opposite end of the same lifecycle. `routes/notes.py` handled `system_ids` **nowhere**, which runs the opposite way round from the premise. And processes took it at neither door despite being notes, and therefore taggable in the data model all along.
The real pattern is that **whichever door nobody exercised for a kind is the one that never grew the parameter** — not something a reviewer reliably catches, because each door is only ever read on its own.
**Milestones are not a fifth gap.** `RecordSystem.note_id` is a `ForeignKey("notes.id")` and milestones are their own table, so they cannot be tagged at any door by construction. Pinned in the test so the next pass doesn't re-open it.
## Then: tagging ran as the wrong user in four places
`set_record_systems` is not a dumb setter — it runs its own `can_write_note` and links only the Systems the given user can **read**. Four of twenty call sites handed it the record's owner, which did two quiet things at once: the access check became trivially true (an owner can always write their own record), and the System filter used the owner's visibility instead of the actor's.
On a single-user install neither is observable. With a share it is an editor acting with the owner's reach — the shape rule 47 exists to prevent, and the same reasoning `routes/notes.py` already spells out for `set_supersedes` two lines away.
**Not a permission change.** Every one of the four establishes the caller's write access first, so nobody gains or loses the ability to edit anything. What changed is whose reach the tagging runs with.
```
routes/lessons.py:243 owner_uid -> uid
routes/snippets.py:211 owner_uid -> uid
mcp/tools/snippets.py:486 note.user_id -> uid
mcp/tools/processes.py:196 note.user_id -> uid
```
The last was written earlier in the same session as the sweep that found it, with a comment confidently explaining why the owner was correct.
## The guards are the deliverable
Fixing `update_lesson` alone would have left the shape that produced it intact. `tests/test_system_tagging_door_parity.py` carries both:
- **`test_the_mcp_door_takes_system_ids_at_both_ends`** / **`test_the_rest_door_handles_system_ids`** assert the *table*: every kind taggable anywhere is taggable everywhere it is written.
- **`test_every_tool_module_that_tags_is_in_the_registry`** keys on the **signature** via AST, not a grep, so a fifth kind is dragged into the parity checks by its own first use of the parameter. (A grep also hits `projects.py`, `search.py` and `systems.py`, which name the argument only in prose — the fix for that is a structural check, not a skip list that would itself go stale.)
- **`test_every_tagging_write_acts_as_the_caller`** walks every `set_record_systems` call in `src/` and asserts the first argument is a bare local, since an attribute access is an owner by construction.
## Verification
Each guard was run against the working tree **and** against `git show HEAD:`, so it is known that they can fail and on what:
- parity guard — **PASS** on the fix; **5 failures** pre-fix (`update_lesson`, `routes/notes.py`, `create_process`, `update_process`, `create_preference`).
- caller guard — **PASS** on the fix; **4 offenders** pre-fix, named above.
CI 7205 and 7206 green on `dev`, all six jobs.
## Known and deliberately not done here
Reads are untouched. `list_record_systems(owner_uid, …)` gates on reading the *note*, which the caller can do anyway, so it returns the same list either way — a different operation, and churning it would add noise without changing behaviour.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
A System tag is how `list_system_records` gathers an area's pile, so a
record that cannot be tagged is reachable by search and by nothing else.
`update_lesson` did not take `system_ids` while `create_lesson` did, which
made tagging available exactly once — at the moment of least information.
A lesson is usually written at the end of a piece of work, which is
precisely when that argument gets dropped, and after that the record could
never be filed at all.
Filling in the rest of the table found three more gaps, and the issue's own
generalisation was wrong. It read as "the REST door can do something the
MCP door cannot", on three instances in a row. In fact:
* `create_preference` is the exact MIRROR of the lesson bug — update
takes `system_ids`, create does not. The same capability missing from
the opposite end of the same lifecycle.
* `routes/notes.py` handled `system_ids` NOWHERE, while the MCP door
handled both ends. That runs the opposite way round from the premise.
* `create_process` / `update_process` took it at neither door, though a
process is a note and has always been taggable in the data model.
The real pattern is that whichever door nobody exercised for a kind is the
one that never grew the parameter — which is a better statement of #4248
than the one recorded there, and is not something a reviewer reliably
notices, because each door is only ever read on its own.
Milestones are NOT a fifth gap. `RecordSystem.note_id` is a ForeignKey to
`notes.id` and milestones are their own table, so they cannot be tagged at
any door by construction. Pinned in the test so the next pass does not
re-open it.
So the guard is the point, not the four parameters. `update_lesson` alone
would have left the shape that produced it intact. The new test asserts the
TABLE — every kind taggable anywhere is taggable everywhere it is written —
and keys the registry-coverage check on the SIGNATURE rather than on a
grep, so a module that only names the argument in prose is not swept in and
no hand-kept skip list can go stale. A fifth kind fails there rather than
shipping half-wired, the same reasoning test_derived_mirror_generic_door.py
records for derived mirrors (#3734).
One inconsistency found and deliberately not changed here: for the same
operation `routes/tasks.py` scopes `set_record_systems` by the caller while
`routes/lessons.py` and `routes/snippets.py` scope it by the owner. The new
notes code follows tasks.py and says why in a comment (#47 — an editor-share
holder should tag from what they can see rather than inherit the owner's
reach). Recorded in #4249 rather than fixed as a drive-by.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
`set_record_systems` is not a dumb setter. It runs its own `can_write_note`
and then links only the Systems the given user can READ. Four of twenty
call sites handed it the record's owner instead of the acting user, which
did two quiet things at once: the access check became trivially true, since
an owner can always write their own record, and the System filter used the
owner's visibility rather than the actor's.
On a single-user install neither is observable. With a share it is an
editor acting with the owner's reach — the shape rule 47 exists to prevent,
and the same reasoning routes/notes.py already spells out for
`set_supersedes` two lines away.
This is NOT a permission change. Every one of the four sites establishes
the caller's write access first: routes/lessons.py and routes/snippets.py
call `can_write_note(uid, …)`, mcp/tools/processes.py does the same, and
mcp/tools/snippets.py reaches `set_record_systems` only after
`update_snippet` has raised PermissionError if the caller may not write. So
nobody gains or loses the ability to edit anything. What changes is whose
reach the tagging runs with, which is exactly the kind of difference that
survives review because every call site reads fine on its own.
The four:
routes/lessons.py:243 owner_uid -> uid
routes/snippets.py:211 owner_uid -> uid
mcp/tools/snippets.py:486 note.user_id -> uid
mcp/tools/processes.py:196 note.user_id -> uid
The last one was written earlier in this same session, an hour before the
sweep that found it, with a comment confidently explaining why the owner
was correct. That is the argument for the guard rather than for care: the
unified stance was known and still got it wrong at the next opportunity.
So the guard is the point again. `test_every_tagging_write_acts_as_the_caller`
walks every `set_record_systems` call in src/ and asserts the first argument
is a bare local named `uid` or `user_id` — an attribute access is a record's
owner by construction. Verified against `git show HEAD:` as well as the
working tree: clean now, four offenders on the code it replaces.
Reads are deliberately untouched. `list_record_systems(owner_uid, …)` gates
on reading the NOTE, which the caller can do anyway, so it returns the same
list either way; it is a different operation and churning it would add noise
without changing behaviour.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
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.
Two commits:
e2e1ea3and43ed7f4.The issue as filed, and what it turned out to be
update_lessondid not takesystem_idswhilecreate_lessondid, so a lesson could be filed under a System exactly once — at the moment of least information. A lesson is usually written at the end of a piece of work, which is precisely when that argument gets dropped, and afterwards the record could never be filed at all. A System tag is howlist_system_recordsgathers an area's pile, so an untagged lesson is reachable by search and by nothing else.Filling in the rest of the table found three more gaps and showed the issue's own generalisation was wrong. It read as "the REST door can do something the MCP door cannot", on three instances in a row:
create_preferenceis the exact mirror — the same capability missing from the opposite end of the same lifecycle.routes/notes.pyhandledsystem_idsnowhere, which runs the opposite way round from the premise. And processes took it at neither door despite being notes, and therefore taggable in the data model all along.The real pattern is that whichever door nobody exercised for a kind is the one that never grew the parameter — not something a reviewer reliably catches, because each door is only ever read on its own.
Milestones are not a fifth gap.
RecordSystem.note_idis aForeignKey("notes.id")and milestones are their own table, so they cannot be tagged at any door by construction. Pinned in the test so the next pass doesn't re-open it.Then: tagging ran as the wrong user in four places
set_record_systemsis not a dumb setter — it runs its owncan_write_noteand links only the Systems the given user can read. Four of twenty call sites handed it the record's owner, which did two quiet things at once: the access check became trivially true (an owner can always write their own record), and the System filter used the owner's visibility instead of the actor's.On a single-user install neither is observable. With a share it is an editor acting with the owner's reach — the shape rule 47 exists to prevent, and the same reasoning
routes/notes.pyalready spells out forset_supersedestwo lines away.Not a permission change. Every one of the four establishes the caller's write access first, so nobody gains or loses the ability to edit anything. What changed is whose reach the tagging runs with.
The last was written earlier in the same session as the sweep that found it, with a comment confidently explaining why the owner was correct.
The guards are the deliverable
Fixing
update_lessonalone would have left the shape that produced it intact.tests/test_system_tagging_door_parity.pycarries both:test_the_mcp_door_takes_system_ids_at_both_ends/test_the_rest_door_handles_system_idsassert the table: every kind taggable anywhere is taggable everywhere it is written.test_every_tool_module_that_tags_is_in_the_registrykeys on the signature via AST, not a grep, so a fifth kind is dragged into the parity checks by its own first use of the parameter. (A grep also hitsprojects.py,search.pyandsystems.py, which name the argument only in prose — the fix for that is a structural check, not a skip list that would itself go stale.)test_every_tagging_write_acts_as_the_callerwalks everyset_record_systemscall insrc/and asserts the first argument is a bare local, since an attribute access is an owner by construction.Verification
Each guard was run against the working tree and against
git show HEAD:, so it is known that they can fail and on what:update_lesson,routes/notes.py,create_process,update_process,create_preference).CI 7205 and 7206 green on
dev, all six jobs.Known and deliberately not done here
Reads are untouched.
list_record_systems(owner_uid, …)gates on reading the note, which the caller can do anyway, so it returns the same list either way — a different operation, and churning it would add noise without changing behaviour.🤖 Generated with Claude Code
https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
A System tag is how `list_system_records` gathers an area's pile, so a record that cannot be tagged is reachable by search and by nothing else. `update_lesson` did not take `system_ids` while `create_lesson` did, which made tagging available exactly once — at the moment of least information. A lesson is usually written at the end of a piece of work, which is precisely when that argument gets dropped, and after that the record could never be filed at all. Filling in the rest of the table found three more gaps, and the issue's own generalisation was wrong. It read as "the REST door can do something the MCP door cannot", on three instances in a row. In fact: * `create_preference` is the exact MIRROR of the lesson bug — update takes `system_ids`, create does not. The same capability missing from the opposite end of the same lifecycle. * `routes/notes.py` handled `system_ids` NOWHERE, while the MCP door handled both ends. That runs the opposite way round from the premise. * `create_process` / `update_process` took it at neither door, though a process is a note and has always been taggable in the data model. The real pattern is that whichever door nobody exercised for a kind is the one that never grew the parameter — which is a better statement of #4248 than the one recorded there, and is not something a reviewer reliably notices, because each door is only ever read on its own. Milestones are NOT a fifth gap. `RecordSystem.note_id` is a ForeignKey to `notes.id` and milestones are their own table, so they cannot be tagged at any door by construction. Pinned in the test so the next pass does not re-open it. So the guard is the point, not the four parameters. `update_lesson` alone would have left the shape that produced it intact. The new test asserts the TABLE — every kind taggable anywhere is taggable everywhere it is written — and keys the registry-coverage check on the SIGNATURE rather than on a grep, so a module that only names the argument in prose is not swept in and no hand-kept skip list can go stale. A fifth kind fails there rather than shipping half-wired, the same reasoning test_derived_mirror_generic_door.py records for derived mirrors (#3734). One inconsistency found and deliberately not changed here: for the same operation `routes/tasks.py` scopes `set_record_systems` by the caller while `routes/lessons.py` and `routes/snippets.py` scope it by the owner. The new notes code follows tasks.py and says why in a comment (#47 — an editor-share holder should tag from what they can see rather than inherit the owner's reach). Recorded in #4249 rather than fixed as a drive-by. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy