Skip to content

tests(attachments): pin the unpinned authorization clauses of the attachments chain — coverage gaps from QA run #9401 #9483

Description

@os-zhuang

Blocked-by: #9719

QA-source: #9401 · attachments-storage.download-authz-both-sides · C3, C4
QA-source: #9401 · attachments-storage.read-inherits-parent-rls · C1, C3
QA-source: #9401 · attachments-storage.attach-requires-parent-edit · C3

The Tier-1 pinned sweep (#9401) found the attachments permission matrix green (15/16) but with authorization-relevant clauses unpinned. Security-priority per the maintainer's 2026-08-18 approval of the QA landing plan; the non-authz attachment gaps (presign envelope shapes, status pipeline, reap ordering, chunked-session round-trip) are parked on the wave anchor #9296, not filed here.

Unpinned authorization clauses to cover with real tests

  1. download-authz-both-sides C3 — acl=public_read re-opens the stable anonymous capability URL (the positive side of the anonymous gate); C4 may stay manual (browser denial copy) with the ref's scope note saying so.
  2. read-inherits-parent-rls C1 — COUNT is filtered identically to rows (a count leak is a real RLS leak); C3 — the filter fails CLOSED on compute failure and past the 2000-row pre-scan cap.
  3. attach-requires-parent-edit C3 — an unscoped multi-delete with neither id nor where is refused outright.

Also note from the run: the cross-tenant clauses are structurally unprovable in the OSS workspace (@objectstack/organizations is cloud-private) — the item ledger should carry that marker rather than each run re-deriving it.

Acceptance: tests land in the package owning the surface (likely packages/qa/dogfood's attachments matrix — triage to confirm lane); the items' automated.ref entries extended so the next Tier-1 run scores them honestly.


Status after PR #9724 (2026-08-18)

PR #9724 landed as Part of, not Fixes. Two of the three clause sets are pinned in full and all three items' automated.ref are re-scoped honestly.

clause set state
download-authz-both-sides C3 ✅ pinned by #9724
read-inherits-parent-rls C1 + C3 ✅ pinned by #9724
download-authz-both-sides C4 manual by construction (browser denial copy)
attach-requires-parent-edit C3 ⛔ blocked on #9719

Why clause 3 above is not pinned. It specifies that an unscoped multi-delete carrying neither an id nor a where is "refused outright (#4757)". Measured end-to-end, that refusal never fires: ObjectQL.delete's predicate path dispatches beforeDelete per row with input.id bound, so attachment-access-hooks.ts always takes its by-id branch and the "neither id nor where" arm is unreachable. A caller who uploaded every matched row deletes the whole table with a predicate-less delete.

Pinning the clause as written would fail; pinning current behaviour would turn the eventual fix red. So the behaviour is filed as #9719 and this clause waits on that decision.

What #9724 did land for this item is the half that holds regardless of #9719: an unscoped AST cannot slip past the per-row gate and deletes nothing — not even the row the caller was entitled to — where:{} behaves the same, a zero-match predicate is allowed through, and the same caller's scoped delete still succeeds.

Activity

  1. added theissue type on Aug 18, 2026
  2. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Triage: lane confirmed as the card requests — domain:cli (the qa package family anchors there, and the deliverable is the dogfood attachments matrix plus automated.ref extensions). If implementation finds a pin belongs in the surface-owning package's own suite (plugin-security / service-storage), that widens the declared file surface, not the seat. Same routing applied to the sibling pin cards #9481 and #9482 for the same reason. (Triage seat, session session_019gCKd9EZfQ6MbGTnMHHJvW.)


    Generated by Claude Code

  3. added
    domain:cliRetired 2026-10-22, alias of domain:server (cli surface); in-flight cards keep it until they land
    on Aug 18, 2026
  4. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 18, 2026
  5. self-assigned this
    on Aug 18, 2026
  6. os-zhuang commented on Aug 18, 2026

    @os-zhuang
    ContributorAuthor

    Claim: PM loop round 19
    Session: session_012WKSnqAaoqtW3QX7SSf1Vk
    Branch: claude/issue-9483-attachments-authz-clause-pins
    Worktree: objectstack-issue-9483
    Domain: domain:cli
    File surface, declared to REGION level: packages/qa/dogfood/test/** — the attachments matrix and new attachment-authz pins only, plus docs/qa/platform-checklist/areas/attachments-storage.json (automated.ref on the three named items). ⛔ Not authz-conformance.test.ts, ⛔ not the access-security or identity-auth areas — those belong to #9481 (landed) and #9482 (in flight).
    Container & model: M, mode:subagent, model: opus
    Clause-②: no — tests plus checklist prose. ⚠️ If pinning a clause needs a product change, stop and report; that flips the answer.

    Serial constraints — running alongside its sibling under the new exemption

    Maintainer, 2026-08-18, verbatim: 「放开同包,同文件仍硬串行」. So this card runs concurrently with #9482 in packages/qa/dogfood rather than after it. The two touch different test files and different checklist area JSONs (attachments-storage here, identity-auth there), so the region declarations are disjoint and same-file never arises.

    The exemption's four replacement rules are in the dispatch: ① region surface (above) · ② merge main before opening the PR · ③ merge again after #9482 lands · ④ conflicts to the merge queue, ⛔ not hand-ordered.

    Premise on origin/main: packages/qa/dogfood/test/ currently holds 125 files including #9481's two new pins landed at 8d982687e, so the attachments matrix is present and the sibling's work is visible to this card's author — useful, since #9481 is the worked example for the shape.

    Scope — security-priority, and the three clause sets are specified

    Per the maintainer's 2026-08-18 approval of the QA landing plan. Non-authz attachment gaps (presign envelope shapes, status pipeline, reap ordering, chunked-session round-trip) are ⛔ parked on wave anchor #9296 and out of scope.

    1. download-authz-both-sides C3 — acl=public_read re-opens the stable anonymous capability URL (the positive side of the anonymous gate). C4 may stay manual (browser denial copy) with the ref's scope note saying so.
    2. read-inherits-parent-rls C1 — COUNT filtered identically to rows (⭐ a count leak is a real RLS leak — a test that only checks rows misses it entirely); C3 — the filter fails CLOSED on compute failure and past the 2000-row pre-scan cap.
    3. attach-requires-parent-edit C3 — an unscoped multi-delete with neither id nor where is refused outright.

    ⚠️ Also from the run, and worth carrying into the ledger rather than re-deriving every wave: the cross-tenant clauses are structurally unprovable in the OSS workspace (@objectstack/organizations is cloud-private). The item ledger should carry that marker.

    ⭐ Same anti-vacuity bar as #9481, which is the accepted precedent: pin both sides of every authz clause (a deny-only suite stays green when everything denies), and show each new pin failing without the behaviour it pins — dogfood resolves through built artifacts, so use scripts/ablation-dist-preflight.mjs to prove an ablation is live in dist/ before its colour counts.


    Generated by Claude Code

  7. os-zhuang commented on Aug 18, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 9483,
      "status": "rework",
      "branch": "claude/issue-9483-attachments-authz-clause-pins",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/9724",
      "premise_still_valid": true,
      "summary": "Two of the three clause sets are pinned in full and all three items' automated.ref are re-scoped, so the next Tier-1 run scores them honestly: download-authz-both-sides C3 (acl=public_read reopens the anonymous capability URL — asserted on ONE file as 401 -> 302-with-real-bytes -> 401 again, with the TTL decoded out of the minted URL to prove the public_read branch ran rather than some other grant, plus the field-owned gating class), and read-inherits-parent-rls C1 (COUNT filtered identically to rows) and C3 (fail-closed past the 2000-row pre-scan cap). Status is rework, not done, because the third clause set cannot be delivered as specified: attach-requires-parent-edit C3 says an unscoped multi-delete is 'refused outright (#4757)', and measured end-to-end that refusal NEVER FIRES — the engine dispatches beforeDelete per row with input.id bound, so the handler always takes its by-id branch and a caller who uploaded every matched row deletes the whole table with a predicate-less delete (2 rows before, resolves, 0 after). That is a product change, which flips the dispatch's Clause-2 answer, so I stopped and filed #9719 rather than pinning current behaviour — pinning it would turn the eventual fix red. The PR is therefore 'Part of #9483', not a closing keyword; #9483 should stay open until #9719 is decided. What I did land for that item is the half that holds: an unscoped AST cannot slip past the per-row gate and deletes NOTHING (not even the row the caller was entitled to), where:{} behaves the same, a zero-match predicate is allowed through, and the same caller's scoped delete still succeeds.",
      "tests": "All on pushed head 7e99100df, after merging origin/main (which carries #9664). UNION (4 new pins + the existing attachments-permission-matrix): 'Test Files 5 passed (5)' / 'Tests 38 passed | 1 skipped (39)' — the single skip is the cross-tenant describe.skipIf block, by design. `pnpm --filter @objectstack/dogfood typecheck` -> 'tsc --noEmit', exit 0, script name echoed (not a zero-match filter). `check:type-check-debt --re-measure` -> 33 ledger entries re-measured, 1926 raw errors, 'surplus: none'. Green: check:platform-checklist (15 areas, 190 items), check:nul-bytes (6219 files), check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check:cross-package-test-inputs, check-affected-docs, and spec's check:empty-state / check:liveness / check:strictness-ledger / check:variant-docs. check:cross-package-test-inputs was PROVEN to reach these files, not assumed: three injections with untracked spellings passed (proving nothing), and the detector's own escaping-binding shape turned it red, green again on restore. ABLATIONS — every leg rebuilt into dist/ and proven live with scripts/ablation-dist-preflight.mjs before its colour counted, each reverted with --absent confirming no marker survives. A (public_read opt-out removed): predicted 4 red/4 green, ACTUAL 4/4, exactly the predicted four. B ('count' dropped from READ_OPS): predicted 5 red/2 green, ACTUAL 5/2, exactly those five — the count assertions go red while every row assertion stays green, which is the whole point of the clause. C (cap warn removed): predicted 1 red, ACTUAL 1 red/2 green. D (cap fails OPEN): predicted 2 red, FIRST RUN 1 red/2 green — PREDICTION MISSED, and the miss was a real vacuity in MY test: the dropped-rows contrast read the broad list with $top=200, so with the filter removed the page filled with unfiltered rows and 'absent from the page' meant 'paged out', not 'excluded by the cap' — it would have stayed green while the surface leaked the whole table. Fixed by asserting the page is not full before drawing the conclusion; on re-run leg D is 2 red/1 green as predicted, second red now a genuine detection. E (parent-edit delete gate always allows): predicted 2 red/3 green, ACTUAL 3 red/2 green — the third red is a CASCADE, not an independent detection (the ablated unscoped delete removes the rows, so the later scoped-delete test fails for want of its fixture). A matching or near-matching count is not a passing verification unless each red fails for the predicted reason; here two did and the third did not, and it is not scored as a detection.",
      "open_questions": [
        {
          "question": "#9719: the #4757 unscoped-multi-delete refusal on sys_attachment is unreachable through ObjectQL.delete, because the predicate path dispatches beforeDelete per row with input.id bound. How should it be resolved — and can #9483 close before it is?",
          "options": [
            "A — surface the predicate-less multi intent to per-row handlers (e.g. an explicit 'unscoped' marker on options the handler can read), so the existing guard fires again where it was written to",
            "B — move the refusal into ObjectQL.delete's multi branch as a general rule, protecting every guarded object rather than sys_attachment alone",
            "C — accept the per-row gate as sufficient, retire the guard AND the unit case that claims it, and amend the checklist clause to match"
          ],
          "recommendation": "B, then keep #9483 open until it lands. Real business need: the guard exists for blast radius, not authorization — every row deleted in the measured case was one the caller could have deleted individually, so this is not an escalation, but 'a predicate-less delete wipes the table' is a real operational hazard on a surface flows/actions/MCP tools reach. Long-term soundness: A re-plumbs one hook's context to restore one guard, which leaves the same trap armed for the next object that adds a delete guard — the defect is that a whole-operation rule was written in a per-row seam, and B fixes the seam. Making AI-written code hard to get wrong: B refuses the unscoped shape at the producer, loudly, for every guarded object, instead of relying on each hook author to notice that their operation-level check silently stopped running — exactly the declared-equals-enforced direction. C is the only option that needs no code, but it retires a safety property while leaving #4757's reasoning ('nothing was ever queried is not nothing to authorize') true and unenforced, so it should be taken only if the maintainer judges the blast-radius guard unwanted."
        }
      ],
      "out_of_scope_findings": [
        "filed as #9719: the #4757 unscoped multi-delete refusal on sys_attachment never fires through ObjectQL.delete — per-row beforeDelete dispatch binds input.id, so the handler's by-id branch always wins and an uploader-of-every-row deletes the whole table with a predicate-less delete; attachment-access-hooks.test.ts is green on a context shape the engine no longer produces"
      ]
    }

    Zone 2 assumptions, falsified or confirmed

    Two further things worth the ledger's memory, both now written into the refs rather than left for the next runner to rediscover:

    1. The $top trap. protocol.findData only calls engine.count() when the request carries a page limit; without one total = records.length, which is trivially self-consistent and stays green with count() fully unfiltered. A count-parity assertion written without $top proves nothing at all.
    2. The pre-scan has no ORDER BY, and the driver was measured not to return insertion order — a first draft of the scan-cap pin assumed it did, and its own control caught that. The seed now places visible rows at both ends of the insertion sequence so one group is outside the window under either ordering.

    Generated by Claude Code

  8. added
    pm:blockedBlocked by another issue/PR — body carries Blocked-by: #N
    and removed
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    on Aug 18, 2026
  9. os-zhuang commented on Aug 18, 2026

    @os-zhuang
    ContributorAuthor

    State change: pm:dispatched → pm:blocked, Blocked-by: #9719

    domain:cli seat, session session_012WKSnqAaoqtW3QX7SSf1Vk. Paired with the label write and body edit above — ⛔ a label change without its comment is a half-state. Labels read back: ['tests','domain:cli','pm:blocked'].

    Dev returned status: rework with PR #9724 (Part of, not Fixes; the Part-of PR must not also close its card gate is green). Two of three clause sets pinned in full; the third cannot be delivered as specified.

    The stop was correct, and this is the part worth keeping

    Clause attach-requires-parent-edit C3 says an unscoped multi-delete is "refused outright (#4757)". The dev measured that the refusal never fires, and stopped rather than pinning either version.

    That is exactly the right call, and the reason is worth stating: pinning the clause as written would fail, and pinning current behaviour would turn the eventual fix red. A test that encodes a defect as the contract is worse than no test — it converts the bug into a thing future work has to argue with.

    I verified the load-bearing half myself rather than taking the report:

    • packages/services/service-storage/src/attachment-access-hooks.ts:31-34 states the intended property verbatim in its own doc comment — "one carrying NEITHER an id NOR a where is refused outright (installAttachmentAccessHooks does not authorize an UNSCOPED multi-delete: no id + no where reads as "nothing to authorize" and deleteMany runs over the whole table #4757) — the engine would hand deleteMany an AST over the whole table, and a gate that resolved no rows for it would be authorizing exactly that."
    • The handler at :202 opens const ids = asIdList(ctx?.input?.id); if (ids) { … }. With input.id bound per row, that branch always wins and the refusal arm is unreachable.
    • ⭐ And attachment-access-hooks.test.ts is green on a context shape the engine no longer produces — the guard's own unit test passes because it constructs the world the code expects, which is why nothing caught this until someone drove it end-to-end.

    ⛔ #9719's A/B/C is not this seat's to rule. Option B moves the refusal into ObjectQL.delete's multi branch for every guarded object — a contract reject-behaviour change (Clause ②). Option C retires a declared safety property and amends the checklist to match. Both are on the human floor; the triage seat routes it and the maintainer decides.

    Why blocked rather than queued

    The remaining clause is not waiting on effort — no amount of test-authoring can pin it until #9719 says what the behaviour should be. Blocked-by: #9719 is line 1 of the body, so the unlock scan returns this card to the queue automatically when that lands. ⛔ No Restart-when: needed; this is a real upstream dependency, not a hold.

    Assignee retained deliberately, same reasoning as #9482: this lane keeps ownership so the card is re-dispatched here rather than re-triaged from scratch, and pm:blocked already removes it from selection. ⛔ The stale-claim criterion (>~24h, no branch, no PR) does not apply — this card has both.


    Generated by Claude Code

  10. added
    pm:queueReady for the PM dispatch loop
    and removed
    pm:blockedBlocked by another issue/PR — body carries Blocked-by: #N
    on Aug 19, 2026
  11. os-warren commented on Aug 19, 2026

    @os-warren
    Collaborator

    Unlock scan (triage seat, 2026-08-19 ~09:1xZ round): the remaining blocker #9719 (attach-requires-parent-edit clause C3) closed via merged PR #9797 → pm:blocked removed, queued. Claiming seat: re-verify which unpinned clauses from QA run #9401 are still unpinned on the merged ref — #9797's own tests may have absorbed some of the list.


    Generated by Claude Code

  12. removed their assignment
    on Aug 23, 2026
  13. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    清 assignee — #11196 维护者裁决 2026-08-23「A:本席清 agent 残留」,skills 席 session 757f0dc0-5dae-51d7-9053-c15c57ebfe3b。证据:原派发(cli 席 session 012WKSnqAaoqtW3QX7SSf1Vk)以 rework→pm:blocked 收束;08-19 解锁扫描回 pm:queue 只翻标签且明言「Claiming seat: re-verify…」——期待的是新认领,assignee 为旧周期残留。标签未动;新认领席重新 assign + 声明即可。


    Generated by Claude Code

  14. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 23, 2026
  15. self-assigned this
    on Aug 23, 2026
  16. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    Claim: PM loop round 31

    Assignee provenance — why this is free to claim

    Cleared at 02:35Z by the skills seat under the maintainer's #11196 ruling, with the disposition stated outright: 「新认领席重新 assign + 声明即可」. The old assignee was residue from the R19 cycle, not a live claim.

    ⚠️ Premise re-verified, and it has MOVED — this is the point of the dispatch

    Triage's unlock note (5340034435) instructed: "Claiming seat: re-verify which unpinned clauses from QA run #9401 are still unpinned on the merged ref — #9797's own tests may have absorbed some of the list." I ran that before claiming, and it matters:

    reading at origin/main value
    PR #9724 (Part of #9483) MERGED 2026-08-18T17:34:19Z
    packages/qa/dogfood/test/attachments-unscoped-delete-gate.dogfood.test.ts exists, describe('sys_attachment delete gate under an unscoped multi-delete (#9483)')
    its cases control (entitled/not split) · unscoped multi-delete deletes NOTHING · empty where: {} same · zero-match predicate allowed through · scoped delete still succeeds
    blocker #9719 closed via merged PR #9797

    ⇒ Two of the three clause sets were already landed by #9724, and the third item's partial pin — the "per-row gate holds" half — is on main too. What is not yet established is whether clause C3 as written ("an unscoped multi-delete … is refused outright (#4757)") is now reachable and pinned as a refusal, after #9797 changed the seam. The existing pin asserts "deletes nothing", which is a different property from "refused outright".

    ⭐ Note the checklist itself already cites "the attach-requires-parent-edit clause-3 lesson" elsewhere in attachments-storage.json — so the distinction has left a mark in the ledger and should be reconciled rather than re-derived.

    What the dispatch actually asks for

    The deliverable is the re-verification first, the pinning second. Establish on the merged ref which clauses from QA run #9401 remain genuinely unpinned; pin those; and re-scope every automated.ref this card owns so the next Tier-1 run scores them honestly.

    ⛔ Do not re-pin what #9724 already landed. ⛔ Do not pin current behaviour as the contract if it contradicts the clause — that is what turned the first attempt into a correct rework, and the reasoning stands: a test that encodes a defect as the contract makes the eventual fix look like a regression.

    premise_still_valid: false with no PR is an explicitly welcome outcome here. If #9724 plus #9797 between them closed everything this card names, say so with the evidence and the card closes — that is a real delivery, not a failure.

    Anti-vacuity bar, inherited and non-negotiable

    Pin both sides of every authz clause (a deny-only suite stays green when everything denies), and show each new pin failing without the behaviour it pins. Dogfood resolves through built artifacts, so use scripts/ablation-dist-preflight.mjs to prove an ablation is live in dist/ before its colour counts. ⭐ Two traps the previous run paid for, recorded so they are not re-bought: protocol.findData only calls engine.count() when the request carries a page limit, so a count-parity assertion written without $top proves nothing; and the pre-scan has no ORDER BY, so seed visible rows at both ends of the insertion sequence.


    Generated by Claude Code

  17. claude commented on Aug 23, 2026

    @claude
    Contributor

    os-dev-report

    {
      "issue": 9483,
      "status": "done",
      "branch": "claude/issue-9483-attachments-authz-remaining-clauses",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/11326",
      "premise_still_valid": true,
      "summary": "Re-verification first, as the unlock note required. Of the clause sets this card names, #9724 already pinned download-authz-both-sides C3 and read-inherits-parent-rls C1+C3 (refs accurate on main, not re-pinned here), and C4 is manual by construction. The blocked clause, attach-requires-parent-edit C3, is reachable again: #9719 was closed by PR #9797 and widened to the update verb by #9974 (the flag is now dispatchUnscopedMultiWrite), so the clause is pinnable AS WRITTEN and no defect had to be encoded as the contract. ONE SUB-PREMISE OF THE DISPATCH IS FALSIFIED: C3 was not merely 'reachable but unpinned' -- #9797's own re-pointed suite (attachment-access-hooks.test.ts:511-638, 'through the wired engine') already pinned it comprehensively, including the decisive uploader-of-every-row case, the empty-table limb and a control proving the per-row gate is a DIFFERENT refusal. What was genuinely open was different and worse: (1) the checklist still declared 'CLAUSE 3 IS NOT PINNED ... does NOT fire through ObjectQL.delete', i.e. it told the next Tier-1 runner that a shipped, working safety guard was broken; (2) the item's oracle for that clause was 'api' and its step 6 prescribed a REST probe the item's own knownGaps says the HTTP lane cannot express (#3897), so a runner would score a schema rejection as the refusal; (3) MEASURED, not assumed: the dogfood lane is blind to the clause -- its fixture splits entitled/not, so the per-row gate answers with the same code, and with the dispatch ablated the landed block stays 5/5 GREEN. Fixed all three: a second dogfood block seeds the only fixture that discriminates (caller entitled to EVERY matched row) plus the empty-table limb; the existing block now asserts status + the #4757 message head so it can tell the two refusals apart; and where:{} is re-documented as the per-row rule it actually is, asserting the per-row message so the #9797 boundary goes red rather than silently agreeing if the refusal ever widens. All clause sets this card names are now resolved, so the PR closes it.",
      "clause_table": [
        {
          "clause": "download-authz-both-sides C3 (acl=public_read reopens the anonymous capability URL)",
          "verdict": "pinned-already (#9724)",
          "evidence": "packages/qa/dogfood/test/attachments-public-read-acl.dogfood.test.ts on main; item automated.ref rev 3 describes it accurately. Not touched."
        },
        {
          "clause": "download-authz-both-sides C4 (browser denial copy)",
          "verdict": "manual-by-design",
          "evidence": "screenshot oracle over a console bundle this repo does not build; the ref's scope note already says so. Not touched."
        },
        {
          "clause": "read-inherits-parent-rls C1 (COUNT filtered identically to rows)",
          "verdict": "pinned-already (#9724)",
          "evidence": "attachments-parent-rls-count-parity.dogfood.test.ts; ref rev 3 accurate. Not touched."
        },
        {
          "clause": "read-inherits-parent-rls C3 (fail-closed past the 2000-row pre-scan cap)",
          "verdict": "pinned-already (#9724)",
          "evidence": "attachments-parent-rls-scan-cap.dogfood.test.ts; ref rev 3 accurate. Not touched."
        },
        {
          "clause": "attach-requires-parent-edit C3 (unscoped multi-delete refused OUTRIGHT, #4757)",
          "verdict": "newly-pinned end-to-end here; already pinned at the wired-unit level by #9797; ledger was STALE and is corrected",
          "evidence": "Behaviour: attachment-access-hooks.ts declares dispatchUnscopedMultiWrite on both sys_attachment write registrations (src lines 441 and 473). Wired-unit pin already on main: attachment-access-hooks.test.ts describe at :511. NEW end-to-end pin: second describe in attachments-unscoped-delete-gate.dogfood.test.ts. Blind-spot proof: ablating the declaration from both registrations (rebuilt, dist-preflight '--absent' -> 'marker absent from all 6 built files') leaves the pre-existing block 5/5 GREEN, while the new block fails by RESOLVING ('promise resolved \"2\"')."
        },
        {
          "clause": "cross-tenant clauses of this area",
          "verdict": "manual-by-design / blocked(dependency)",
          "evidence": "@objectstack/organizations is cloud-private; the MARKER already sits in the item's knownGaps from #9724. Not touched."
        }
      ],
      "gates": {
        "at_sha": "e3da4cda",
        "derivation": "node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, no paths (self-derived from merge base e278a2970, three-dot). --repo assertion held against this checkout's origin. 2 paths in the change set.",
        "green": [
          "check:doc-authoring",
          "check:doc-formula-expressions",
          "spec check:empty-state",
          "spec check:liveness",
          "spec check:strictness-ledger",
          "spec check:variant-docs",
          "check:published-files",
          "check:slot-lookup",
          "check:test-source-alias",
          "check:type-source-resolution",
          "check-ci-filter-parity",
          "check-plugin-teardown-shape",
          "check-affected-docs",
          "check:query-options-erasure",
          "check:type-check-coverage",
          "check:engine-double-contract",
          "check:cross-package-test-inputs",
          "check:where-matcher",
          "check:nul-bytes",
          "check:platform-checklist",
          "@objectstack/dogfood typecheck",
          "check:type-check-debt --re-measure",
          "eslint (narrowed, declared)"
        ],
        "verdict_lines": {
          "check:platform-checklist": "check-platform-checklist: OK - 15 areas, 205 items (205 active)",
          "check:nul-bytes": "check-nul-bytes: OK (scanned 6349 text file(s) ... no raw ASCII control bytes)",
          "check:engine-double-contract": "check-engine-double-contract: OK - 384 pinned, 133 in the DEBT ledger, 2 exempt",
          "check:where-matcher": "where-matcher conformance holds: 283 matcher(s) ... 0 silently-wrong ... none new",
          "check:cross-package-test-inputs": "OK: 14 package(s) read outside themselves, all declared",
          "check:test-source-alias": "check-test-source-alias OK - 72 packages with tests scanned",
          "check:type-check-debt": "check-type-check-coverage --re-measure: OK - 33 ledger entr(ies) re-measured in 233.9s, 1897 raw tsc error(s) total, none above its recorded number",
          "@objectstack/dogfood typecheck": "> @objectstack/dogfood@0.0.42 typecheck ... > tsc --noEmit (script name echoed - not a zero-match filter)"
        },
        "not_measured_then_measured": "check:type-check-debt --re-measure REFUSED on the first attempt (unbuilt closure) and was recorded as NOT MEASURED, never as passed. The closure was then built exactly as lint.yml does (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70 successful) and it re-ran green at the final head. No baseline moved anywhere.",
        "in_progress": [],
        "labels_read_back": [
          "size/m",
          "skip-changeset"
        ],
        "changeset": "none - @objectstack/dogfood is private:true and the diff is tests + checklist prose, so it releases nothing. skip-changeset applied via the additive POST endpoint (http=200) and READ BACK after the size-labeler had already run."
      },
      "tests": "UNION at final commit e3da4cda (sha read with git rev-parse in the same run): 'Test Files 1 passed (1)' / 'Tests 10 passed (10)' -- 5 pre-existing cases plus 5 new. Baseline before any edit was 5 passed (5). All build/test through scripts/pm/os-verify-lock.sh; conclusions read from its VERDICT lines, and every gate exit code captured by redirect-then-capture, never after a pipe. ABLATION -- one mutation, two measurements, both legs rebuilt and dist-preflighted. Mutation: remove 'dispatchUnscopedMultiWrite: true' from BOTH sys_attachment write registrations = the pre-#9797 world. On-disk confirmation was ANCHORED on the text being changed, not on an editor exit code: src count of that exact code spelling went 2 -> 0, and the bare identifier stayed at 5 because bundled COMMENTS survive -- which is why the marker had to be the code spelling, not the identifier (the identifier appears 4x in dist: 2 code, 2 comment). MEASUREMENT A (existing suite alone): 'Tests 5 passed (5)', ABLATED_VITEST_EXIT=0 -- the landed block is entirely BLIND to the clause it is named for. That negative result is the justification for the new block. MEASUREMENT B (extended suite): predicted 3 red / 7 green; ACTUAL 'Tests 3 failed | 7 passed (10)', and each red failed for its PREDICTED reason, checked individually rather than by count: (i) block 1 'refused OUTRIGHT' -> 'expected Error: Cannot delete attachment ... to match object', i.e. the per-row gate answered with the wrong message; (ii) block 2 uploader-of-every-row -> 'promise resolved \"2\" instead of rejecting', which is #9719's measured wipe reproduced verbatim; (iii) block 2 empty table -> 'promise resolved \"+0\" instead of rejecting'. NO CASCADE: the block-2 scoped sweep stayed green, so no red is being scored that merely lost its fixture. Preflight both directions: mutation leg 'marker absent from all 6 built files'; restore leg 'marker present in 2 built files (plus 2 sourcemap hits, not counted)'. Tree verified clean afterwards (git status empty, src count back to 2). Every ablation script carried a trap '<restore>' EXIT INT TERM.",
      "files_changed": [
        "packages/qa/dogfood/test/attachments-unscoped-delete-gate.dogfood.test.ts (+238/-30 region of the diff: stale header rewritten, 2 case titles/assertions corrected, 1 new describe block of 5 cases)",
        "docs/qa/platform-checklist/areas/attachments-storage.json (item attach-requires-parent-edit only: revision 3->4, knownGaps clause-3 entry, step 6, clause-4 oracle/verify/evidence, automated.ref, source line, history entry)"
      ],
      "deviations": [
        "Declared file surface honoured exactly: only packages/qa/dogfood/test/** (attachments authz) and the attachments-storage.json area. authz-conformance.test.ts, access-security and identity-auth untouched. No product code changed -- the Clause-2 'no' held, because #9797/#9974 had already made the product change the first attempt correctly refused to make.",
        "FIRST ABLATION ATTEMPT WAS VOID and is declared rather than quietly re-run: I planted an extra property on the registration options object; tsup's DTS pass type-checks that object and rejected it (TS2353), so the build failed -- BUT the ESM/CJS legs had already emitted, leaving dist/ mutated while src was restored by the trap. Caught it, rebuilt, and proved the stray marker gone ('marker absent from all 6 built files') BEFORE any measurement was taken. No reading from that attempt is reported.",
        "ESLint NARROWED to the diff, declared as a narrowing with all three evidences: (1) population -- eslint's own config governs, and it reports the .json file 'ignored because no matching configuration was supplied', so the effective population of this diff is the single .ts file; (2) count read from --format json: 2 files submitted, .ts linted with 0 errors / 0 warnings; (3) invariance -- eslint.config.mjs states no parserOptions.project and no typed @typescript-eslint rules, so this diff cannot move the verdict on any untouched file. Repo-wide 'pnpm lint' not run; CI owns that run.",
        "Report shape: the dispatch asked for machine-checkable fields only, while the standing os-dev contract requires summary/tests/open_questions. Both are included; clause_table added as the dispatch requested."
      ],
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  18. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    ✅ ACCEPT — PR #11326

    Reviewer of record: domain:cli seat, session session_019siH5jDmk5hrayvfyojUqR. Verified against the tree, ⛔ not the report.

    Checklist

    item verdict
    Scope exactly 2 files — the dogfood pin (+238/−30) and the attach-requires-parent-edit item in attachments-storage.json. Declared surface honoured ✅
    Fences authz-conformance.test.ts, access-security, identity-auth — all untouched ✅
    Product code unchanged — the Clause-② no held ✅
    ACCEPT path fork packages/qa/dogfood/** + docs/qa/** — docs/qa is not in the governed set ⇒ ordinary landing ✅
    skip-changeset applied and read back as ["size/m","skip-changeset"]; @objectstack/dogfood is private: true and the diff releases nothing ✅

    ⛔ A sub-premise of MY dispatch was falsified, and the truth was worse

    I wrote that clause C3 was "reachable but unpinned" after #9797. Wrong — #9797's own re-pointed suite (attachment-access-hooks.test.ts:511-638) had already pinned it comprehensively at the wired-unit level, including the decisive uploader-of-every-row case.

    What was actually open is three things I did not anticipate, and each is worse than an unpinned clause:

    1. The ledger was lying about a working guard. The checklist still declared "CLAUSE 3 IS NOT PINNED … does NOT fire through ObjectQL.delete". Verified: that string is present on origin/main and 0 on the branch. So the next Tier-1 runner would have been told a shipped, functioning safety guard was broken.
    2. The item's oracle would have manufactured a false pass. Its api oracle and step 6 prescribed a REST probe that the item's own knownGaps says the HTTP lane cannot express (deleteManyData 把请求体的 options 展开在 where 之后 —— body 可覆盖 id 谓词,把"删这几条"放大成"删调用者可见的全部" #3897) — a runner would have scored a schema rejection as the refusal.
    3. ⭐ The landed dogfood block was blind to the clause it is named for — and this is measured, not argued. With the guard ablated, the pre-existing block stays 5/5 GREEN. Its fixture splits entitled/not, so the per-row gate answers with the same code and the shape-based refusal never has to exist.

    That third finding is the substance of this card. A pin that cannot fail when the thing it guards is removed is not coverage; the new block's title says exactly what the old one couldn't distinguish — "refused on its SHAPE, not on entitlement".

    The ablation is the best I have reviewed this round

    • Measurement A (existing suite alone, ablated): Tests 5 passed (5). The negative result is the justification for the new work — that is the right way round, and most reports never produce it.
    • Measurement B (extended suite, ablated): predicted 3 red / 7 green; actual 3 failed | 7 passed, each red checked individually against its predicted reason rather than by count — wrong-message on block 1, promise resolved "2" (i.e. The #4757 unscoped multi-delete refusal on sys_attachment never fires through ObjectQL.delete — per-row hook dispatch bypasses it #9719's wipe reproduced verbatim) on the uploader-of-every-row case, resolved "+0" on the empty-table limb. No cascade: the scoped sweep stayed green, so no red is scored that merely lost its fixture.
    • ⭐ The marker had to be the code spelling, not the identifier, because bundled comments survive into dist (the identifier appears 4× there: 2 code, 2 comment). Anchoring on the identifier would have made the preflight lie in the safe-looking direction.

    ⭐ The void ablation attempt, declared rather than buried

    The first attempt planted a property that tsup's DTS pass rejected (TS2353). The build failed — but the ESM/CJS legs had already emitted, leaving dist/ mutated while the trap restored src. That is a phantom-result generator: a mutated artifact with clean source, and any measurement taken there would have been unattributable. It was caught, dist rebuilt, the stray marker proven gone ("absent from all 6 built files") before any reading was taken, and ⛔ no reading from that attempt is reported.

    A partially-succeeded build is a trap I had not seen named in this lane before. Worth carrying.

    Also correct

    check:type-check-debt --re-measure refused first (unbuilt closure), was recorded as not measured rather than passed, closure built exactly as lint.yml does, then re-run green. Third dev today to get that distinction right.

    Landing — ⛔ not enqueued yet

    Zero failures, but Lint & Repo Gates, TypeScript Type Check and all three Dogfood Regression Gate shards are still in_progress — and the Dogfood shards are the jobs that actually exercise this PR's new pins, so an aggregate "nothing red yet" is worth even less than usual here. Flip on the named jobs reaching completed: success, then arm and read back queue membership (allowing for the ~20s mount lag).


    Generated by Claude Code

  19. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    ✅ Landing audit — PR #11326 MERGED as 9ca16183

    domain:cli seat, session session_019siH5jDmk5hrayvfyojUqR.

    Two independent readings

    1. origin/main carries 9ca16183 tests(attachments): pin clause C3 end to end and correct the ledger that still calls it broken (#11326).
    2. Probe in both directions, plus four controls on what the card promised NOT to touch:
    probe want on origin/main
    removed: stale ledger claim CLAUSE 3 IS NOT PINNED 0 0 ✅
    added: refused on its SHAPE, not on entitlement block 1 1 ✅
    control — original delete gate under an unscoped multi-delete block 1 1 ✅ (added to, ⛔ not replaced)
    control — attachments-public-read-acl.dogfood.test.ts present present ✅
    control — attachments-parent-rls-count-parity.dogfood.test.ts present present ✅
    control — attachments-parent-rls-scan-cap.dogfood.test.ts present present ✅

    ⭐ The three file-presence controls are the ones that matter on a re-verification card. Its central claim was "these clauses are already pinned by #9724, so I am not touching them" — and the risk on such a card is re-pinning or disturbing work that already exists. All three survived untouched, and the original describe block was extended rather than rewritten.

    What landed, and what it was actually worth

    The card read as "pin a missing clause." What it delivered was three corrections that were each worse than a missing pin:

    1. A ledger that lied about a working guard — the checklist told the next Tier-1 runner that a shipped, functioning #4757 safety guard "does NOT fire through ObjectQL.delete". Now corrected.
    2. An oracle that would have manufactured a false pass — the clause's api oracle prescribed a REST probe the item's own knownGaps says the HTTP lane cannot express, so a schema rejection would have scored as the refusal.
    3. A pin that could not fail. Measured, not argued: with the guard ablated, the pre-existing block stayed 5/5 green. Its fixture split entitled/not, so the per-row gate answered with the same code and the shape-based refusal never had to exist. The new block seeds the only discriminating fixture — a caller entitled to every matched row — plus the empty-table limb, and the existing cases now assert the #4757 message head so the two refusals can be told apart.

    ⛔ Nothing was landed by weakening anything: no gate bypassed, no test skipped or quarantined, no product code touched, and ⛔ no defect encoded as a contract — the clause was pinnable as written because #9797/#9974 had already made the product change the first attempt correctly refused to make.

    Carried for the next runner

    ⭐ A partially-succeeded build is a phantom-result generator. The dev's first ablation attempt failed tsup's DTS pass (TS2353) after the ESM/CJS legs had emitted — leaving dist/ mutated while the trap restored src. It caught this, rebuilt, proved the stray marker gone before taking any reading, and reported no measurement from that attempt. A build that exits non-zero has not necessarily left the artifact clean.

    ⭐ Anchor an ablation marker on the code spelling, not the identifier — bundled comments survive into dist (the identifier appeared 4× there: 2 code, 2 comment), so an identifier-anchored preflight lies in the safe-looking direction.

    Card closed by the merge. All clause sets it names are resolved; the cross-tenant clauses remain blocked(dependency) with their marker already in knownGaps.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:cliRetired 2026-10-22, alias of domain:server (cli surface); in-flight cards keep it until they landpm:dispatchedDispatched to a dev agent by /pm-dispatchtests

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions