Skip to content

fix(storage)!: a file its record lets go keeps that record's read rule - #26144

Merged
os-zhuang merged 2 commits into
mainfrom
claude/project-thread-jzy7m7
Oct 26, 2026
Merged

os-zhuang merged 2 commits into
mainfrom
claude/project-thread-jzy7m7

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #26046

Clause-②: no (narrowing)
Metadata-protocol: none

What this changes

ADR-0104 D3 promises one file, one owning record and one access rule. This makes the rule hold after the owning record lets the file go. Until a writer who may read that record claims the file again, or the platform collects it, the file is read by that record's rule.

The measurements and the exact path are with the server seat, not here.

Declarations

  • Clause-②: no (narrowing), measured on this diff: a file read that the owning record's rule refuses is now refused after the record lets the file go. Nothing refused becomes accepted. The claim wrote Clause-②: no and named this arm as expected.
  • Metadata-protocol: none: nothing a metadata author writes changes. The two fields are platform-managed bookkeeping on a platform object.
  • One patch changeset on @objectstack/service-storage, fix(storage)!:, with the BREAKING section and the ADR-0087 disposition not-required (no-migration-prescription).
  • Existing rows: schema sync adds the two columns, existing rows read NULL, and a NULL row is judged exactly as before. No migration is needed for the shape.

Tests

HEAD 7cc88889e.

  • New file-reference-release-read-rule.integration.test.ts: one real ObjectQL + sqlite engine with StorageServicePlugin booted as shipped, asked through the download door and the claim path, for every way a file leaves its slot, under both collection settings. On base 0a6948210 it is red (13 failed, 8 passed); on this branch 21/21 pass.
  • Ablation: the new verdict arm disabled through scripts/ablation-replace.mjs (anchor 1 to 0, blob 24540c7fe6b3 to 12afcb53c051) turns the same 13 red; restored, blob equals HEAD and git diff HEAD is empty.
  • pnpm --filter @objectstack/service-storage test: 66 files, 1218 tests passed. typecheck (including check:test-typecheck) passed. eslint on packages/services/service-storage with --no-inline-config: clean.
  • Full dogfood battery at HEAD: pnpm --filter @objectstack/dogfood test under the verify lock, VERDICT command-exit 0, Test Files 296 passed | 3 skipped (299), Tests 2523 passed | 18 skipped (2541).

Gates

Local subset at HEAD 7cc88889e: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 80 families from this diff; all 80 ran with exit 0 (each code captured before any pipe), and --ran reconciles 80 run, 0 NOT MEASURED. Before the full build, check:i18n and check:dual-build-cjs-loads answered exit 3 (prerequisite not met) and check-plugin-teardown-shape --self-test exit 3 (shallow clone); after pnpm build and fetching the self-test's control commit all three are 0, and check:i18n drove the translation-bundle commit. node scripts/check-platform-checklist.mjs --changed-from origin/main: OK, 4 anchors judged, 0 lost. Every other family is CI's; this PR's check-runs are the record.

Acceptance notes

  • Rows that predate this change. One residual about them is reported to the server seat with the measurements, for the maintainer to weigh. No follow-up card is filed by this PR.
  • Signed-URL lifetime. The download door still mints a released file's URL with the presigned TTL of an unowned file, not the shorter TTL of a record-governed one (authorizeDownload's gated test reads the ownership columns only). That decides how long an already-issued URL lives, not who gets one.
  • #26043 remains open; its path is not touched.

维护者速读(草稿)

改了什么:记录放手一个文件后,这个文件继续按原记录的读权限判定,直到有能读原记录的人重新认领它,或它被平台回收。sys_file 加了两个平台自管的空值字段记住"原记录";ADR-0104 的 2026-10-26 补记里改了一行。

为什么改:ADR-0104 承诺"一个文件、一个所属记录、一条访问规则"。实测发现这条规则在记录放手文件之后不成立(细节只在席位线程)。合同、证件、HR 附件这类私密记录里的文件,替换或删除后也要保持原来的访问范围。

风险与代价(含回滚):这是收窄:以前能读到的这类文件,现在不能读原记录的成员读不到了(上传者本人始终能读)。修复前已有的数据有一处遗留,详情在席位线程。回滚即 revert 本 PR;两个新列留在库里无害。

