duckworth: row 206 manifest review of 631a256ba (duckworth-0169)
This commit is contained in:
jason.woltje committed
1 parent
a28967e78a
commit
84458606db
2 files changed
+122
No files matched your search
@@ -0,0 +1,121 @@
|
||||
# Row 206 (193b2, #1704) manifest review: 631a256ba (duckworth-0169)
|
||||
|
||||
Reviewer: Duckworth. Author: Launchpad. Request: launchpad-0030.
|
||||
Candidate: aca16404c..631a256ba, 2 commits (16b9c6df6 build, 631a256ba sage-2818 rulings), 22 paths.
|
||||
Bar: brief "193b2, thread shares" (docs/plans/2026-10-20_human-users-build.md) and design e2d161e2e
|
||||
sections 4.3, 5.2-5.6 and 8.
|
||||
Method: static git reads only. Gander holds the slot, so no node, no fetch and no test run.
|
||||
|
||||
## Verdict: APPROVE, with C1 and C2 for the 275 rebase round
|
||||
|
||||
No blockers. Sage's gate runs (bus suite, cli suite, every scripts/test-*.sh, the mutants) are still
|
||||
to come, and this approval doesn't stand in for them.
|
||||
|
||||
## Checked
|
||||
|
||||
- `#readable` (broker.mjs:823-834) follows design 5.4 in order. Rule 4 comes first: a tester
|
||||
session, or a thread the tester owns, reads nothing. Then the Q2 human-principal guard. A reader
|
||||
matches as the business human. Rule 1 (`#shared` to me or `*`), rule 2 (`#covered` for a user
|
||||
caller) and rule 3 (`#covered` for a user owner) follow.
|
||||
- Session shapes: bindHuman (:1067) sets `human: b.human`. bindLaunch (:995), tester (:1075), user
|
||||
(:1086) and reader (:1092) set `human: false`. So the guard at :825 lets through only the human,
|
||||
a reader and a user. Agent sessions fail it, and so does the tester's reader view (:4301), which
|
||||
rule 4 also stops.
|
||||
- `#shared` (:836-838) reads the latest row per (thread, target) through the index
|
||||
`thread_shares_thread (thread, target, seq)`.
|
||||
- `#covered` (:841-851) reads the access document once per call and covers nothing when the read
|
||||
fails. It calls covers() at level viewer. covers() checks `grant.business` itself
|
||||
(vocabulary.mjs:295), so dropping #userGrants' business filter changes nothing.
|
||||
- `#threadRead` (:769-775) is used only by thread.messages. The owner path keeps
|
||||
`#grantedThread`. send, open, share, unshare, shares and session.list keep `#thread`'s owner
|
||||
check.
|
||||
- share/unshare (:5901-5915): `#human` runs unless the caller is a user, then the owner check and
|
||||
the collaborator grant. `#shareTarget` refuses a self-share, a malformed id and a reserved id.
|
||||
unshare requires a share in force. share checks the user target's coverage, then share-exists.
|
||||
Each call writes one row.
|
||||
- `thread.shares` (:5917-5925) lists the latest row per target where the op is share.
|
||||
- `thread.list {shared:true}` adds `owner` and nothing else. The plain list keeps its shape
|
||||
(test :85-89).
|
||||
- Migration 3l-3m.sql has the same text as the schema.sql hunk. It adds a table, an index and three
|
||||
triggers and changes no existing row. The thread FK targets `threads.id UNIQUE`, and
|
||||
`foreign_keys=ON` is set (store.mjs:189), so the FK test at :405 is real. The step creates an
|
||||
empty AUTOINCREMENT table, which adds no sqlite_sequence row. The migration test compares every
|
||||
table but meta, sqlite_sequence included (:374-389).
|
||||
- Restore guard: `thread_shares` is in COUNTED (stack-release.sh:420), in LISTED
|
||||
(stack-release.test:861), in timedTables (store.mjs) and in the broker clock list. It has `at`,
|
||||
so it belongs in the counted list.
|
||||
- Pins: schema.test is at 23 tables and 89 triggers (README says 90; schema.test drops one first).
|
||||
USER_VERBS, every SCHEMA_VERSION replace, every engine copy list (3l-3m.sql added) and every
|
||||
`later3l` / rows() exclusion is updated. comment-migration's future step is now 3m→3n. The
|
||||
untouched files that mention 3l (end-launch, message-filter, thread-claims, threads, thread-cli,
|
||||
thread-launch, chat) mention it only in a comment.
|
||||
- Hash pins: tests and scripts pin no 64-hex value next to broker.mjs, store.mjs, schema.sql,
|
||||
stack-release.sh or README.md. The only other match for the 3l digest is the fixture check at
|
||||
thread-shares.test:349.
|
||||
- Drift: local origin/refactor is aca16404c, the base, so the drift is zero. The ref is unfetched
|
||||
(slot hold). Recheck it on the post-253/275 rebase.
|
||||
|
||||
## Mutants (design 8, brief: 2, 3, 4, 7, 8, 9)
|
||||
|
||||
Each kill is an assertion, not a throw before one:
|
||||
|
||||
- M2: delete rule 2. bob keeps a collaborator grant on p2, so the gate passes and he reads A. The
|
||||
`code() === 'thread-not-owned'` check at :132 fails. The carol check at :117 also kills it.
|
||||
- M3: delete rule 3. The human reads A after alice's grant is removed, and :166 fails.
|
||||
- M4: any share row counts. bob reads after the unshare, and :183 fails.
|
||||
- M7: send through `#threadRead`. The loop at :91-98 runs after the share to bob at :82, so bob's
|
||||
send passes and `code` returns null.
|
||||
- M8: delete the share-not-covered check. The carol share passes, and :209 fails.
|
||||
- M9: `#threadRow` in share/unshare. bob's grant covers p1, so his share to `*` passes, and :98
|
||||
fails.
|
||||
- Q2 probe: delete the guard at :825. The agent bound to A matches the `*` share and rules 2 and 3
|
||||
pass, so its trail shows `sent` and :330 fails. The precondition at :325 shows it was hidden
|
||||
before.
|
||||
|
||||
## Departures 1-7: rulings
|
||||
|
||||
1. Label 3m: accepted. Landing order decides, as with 264a. Gizmoduck's row 271 draft
|
||||
(gizmoduck-0264) also plans a new schema step, and its Q1 is with Mos (mos-1292). Whichever
|
||||
lands second takes the next label and moves the comment-migration future step.
|
||||
2. session.list stays owner-only: accepted (sage-2818 Q1, Sage RULINGS Q7). The follow-up binding
|
||||
is recorded in BUILD.md. Note that the user sharees' `access-refused` at test :313 comes from the
|
||||
USER_VERBS gate, since session.list isn't a user verb. The owner check is tested only through
|
||||
the human at :311. That's enough for this row.
|
||||
3. A reader matches as the business human: accepted (sage-2818 Q2). It departs from design 4.3's
|
||||
"reader sessions stay owner-only", read as "the reader sees what the owner sees". The 193d UI
|
||||
should label it that way.
|
||||
4. Access read per request: accepted (sage-2818 Q3). The brief's "30 s timer and access.reload
|
||||
paths each have a case" can't be met at this base because neither path exists
|
||||
(runtime.mjs:263). It isn't a missing test. It binds whichever row adds `access.reload`
|
||||
(193c per design 8: "call `access.reload`"), which must add the share-read case.
|
||||
5. Rule 3 at any level: accepted. It matches design 5.4 ("a live grant covering t") and is tested
|
||||
at :173-175.
|
||||
6. unshare checks no coverage: accepted. An owner must always be able to end a share. Tested at
|
||||
:214-216.
|
||||
7. `thread.shares` through `#grantedThread`: accepted. See C1.
|
||||
|
||||
## Conditions (the 275 rebase round)
|
||||
|
||||
- C1. Design 4.3's viewer column. Test that share, unshare and shares each refuse a viewer, and
|
||||
that a viewer's `thread.list {shared:true}` and `thread.messages` on a thread shared with it read
|
||||
(rule 2 at viewer level). 275 makes `#grantedThread` and `#granted` take the verb. At the
|
||||
rebase, the owner path in `#threadRead` (:771) must pass `thread.messages`, so a viewer owner
|
||||
doesn't read its own threads at collaborator level, and the share verbs must stay at
|
||||
collaborator.
|
||||
- C2. Rerun the drift and pin sweeps against the rebased base. 253 and 275 both touch broker.mjs,
|
||||
and 275 touches user-principal.test's USER_VERBS pin, which this row also edits.
|
||||
|
||||
## Notes (non-blocking)
|
||||
|
||||
- N1. `#readableThreads` binds `this.#owner(s) || null`. A reader's owner is false, so the query
|
||||
runs `owner IS NOT NULL` and can return the human's own threads shared with `*`. It does no harm
|
||||
today: a reader can't reach `thread.list` (`#human` refuses `read-only`), and replies hides only
|
||||
`user:` threads. If the reader ever gets a shared list, filter by `me`.
|
||||
- N2. replies calls `#readableThreads(s)` on every request. Each candidate thread runs `#readable`,
|
||||
which reads the access file up to twice through `#covered`, and that file is read from disk on
|
||||
every access read (runtime.mjs:263). Fine at current scale. If shares grow, compute the set only
|
||||
when the reply page holds a user-thread message, or read the access document once per request.
|
||||
- N3. `#shareTarget` accepts `user:<id>` for ids that aren't in the access file's `users`. Only
|
||||
share-not-covered stops a share to a stranger. That's fine because coverage implies a grant.
|
||||
- N4. BUILD.md "Runs" says none yet. The gate evidence, kill lines included, goes in BUILD.md
|
||||
before in-review.
|
||||
@@ -1505,3 +1505,4 @@ are never rewritten or removed; corrections are new entries.
|
||||
2026-10-22T16:46Z | Duckworth (Opus 5.5) | row 275 (#1780) early look, Morgana r2 4d2888471 (duckworth-0166) | B1 viewer.test:105-108 bob open/send answer thread-not-owned (red on base); mos-1311 C3 confirmed: both user-principal flips match design 4.3 :125/:139; N1-N3 |
|
||||
2026-10-22T16:47Z | Duckworth (Opus 5.5) | row 275 r3 957a9bfe0 check (duckworth-0167) | B1 cleared, N1 met, N2 left to 193a; no open early-look findings |
|
||||
2026-10-22T16:53Z | Duckworth (Opus 5.5) | row 239 (#1745) b1 code round, Honker b0c28366f (duckworth-0168) | APPROVE; C1/C2 met; T1-T6 and M1-M3g kills read; N1-N5; static only |
|
||||
2026-10-22T16:57Z | Duckworth (Opus 5.5) | row 206 (193b2, #1704) manifest review of 631a256ba (duckworth-0169) | APPROVE, C1-C2 bind the 275 rebase round, departures 1-7 accepted, N1-N4 |
|
||||
Reference in new issue
Block a user