席位意见:

你要做的:审阅 ADR 那一行与修复,批准后合并(Tier H,保持 draft 等你)。


Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_018e5yJf1ptbMURVgWb3LrF6

ADR-0104 D3: one file, one owning record, one access rule. Until a writer
who may read it claims it again, or it is collected, a file its record let
go is read by that record's rule. sys_file gains two nullable bookkeeping
fields naming that record; release writes them, a claim clears them, and
the door's verdict (shared with the claim path) reads them.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018e5yJf1ptbMURVgWb3LrF6
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-storage, touching 8 documentable anchor(s).

7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/plugin-endpoints.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/automation/approvals.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/deployment/cli.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/kernel/contracts/storage-service.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/permissions/attachments-access.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/protocol/objectql/types.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/ui/translations.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))

⛔ 7 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/releases/index.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/releases/v15.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/releases/v16.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/releases/v17/17-0.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/releases/v17/17-5.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))
  • content/docs/releases/v17/index.mdx (via sys_file (symbol, a field of const object enObjects; a field of const object esESObjects; a field of const object jaJPObjects; a field of const object zhCNObjects))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json d1ea2363f6389a4cbd812843d670692283f52b37 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 29ad22f967de28b93b04e77c9021ecee66f96b66 — the merge of head 7cc88889e2294032fbb467107499c8f04d9a77da into base d1ea2363f6389a4cbd812843d670692283f52b37, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 29ad22f967de28b93b04e77c9021ecee66f96b66 && git checkout 29ad22f967de28b93b04e77c9021ecee66f96b66
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d1ea2363f6389a4cbd812843d670692283f52b37 7cc88889e2294032fbb467107499c8f04d9a77da && git checkout -B drift-repro d1ea2363f6389a4cbd812843d670692283f52b37 && git merge --no-ff 7cc88889e2294032fbb467107499c8f04d9a77da

node scripts/docs-audit/affected-docs.mjs --json d1ea2363f6389a4cbd812843d670692283f52b37

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs d1ea2363f6389a4cbd812843d670692283f52b37 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 26, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 7cc88889e2294032fbb467107499c8f04d9a77da
Local-runs: none
Scope: docs/adr/0104-field-runtime-value-shape-contract.md, packages/services/service-storage/src/file-reference-lifecycle.ts, packages/services/service-storage/src/storage-service-plugin.ts, packages/services/service-storage/src/metadata-store.ts, packages/services/service-storage/src/objects/system-file.object.ts, packages/services/service-storage/src/file-reference-release-read-rule.integration.test.ts, packages/services/service-storage/src/translations/en.objects.generated.ts, packages/services/service-storage/src/translations/es-ES.objects.generated.ts, packages/services/service-storage/src/translations/ja-JP.objects.generated.ts, packages/services/service-storage/src/translations/zh-CN.objects.generated.ts, .changeset/26046-released-file-keeps-record-rule.md + packages/services/service-storage/src/storage-routes.ts (the download door's authorizeDownload and its TTL choice), packages/services/service-storage/src/attachment-lifecycle.ts (the reap guard's column test), packages/services/service-storage/src/index.ts (the FileRecord export), packages/objectql/src/engine.ts (record file-field hydration and its closed column list), packages/drivers/driver-sql/src/schema-drift.ts (additive column sync), content/docs/permissions/attachments-access.mdx

Inputs: card #26046 (body, triage, both claims, the os-dev-report), PR #26144 (body, file list, the net diff of 0a6948210c..7cc88889e2 against main), and the check-runs on the head. Read-only throughout; the ablation and the base-red counts are the dev's own readings and are not re-run here.

① Derived judgments

  • The one accept-set change, at both doors. judgeFileRead gains one arm between the field-owned arm and the arm for a file no record has owned: a file whose owning record let it go, and that no record has claimed since, is judged by that record's read rule, through readableFieldOwners, the same function the field-owned arm asks (so a declared fileAccessDelegate decides here too). The download door (buildFileReadAuthorizer) and the record-write claim path (buildFileReferenceReadAuthorizer, PR fix(storage)!: a record write claims or copies only a file its caller may read #25951) both call judgeFileRead, so both follow it. Right, and a narrowing only: the uploader arm precedes it unchanged; the field-owned arm is unchanged; the arm for a file no record has owned (PR fix(storage)!: an unclaimed file is served only within its organization #25704's rule) is reached only when the two new columns are NULL, which is every file no record has let go and every row that predates this change; the attachments-scope arm and public_read (answered before the verdict at both doors) are unchanged; PR fix(storage)!: an anonymous write carries no file, at every anonymous door #26036's anonymous-write rule is in hunks this diff does not touch. No caller refused before is admitted now.
  • A record that no longer exists. The arm asks the former record by id under the caller's context; a row that is gone answers nothing, so the verdict is deny for everyone but the uploader, whose arm comes first. Where the former record's object declares a delegate, the delegate's answer governs, as the ADR line already says. Right, and the changeset's "until it is claimed again or collected" matches.
  • Clearing the memory. claimFile clears the two fields only on the path that writes the ownership columns, and that path runs only after findReferenceableFile has asked the write's caller through the same verdict, so the memory is cleared only by a writer who may read the former record. A refused writer's write keeps the id as written (the quiet answer of PR fix(storage)!: a record write claims or copies only a file its caller may read #25951), claims nothing, and leaves the row exactly as it was, its collection state included. A system or context-less write still claims without being asked, as PR fix(storage)!: a record write claims or copies only a file its caller may read #25951's "no caller, no question" rule already allows. Right. The ordering of the arm before the attachments-scope arm matches the ADR line's order and differs only for an attachments-scope file that a record's field had claimed and then let go, which is then judged by that record until claimed again or collected; consistent with the rule, named here so it is not a surprise.
  • The two sys_file fields. released_ref_object and released_ref_id are nullable text fields on a platform object: additive sync adds the columns (ALTER TABLE ADD COLUMN, schema-drift.ts), existing rows read NULL and take the arms they took before, and the verdict reads the row by file id (getFile / findOne with no field projection, so the new columns reach both doors). No index is needed. copyOwnedFile builds a copy's row field by field, so a copy never inherits them. The reap guard tests the ownership columns alone (attachment-lifecycle.ts), so the new fields never hold a file back from collection, as the PR says. Exposure: the fields are readable or writable only by a principal granted sys_file itself, exactly as ref_object / ref_id / ref_field are today; no permission set seeds a grant, and record hydration projects a closed column list that does not name them. That is the right exposure for platform-managed bookkeeping and should stay so; the labels and help text are abstract. Right.
  • Public type surface. FileRecord (exported from @objectstack/service-storage) gains two optional members; nothing is removed or narrowed. packages/spec/** is untouched, as the claim required. Right.
  • ADR-0104, 2026-10-26 addendum, one line in the list of who the verdict admits. Abstract, placed in the order the code asks the arms, and it matches judgeFileRead, claimFile and the collection path. It changes D3's text, so the PR is Tier H on docs/adr/**. Right.
  • Test. file-reference-release-read-rule.integration.test.ts boots the shipped plugin on a real engine and asks both doors through the shipped route and the shipped hooks, for each of the four ways a file leaves its slot, under both collection settings, with the controls the rule sits beside (a file no record has owned; the uploader; a reader of the record who takes the file back). The stand-ins are the ones the family's existing door tests use. Right.

② Semver level

  • One changeset, @objectstack/service-storage: patch, fix(storage)!:, with the BREAKING section (Before / Now / Fix) and the ADR-0087 disposition not-required (no-migration-prescription). The level follows the kind: Metadata-protocol: none (nothing a metadata author writes moves; the two fields are platform-managed), so patch; the narrowing keeps its banner. The disposition's facts hold on the diff: no spec key or authorable spelling moves, no export is removed or narrowed, no objectstack migrate meta rewrite exists. Right.
  • Clause-②: no (narrowing) is line-initial on the PR body and on the changeset, the same arm on both, and it is the arm the claim named as expected. The wording on the changeset, the ADR line and the PR body is abstract. Right.
  • The four translation bundles carry the two new labels in every locale, regenerated by the gate with hand-written translated values. Right.

③ Boundary flags

  • open_questions A/B (rows that predate this change). Escalated to the maintainer with this record; this is a Tier H PR and the question is theirs. The reviewer's reading: A. Such rows hold NULL in the new fields and are judged as before; the fact the fields would hold was never recorded for them, so no exact backfill exists, and a heuristic narrowing would also refuse uploads no record has ever claimed, the quiet-failure shape. Collection, where a deployment runs it, bounds the class in time. Not blocking.
  • Signed-URL lifetime (the PR's acceptance note, confirmed in the code). authorizeDownload decides the TTL from the ownership columns alone, so a URL minted for a file its record let go lives the unowned-file TTL (presignedTtl, default 3600 s) rather than the record-governed one (downloadTtl, default 300 s). Who receives a URL is this PR's rule and is right; how long an issued URL lives is a lane follow-up on the same door, and under the maintainer's standing word for this family it goes to the lane without a new card. Not blocking.
  • #26043 remains open and its path is untouched. Confirmed: the quiet-answer hunk is not in the diff.
  • A hand-written page states the door's arms. content/docs/permissions/attachments-access.mdx lists who the download door admits, by ownership; it predates PR fix(storage)!: an unclaimed file is served only within its organization #25704 and PR fix(storage)!: a record write claims or copies only a file its caller may read #25951 (its last change is docs(pm-dispatch, os-dev): the claim template and the dev definition carry the Metadata-protocol: line beside Clause-②: #25240's) and now also lacks this arm. Not governed; a docs-only follow-up or a line in this PR is the owning seat's call. Not blocking.
  • PR state the reviewer did not set, for the owning seat to read. At review time the PR is no longer draft, carries an APPROVED review on this head by an account in GOVERNED_APPROVERS, and auto-merge is armed by that account. The PR body still says it stays draft for the maintainer; the owning seat reads the actor before touching any of it.
  • Check-runs on the head at review time. Every concluded gate-carrying run is success (Lint & Repo Gates, Build Core, Governed Surface Queue Guard on its latest run, an earlier run of it cancelled and superseded, Test Core 1/8, 2/8, 6/8 and 8/8, Check Changeset, Dogfood Regression Gate, the lint, type-check source and debt-ledger legs). UI legs are skipped for a diff with no ui/ path. Those still in progress at the review's last read (Test Core 3/8, 4/8, 5/8, 7/8, Type Check · consumer gates, Type Check · workspace) have since concluded success: the head's runs read 36 success, 11 skipped and the one superseded cancelled run, with none failing.

Implemented-by: claude/project-thread-jzy7m7
Reviewed-by: session_0186vfjHHMsRXC9EUT9hTFMo

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · domain:server seat 2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-10-26T14:07Z · head 7cc88889e2294032fbb467107499c8f04d9a77da · #26046

The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」. This PR is Tier H (one ADR-0104 D3 addendum line): it carries an APPROVED review from os-zhuang (a governed approver) on this head, and auto-merge was enabled by that account.

  1. Shape: Fixes #26046. Clause-②: no (narrowing) and Metadata-protocol: none are line-initial in the body and the changeset. One patch changeset with a BREAKING banner and adr-0087: not-required (no-migration-prescription). The ADR-0104 D3 addendum line is abstract and matches the code's order.
  2. Contract review: record 6418045750, VERDICT PASS, at this head. It finds:
  3. Gates: at this stamp the head's check-runs read 36 success and 11 skipped (path-filtered), plus one superseded Governed Surface Queue Guard run that was cancelled and re-ran green; none failing or in progress. The PR is in the merge queue.
  4. Accepted deviations: none.
  5. Out of scope:
    • Rows released before this change carry nothing in the new fields and are judged as before. The dev's open question (A: no operator step; B: a heuristic backfill) is the maintainer's; the dev, the reviewer and this seat recommend A.
    • The download door gives such a file the unowned-file link lifetime rather than the record-governed one (who gets a link is right; how long it lives differs). Under 「所有的安全卡不要创建issue,直接派发处理修复」, this seat carries it as a follow-up with no new card.
    • The hand-written access page for attachments predates several recent rules and does not list this one; a docs-only follow-up.
  6. Landing: the maintainer's approval is on this head and auto-merge is on. This seat confirms MERGED on both readings and records the landing on files: an authorization gap in the file lifecycle after a file is released, within one organization (detail withheld) #26046, which the merge closes.

Generated by Claude Code

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

files: an authorization gap in the file lifecycle after a file is released, within one organization (detail withheld)

3 participants