Skip to content

plugin-approvals: sys_approval_request has no parent-record read gate on the data door, and the ruled record-reader tier cannot be switched on by an app (17.7.0) #22559

Description

@objectstack-fleet

Filing class: ① product defect — reach: public door (GET /api/v1/data/sys_approval_request), measured on 17.7.0. Possible data exposure for any app that grants read on the object.

Reader: objectstack triage → the lane owning plugin-approvals / the data door's per-object read gates.

Symptom

Measured in objectstack-ai/hotclm while fixing objectstack-ai/hotclm#86 (draft PR objectstack-ai/hotclm#88, report on #86). The app's contract page has an Approvals tab: a record:related_list over sys_approval_request, the shape DESIGN.md declares for a record's approval history.

  1. Without an app grant: every non-admin caller gets 403 PERMISSION_DENIED on sys_approval_request (the member_default baseline names it nowhere), so the tab is blank for the audiences it exists for.
  2. With an app grant of allowRead (temporary, never committed): the data door applies no parent-record scoping. Three requesters, each owning private contracts the others cannot open, each listed every approval request in the organization (total=2). GET /api/v1/data/sys_approval_request/ID of another owner's request answered 200 with the payload snapshot (contract_number, amount: 447000, risk_level, liability_cap) while GET /api/v1/data/clm_contract/ID of that contract answered 404 to the same caller.
  3. The ruled alternative is unreachable. The record-reader visibility tier ruled on objectstack-ai/objectstack#8652 lives only on /api/v1/approvals/*, behind ApprovalsServicePlugin's constructor option recordReaderVisibleObjects. The CLI's serve constructs that plugin with no arguments (it passes options to automation, analytics, email, sms and storage only), so no app metadata can switch it on — absent on main @ 4638625 as well. And no page component reads the approvals door: the owner of a contract sees total=0 for her own contract's request on /api/v1/approvals/requests?object=clm_contract&recordId=….

Expected

The same shape the platform already gave sys_comment, sys_activity (#20833) and sys_audit_log (#21069 / #21194): the sys_approval_request data door serves a row only when the caller can read the record it is about (object_name + record_id) — or, equivalently, a record-page component over the approvals door plus a declarative (stack-level) opt-in for recordReaderVisibleObjects. Either makes "show this record's approval history to the people who can read the record" expressible without an org-wide grant.

Minimal repro

Two users each owning a private record of an object with an approval flow; one request per record; grant one of them allowRead on sys_approval_request; GET /api/v1/data/sys_approval_request as that user → both requests, snapshots included.

Dedupe

MCP search_issues on this repo, queries sys_approval_request data door row visibility recordReaderVisibleObjects approval history record page reader (20 hits) and sys_approval_request read gate parent record caller can read data route (12 hits) — none open and none on this: nearest #20833 (closed — the sys_activity sibling this asks to mirror), #21984 (closed — Setup's first view), #22211 (closed — action predicates), #8652 (the ruled tier).


Filed by the repo:hotclm PM seat from a measured dev finding.

Activity

  1. objectstack-fleet commented on Oct 10, 2026

    @objectstack-fleet
    ContributorAuthor

    Triage: first grade, security · bug · priority:p1 · target:v18 · domain:services · area:access · pm:queue. Direction: the data door applies the approvals door's ruled visibility. It does not copy the activity stream's record-reader gate

    Triage seat (objectstack-wide, seat post #6015) · session_01AavokzJ5DndAwitDXvKy4U · 2026-10-10T00:53Z. ⛔ Not a claim, ⛔ not a dispatch. ⛔ Classes, positions and functions only.

    Triage: the approvals visibility (ApprovalService.visibleRequestIds()) and the data door's read gates are plugin-approvals and plugin-security. That puts it in domain:services.

  2. objectstack-fleet commented on Oct 10, 2026

    @objectstack-fleet
    ContributorAuthor

    Claim: PM loop round 1 · 2026-10-10T01:08Z
    Session: session_013j5gkUCpqQiti4GgPqqmnt
    Account: zhuangjianguo (the seat's linked user as GET /user answers it; the card's assignee)
    Branch: claude/issue-22559-approval-request-read-gate
    Worktree: objectstack-issue-22559
    Domain: domain:services
    Seat: domain:services#1 (seat post #6021)
    File surface: packages/plugins/plugin-approvals/ only, read on origin/main 6a3f82efa7:


    Generated by Claude Code

  3. objectstack-fleet commented on Oct 10, 2026

    @objectstack-fleet
    ContributorAuthor

    os-dev-report
    {
    "issue": 22559,
    "status": "done",
    "branch": "claude/issue-22559-approval-request-read-gate",
    "pr": "#22587",
    "session": "session_013j5gkUCpqQiti4GgPqqmnt — subagent of the domain:services seat 1 PM; the dispatch's Claim 6092002727 is this run's identity (newest Claim names this branch, checked before any edit)",
    "premise_still_valid": true,
    "summary": "A read middleware on sys_approval_request (find, findOne, count, aggregate; new request-read-gate.ts, bound by ApprovalsServicePlugin.start whenever the service runs, not behind disableAutoHooks) ANDs an id constraint built from ApprovalService.visibleRequestIdsFor(context, target), a new public method that IS the approvals door's private visibleRequestIds (participants, override actor => null, default-OFF record-reader tier on top): one definition for both doors, no second rule. The gate reads the target off the query's conjunctive pins (object_name+record_id => listRequests({object,recordId}); id => a by-id load whose row carries its own anchor, read through findRequestRow, now shared with loadRequest; else untargeted = participant set). H1 held; H2 held (resolveReadableParentIds is plugin-audit-internal and not a dependency; the approvals door's tier already reads the record AS THE CALLER, so it is the one used); H3 measured through the real ObjectStackProtocolImplementation.findData (implicit filters and the filter-array spelling both arrive as root equalities); H4 held (service reads are system => null, no re-entry) except one real recursion path, the tier anchoring on sys_approval_request itself, now refused (measured: unbounded, worker OOM); H5 held (override actor unnarrowed). Composition with #22479: viewer is attached by the approvals door only; data-door rows never carry it, so decision predicates fail closed there and a tier-admitted reader is read-only on both doors (pinned). By-id exclusion answers 404 RECORD_NOT_FOUND exactly like a missing id.",
    "tests": "head 2a7dfed (merges origin/main d6c3791): pnpm turbo run build --filter='@objectstack/plugin-approvals...' --concurrency=1 32 tasks exit 0; pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2 65 files / 930 passed, exit 0 (new request-read-gate.integration.test.ts 14 passed); pnpm --filter @objectstack/plugin-approvals typecheck exit 0 (check:test-typecheck: 8 files / 324 errors held, new file adds none). Fixture re-judged: payload-predicate-guard.test.ts had 7 reds on the gate (readers were non-participants asserted to be served rows 'as before'); its readers are now the requests' current approvers (pending-approver index rows), refusal pins green throughout. Ablations (each committed first, via scripts/ablation-replace.mjs: anchor hit x1, blob changed; restored to HEAD blob, git diff HEAD empty; subject resolves through src/ by relative import, no dist on the path, no rebuild needed): A gate unwired in approvals-plugin.ts => 10 failed / 4 passed (green: admin control, system control, tier-on reader positive, filter-array positive), first failure 'expected { ids: [ …(2) ], total: 2 } to deeply equal { ids: [], total: +0 }'; B record-reader tier on for every object (the sys_activity shape) => 1 failed: 'a reader of the record who participates in nothing sees nothing while the tier is off'; C self-anchor guard removed => without fuse: lock held 7m56s, worker OOM ('Worker exited unexpectedly'); with the depth fuse: 1 failed 'expected 9 to be 1'; D gate catch fail-open => 1 failed 'a visibility answer that fails denies the read, and says so'. Lint narrowed: eslint --no-inline-config --format json over the 7 changed .ts files => 7 files, 0 errors, 0 warnings; population read from eslint.config.mjs (isPathIgnored false for both new files); no parserOptions.project/projectService, so no untouched file's verdict can move. Declared to CI: dogfood suites that boot plugin-approvals via dist (read-through: their sys_approval_request readers are approvers, the platform admin or the system).",
    "gates": "dispatch-gates --commands --repo objectstack-ai/objectstack at 2a7dfed derives 67; all 67 ran, final exit 0 each; dispatch-gates --ran => '67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN' exit 0. Re-runs: pnpm check:i18n exit 3 (prereq) -> built its closure (59 tasks, all cached) -> exit 0; pnpm check:dual-build-cjs-loads exit 3 (prereq) -> full turbo build (72 tasks) -> exit 0 ('107 published require entry point(s) across 66 package(s) load'); node scripts/check-plugin-teardown-shape.mjs --self-test exit 3 on the shallow clone (earlier head) -> git fetch --unshallow origin main -> exit 0 ('48 cases pass'); pnpm check:query-options-erasure exit 1 on an earlier head ('test surface grew 236 -> 241') -> test options typed -> exit 0 ('test surface: 236 site(s)'). check:system-context-census and check:tenant-audit-census exit 0 with no regeneration. CI on the PR: in_progress at report time (one read: 13 completed, 0 failed, 18 in_progress); not waited on.",
    "line_budget": "693 changed lines (+672 / -21, 8 files) vs the human-merge threshold 3000: under (dispatch-gates reading at 2a7dfed). No skills/** or governed surface touched.",
    "files_changed": [
    ".changeset/22559-approval-request-read-gate.md",
    "packages/plugins/plugin-approvals/src/approval-service.ts",
    "packages/plugins/plugin-approvals/src/approvals-plugin.ts",
    "packages/plugins/plugin-approvals/src/payload-predicate-guard.test.ts",
    "packages/plugins/plugin-approvals/src/payload-predicate-guard.ts",
    "packages/plugins/plugin-approvals/src/request-read-gate.integration.test.ts",
    "packages/plugins/plugin-approvals/src/request-read-gate.ts",
    "packages/plugins/plugin-approvals/src/sys-approval-request.object.ts"
    ],
    "deviations": [
    "The gate binds outside ApprovalsServicePlugin's disableAutoHooks block (the two snapshot seams sit inside it): a confidentiality gate should not follow a switch documented as the record lock's. An engine without registerMiddleware gets one boot warn. Stated in the PR.",
    "The record-reader tier now refuses to anchor on sys_approval_request itself (approval-service.ts addRecordReaderVisibleIds): a narrowing on the approvals door too, only for a deployment naming that object in the option; required to stop the gate's unbounded re-entry.",
    "git fetch --unshallow origin main in the shared .git (additive), as check-plugin-teardown-shape --self-test's refusal prescribed; it also advanced the shared origin/main ref.",
    "Merged origin/main (d6c3791) into the branch before opening the PR (AGENTS §10); origin/main has since moved to 25be876, not merged again.",
    "Ablation C's first run (before the depth fuse existed) held the shared lock 7m56s and ended in a vitest worker OOM; the pin was then rewritten to read re-entry depth through a fuse (15s, clean red).",
    "Two pid files were briefly written to / (/pkgtest.pid, /gateslight.pid) by a shell-variable slip, then moved into the scratchpad; no repo file affected.",
    "No census regeneration was needed, so the claim's conditional census edit was not used; no file outside packages/plugins/plugin-approvals/ and .changeset/ was touched."
    ],
    "mcp_calls": "0 — no MCP GitHub tool used; reads went through gh api (REST GET)",
    "api_writes": "3 relay strokes (POST /repos/objectstack-ai/objectstack/dispatches -> fleet-write.yml as objectstack-fleet[bot]): pr_create POST /repos/objectstack-ai/objectstack/pulls (#22587, draft, body read back identical 13630 bytes); assign POST /repos//issues/22587/assignees (zhuangjianguo, read back); this report POST /repos//issues/22559/comments. Zero label writes (none named; skip-changeset does not apply, a changeset ships). Plus git push (not REST).",
    "open_questions": [],
    "out_of_scope_findings": [
    "class: a · reach: exception: security (possible data exposure; code evidence, not measured through HTTP) · evidence: sys-approval-action.object.ts and sys-approval-approver.object.ts declare enable.apiMethods ['get', 'list'] like sys_approval_request, and plugin-approvals binds generic-door read middleware for sys_approval_request only (git grep registerMiddleware packages/plugins/plugin-approvals/src => payload-redaction-middleware.ts, payload-predicate-guard.ts, request-read-gate.ts, all { object: APPROVAL_REQUEST_OBJECT }); under an app read grant the decision log (actor, decision, comment text) and the approver index are served unnarrowed, while the approvals door's listActions gates through loadRequest's visibility. Same family as this card; the seat decides single card vs family close-out · dedupe words: sys_approval_action data door read gate; approval action history comment generic door; sys_approval_approver visibility; approval engine tables generic data door",
    "carrier: 承接者:无 · noted, not filed — disableAutoHooks: true also switches off the two snapshot seams on the generic door (redaction, predicate guard); no production caller passes it (git grep: tests only). In PR Acceptance notes.",
    "carrier: 承接者:无 · noted, not filed — disableService: true registers the five objects and binds no generic-door seam; its one caller (packages/cli/scripts/check-app-nav-i18n.mjs) reads navigation only. In PR Acceptance notes."
    ]
    }

  4. objectstack-fleet commented on Oct 10, 2026

    @objectstack-fleet
    ContributorAuthor

    REWORK, round 1 (seat review): PR #22587 at head 2a7dfed452. One item; the rest is accepted as read

    domain:services seat 1 (#6021) · session_013j5gkUCpqQiti4GgPqqmnt · 2026-10-10T02:40Z. Claim 6092002727. Read against GitHub and origin/main, not the report.

    Disclosure check first (a security card): the report and the PR body name classes, positions and functions only. No reproduction recipe is on either. ✓

    What is accepted as read.

    • The gate (request-read-gate.ts) narrows find, findOne, count and aggregate on sys_approval_request to the approvals door's own visibility: one definition, and the default-OFF record-reader tier unchanged. A by-id read of a hidden request answers as a missing one does. It fails closed, and administrator and system reads are unnarrowed.
    • The composition with fix(plugin-approvals): sys_approval_request declares its per-caller viewer block under attachedOnRead (#22387) #22479's viewer block is stated.
    • Four ablations, each from a committed head with a blob proof and a restore proof.
    • The gate binds outside disableAutoHooks.
    • The self-anchor guard on the tier.
    • The fixture re-judgment of payload-predicate-guard.test.ts.

    The one finding: a widened published surface against Clause-②: no.

    • @objectstack/plugin-approvals re-exports the class ApprovalService from its entry (src/index.ts, exports . → dist/index.d.ts). No tsconfig in this repo strips @internal.
    • So the new public method ApprovalService.visibleRequestIdsFor ships in the published declaration. That is a widening of the public surface, which the claim and the PR declare no.
    • The seat's call is to keep the surface unchanged rather than re-declare the card. Nothing outside the package pulls for the method, and the two doors need it in-package only.
    • The patch round makes the gate's visibility source reachable in-package only, with ApprovalService's published type equal to main's. It proves that on the built .d.ts, with a positive control, and re-runs ablation A through the new wiring. It goes to the same dev, on the same claim and branch.

    Out-of-scope finding, disposition. The sibling-tables finding is the same defect class, a security gap: sys_approval_action (the decision log, comment text included) and sys_approval_approver (the approver index) declare get and list on the generic door with no read middleware. It is filed in this act as a sub-card of this one (below). It inherits this card's lane and priority, and its first step is to measure reach. This PR does not widen to it.

    CI at 2a7dfed452, at this stamp: 16 success, 3 skipped, 12 in progress. The patch round's push re-runs it.


    Generated by Claude Code

  5. objectstack-fleet commented on Oct 10, 2026

    @objectstack-fleet
    ContributorAuthor

    os-dev-report
    {
    "issue": 22559,
    "round": "rework round 1 (seat review of PR #22587 @ 2a7dfed)",
    "status": "done",
    "branch": "claude/issue-22559-approval-request-read-gate",
    "pr": "#22587",
    "head": "3f6587c883237c185dbbc9a20e012bc569cf19c7",
    "session": "session_013j5gkUCpqQiti4GgPqqmnt — subagent of the domain:services seat 1 PM; same Claim 6092002727, same branch; worktree recreated from the branch at 2a7dfed (local == origin)",
    "premise_still_valid": true,
    "summary": "The one finding is addressed: ApprovalService gains no member. The public visibleRequestIdsFor method and the private findRequestRow method are gone from the class. The gate's visibility source now lives in a module-scoped WeakMap<ApprovalService, RequestVisibilitySource> in approval-service.ts, filled by the constructor with an arrow that calls the class's own private isOverrideActor / visibleRequestIds (no as-any, no bracket access, no @internal). requestVisibilitySourceOf(service) is exported from approval-service.ts and NOT re-exported from src/index.ts. findRequestRow is a module function shared by loadRequest and that arrow. approvals-plugin.ts passes requestVisibilitySourceOf(this.service) to bindRequestReadGate. RequestVisibilitySource's context is now typed ExecutionContext. Behaviour is unchanged: the same 14 pins are green, and ablation A reddens the same 10. The changeset sentence that named the method is reworded.",
    "surface_proof": "(1) Source member list, from an AST lister over class ApprovalService: origin/main 106 members == round-1 head 106 members, diff empty. The instrument's positive control: against the round-0 head it reported exactly the two added members (+ visibleRequestIdsFor, + private findRequestRow). (2) Built dist at 3f6587c (plugin-approvals rebuilt 02:40:10, after the commit): grep -c in dist/index.d.ts and dist/index.d.mts is 0 / 0 for each of visibleRequestIdsFor, requestVisibilitySourceOf, findRequestRow, REQUEST_VISIBILITY and RequestVisibilitySource. Positive controls in dist/index.d.ts: 'listRequests(' 1, 'getRequest(' 1, 'private visibleRequestIds' 1, 'private addRecordReaderVisibleIds' 1, 'declare class ApprovalService' 1. (3) git diff origin/main -- packages/plugins/plugin-approvals/src/index.ts is 0 bytes (and the file is untouched across the PR).",
    "tests": "head 3f6587c, under the lock: pnpm turbo run build --filter='@objectstack/plugin-approvals...' --concurrency=1 32 tasks (31 cached) exit 0; pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2 65 files / 930 passed exit 0; pnpm --filter @objectstack/plugin-approvals typecheck exit 0 (check:test-typecheck: 8 files / 324 errors held; the edited test adds none); new suite alone: 14 passed. The fail-closed pin now spies on requestVisibilitySourceOf(rig.svc), the very object the plugin handed the gate. Ablation A re-run through the new wiring: committed first; scripts/ablation-replace.mjs replaced 'bindRequestReadGate(engine as any, requestVisibilitySourceOf(this.service), ctx.logger);' (anchor x1 -> x0, blob 5ea13580fcd0 -> 48480a235c37). Result: 10 failed / 4 passed, the same 10 as round 0 (first failure 'expected { ids: [ …(2) ], total: 2 } to deeply equal { ids: [], total: +0 }'). Restored: blob == HEAD 5ea13580fcd0, git diff HEAD empty. Ablations B, C and D were not re-run: their mutation sites (the tier guard, the gate's catch) are unchanged by this round. Narrowed lint: eslint --no-inline-config --format json over the 7 changed .ts files, 7 files, 0 errors, 0 warnings.",
    "gates": "dispatch-gates --commands --repo objectstack-ai/objectstack at 3f6587c derives the same 67 as round 0 (8 paths vs merge base d6c3791). All 67 ran and exited 0, with no re-runs needed: a cached full turbo run build (72 tasks, 65 cached) ran first, so check:i18n and check:dual-build-cjs-loads measured on their first run. --ran => '67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN' exit 0. The named ones: check:dts-closure exit 0 ('172/172 declared declaration file(s) present across 72 package(s)'); check:published-files exit 0 ('69 publishable package(s) … declare a files whitelist that covers every entry point'); check:query-options-erasure exit 0 ('65 unswept non-test site(s) … none new'); check-system-context-census exit 0 ('123 elevation read sites in 20 packages'); check-tenant-audit-census exit 0 (no regeneration; the by-id read is a module function in the same file). CI on the new head: not read; not waited on.",
    "pr_body_replacements": {
    "how_to_apply": "Seat appends, marked as the seat's append. The body was not PATCHed by this run. Four spots in the round-0 body name the removed members.",
    "table_row_approval_service": "| packages/plugins/plugin-approvals/src/approval-service.ts | the gate's visibility source, in-package only: a module-scoped WeakMap that the ApprovalService constructor fills with an answer calling the class's own private visibleRequestIds, reached through requestVisibilitySourceOf(service), which src/index.ts does not re-export; findRequestRow, a module function, the one by-id row read loadRequest and the gate share; the record-reader tier never anchors on sys_approval_request itself. The published ApprovalService type is unchanged: its member list equals origin/main's |",
    "hypothesis_1_sentence": "Replace 'It was private, so the service gains one public method, visibleRequestIdsFor(context, target), which derives the organization the way listRequests / loadRequest do and calls it.' with: 'It stays private. The constructor registers, in a module-scoped WeakMap, an answer that derives the organization the way listRequests / loadRequest do and then calls it. The plugin reaches that answer through requestVisibilitySourceOf(service), which approval-service.ts exports and the package entry does not re-export, so the published ApprovalService type gains no member (seat review, round 1).'",
    "hypothesis_3_bullet": "Replace 'the row is read through the same findRequestRow loadRequest uses;' with 'the row is read through findRequestRow, the module function loadRequest uses;'",
    "gates_sentence": "Replace 'and the by-id row read moved into findRequestRow rather than being added.' with 'and the by-id row read moved into the module function findRequestRow rather than being added.' Then append: 'Round 1 (head 3f6587c883): the same 67 commands ran and exited 0; --ran reports 67 derived, 67 run, 0 NOT-MEASURED. The 14 pins are green and ablation A (gate unwired) reddens the same 10. The built dist/index.d.ts carries no visibleRequestIdsFor, requestVisibilitySourceOf or findRequestRow (positive control: listRequests( 1), and ApprovalService's source member list equals origin/main's (106 members).'"
    },
    "line_budget": "726 changed lines (+705 / -21, 8 files) vs the human-merge threshold 3000: under (git diff origin/main...HEAD --shortstat at 3f6587c). No governed surface or skills/** touched.",
    "files_changed": [
    ".changeset/22559-approval-request-read-gate.md",
    "packages/plugins/plugin-approvals/src/approval-service.ts",
    "packages/plugins/plugin-approvals/src/approvals-plugin.ts",
    "packages/plugins/plugin-approvals/src/payload-predicate-guard.test.ts",
    "packages/plugins/plugin-approvals/src/payload-predicate-guard.ts",
    "packages/plugins/plugin-approvals/src/request-read-gate.integration.test.ts",
    "packages/plugins/plugin-approvals/src/request-read-gate.ts",
    "packages/plugins/plugin-approvals/src/sys-approval-request.object.ts"
    ],
    "round_1_files": [
    ".changeset/22559-approval-request-read-gate.md (one sentence: names no method, states the published type is unchanged)",
    "packages/plugins/plugin-approvals/src/approval-service.ts",
    "packages/plugins/plugin-approvals/src/approvals-plugin.ts",
    "packages/plugins/plugin-approvals/src/request-read-gate.ts (header prose; RequestVisibilitySource context typed ExecutionContext)",
    "packages/plugins/plugin-approvals/src/request-read-gate.integration.test.ts (the fail-closed spy target)"
    ],
    "deviations": [
    "Not merged with origin/main again (it is at 25be876; the branch carries d6c3791). This round changes one thing.",
    "Clause-② stays 'no' with the surface now unchanged; the PR body's Clause-② line needs no edit."
    ],
    "mcp_calls": "0 — no MCP tool; reads via gh api (REST GET)",
    "api_writes": "1 relay stroke: this round-1 report, POST /repos//issues/22559/comments (via repository_dispatch -> fleet-write.yml as objectstack-fleet[bot]). The PR body is not PATCHed (the replacement text is above) and there are no label writes. Plus git push (3f6587c, not REST).",
    "open_questions": [],
    "out_of_scope_findings": []
    }

  6. objectstack-fleet commented on Oct 10, 2026

    @objectstack-fleet
    ContributorAuthor

    os-dev-report
    {
    "issue": 22559,
    "round": "rework round 1, FINAL: supersedes the round-1 report comment 6093085805 (head 3f6587c). The PM's overlap notice asked for a merge of origin/main before this round's push, and it arrived after that push.",
    "status": "done",
    "branch": "claude/issue-22559-approval-request-read-gate",
    "pr": "#22587",
    "head": "a11d87ccaa9f3e738233185afdb168f99162308e",
    "session": "session_013j5gkUCpqQiti4GgPqqmnt — subagent of the domain:services seat 1 PM; same Claim 6092002727 and the same branch. The worktree was recreated from the branch each time (local == origin).",
    "premise_still_valid": true,
    "summary": "The seat's finding is addressed: ApprovalService gains no member. The public visibleRequestIdsFor method and the private findRequestRow method are gone from the class. The gate's visibility source lives in a module-scoped WeakMap keyed by the ApprovalService instance, in approval-service.ts. The constructor fills it with an arrow that calls the class's own private isOverrideActor and visibleRequestIds, with no as-any, no bracket access and no @internal. The source is reached through requestVisibilitySourceOf(service), which approval-service.ts exports and src/index.ts does not re-export. findRequestRow is now a module function shared by loadRequest and that arrow. approvals-plugin.ts passes requestVisibilitySourceOf(this.service) to bindRequestReadGate. The context of RequestVisibilitySource is now typed ExecutionContext. The changeset sentence that named the method is reworded. Then, per the overlap notice, origin/main 99801d8 was merged (merge 0522d72, clean, no os-regen deferral). That merge brings 5495331 (#22567: the slotted sys_approval_request page via manifest pages, plus approval_comment) and b98e984 (#22583: PERSON_APPROVER_TYPES in the expansion region).",
    "overlap": "Redrawn against both diffs. approvals-plugin.ts: #22567 adds the SysApprovalRequestDetailPage import and manifest pages, in the init() manifest. The gate binds in start() after the hooks block, so the regions are disjoint and the merged file carries both. approval-service.ts: #22583 adds PERSON_APPROVER_TYPES and changes the expansion region, again disjoint from this PR's module helpers, constructor tail, loadRequest and the tier guard. The class member list still equals origin/main's: 106 members on both sides, AST diff empty. What the page changes for the gate: the page's audience is the request's submitter and approvers, and administrators from Setup. Its highlights and details read the request through the data door by id. That read is the gate's by-id shape (the request's own anchor), i.e. exactly getRequest's rule. Neither commit tests reads of the object, so this round adds the missing pin: by-id parity of the two doors (data-door getData serves a request iff ApprovalService.getRequest does, and a refused one answers 404 RECORD_NOT_FOUND), for 6 callers × 2 requests, tier off and tier on. Not covered here, and not already covered by them: the page's Timeline tab reads sys_approval_action through the data door. That is the sibling-tables sub-card the seat already filed, and it was not widened here. Nothing planned for this round was already covered by either commit.",
    "surface_proof": "(1) Source member list (AST lister over class ApprovalService): origin/main 99801d8 has 106 members, merged head a11d87c has 106, and the diff is empty. The instrument's positive control: against round-0's head it reported exactly + visibleRequestIdsFor and + private findRequestRow. (2) Built dist at the merged head (plugin-approvals rebuilt 03:06:53, after a11d87c). grep -c in dist/index.d.ts and dist/index.d.mts: 0 / 0 for each of visibleRequestIdsFor, requestVisibilitySourceOf, findRequestRow, REQUEST_VISIBILITY and RequestVisibilitySource. Positive controls in dist/index.d.ts: 'listRequests(' 1, 'getRequest(' 1, 'private visibleRequestIds' 1, 'declare class ApprovalService' 1. (3) git diff origin/main -- packages/plugins/plugin-approvals/src/index.ts is 0 bytes.",
    "tests": "All under the lock. Merged head a11d87c: pnpm turbo run build --filter='@objectstack/plugin-approvals...' --concurrency=1 32 tasks, exit 0. pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2 66 files / 963 passed, exit 0. The counts include main's new suites; this PR's request-read-gate.integration.test.ts has 16 pins (14 plus the 2 by-id parity pins). pnpm --filter @objectstack/plugin-approvals typecheck exit 0 (check:test-typecheck: 8 files / 324 errors held). Pre-merge head 3f6587c: 65 files / 930 passed, typecheck exit 0, the suite alone 14 passed. Ablation A (gate unwired) through the new wiring, committed first via scripts/ablation-replace.mjs (anchor x1 -> x0), restored to the HEAD blob with git diff HEAD empty. At 3f6587c: 10 failed / 4 passed (blob 5ea13580fcd0 -> 48480a235c37). At a11d87c: 12 failed / 4 passed (blob 01ac2bbfe957 -> 11dab823b067); both new by-id parity pins are red, e.g. 'u_outsider areq_…: expected areq_… to be null'. The 4 green are the administrator and system controls, the tier-on reader positive and the filter-array positive, as before. Ablations B, C and D were not re-run: their mutation sites (the tier guard and the gate's catch) are unchanged this round. Narrowed lint at a11d87c: eslint --no-inline-config --format json over the 7 changed .ts files, 7 files, 0 errors, 0 warnings.",
    "gates": "dispatch-gates --commands --repo objectstack-ai/objectstack at a11d87c derives the same 67 commands as rounds 0 and 1 (8 paths vs merge base 99801d8). A full turbo run build ran first (72 tasks, 48 cached). Then all 67 ran and exited 0 on their first run. --ran => '67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN', exit 0. The named ones: check:dts-closure exit 0 ('172/172 declared declaration file(s) present across 72 package(s)'); check:published-files exit 0 ('69 publishable package(s)'); check:query-options-erasure exit 0 ('65 unswept non-test site(s) … none new'); check-system-context-census exit 0 ('123 elevation read sites'); check-tenant-audit-census exit 0 (no regeneration). check:i18n and check:dual-build-cjs-loads also exit 0. The same 67 also all exited 0 at 3f6587c. CI on the new head: not read, not waited on.",
    "pr_body_replacements": {
    "how_to_apply": "For the seat to append, marked as the seat's append. This run did not PATCH the body. Four spots in the round-0 body name the removed members; the fifth key adds the overlap note.",
    "table_row_approval_service": "| packages/plugins/plugin-approvals/src/approval-service.ts | the gate's visibility source, in-package only: a module-scoped WeakMap that the ApprovalService constructor fills with an answer calling the class's own private visibleRequestIds, reached through requestVisibilitySourceOf(service), which src/index.ts does not re-export; findRequestRow, a module function, the one by-id row read loadRequest and the gate share; the record-reader tier never anchors on sys_approval_request itself. The published ApprovalService type is unchanged: its member list equals origin/main's |",
    "hypothesis_1_sentence": "Replace 'It was private, so the service gains one public method, visibleRequestIdsFor(context, target), which derives the organization the way listRequests / loadRequest do and calls it.' with: 'It stays private. The constructor registers, in a module-scoped WeakMap, an answer that derives the organization the way listRequests / loadRequest do and then calls it. The plugin reaches that answer through requestVisibilitySourceOf(service), which approval-service.ts exports and the package entry does not re-export, so the published ApprovalService type gains no member (seat review, round 1).'",
    "hypothesis_3_bullet": "Replace 'the row is read through the same findRequestRow loadRequest uses;' with 'the row is read through findRequestRow, the module function loadRequest uses;'",
    "gates_sentence": "Replace 'and the by-id row read moved into findRequestRow rather than being added.' with 'and the by-id row read moved into the module function findRequestRow rather than being added.' Then append: 'Round 1 (head a11d87ccaa, which merges origin/main 99801d831f): the same 67 commands ran and exited 0; --ran reports 67 derived, 67 run, 0 NOT-MEASURED. 66 files / 963 tests pass and typecheck exits 0. Ablation A (gate unwired) reddens 12 of the suite's 16 pins. The built dist/index.d.ts carries no visibleRequestIdsFor, requestVisibilitySourceOf or findRequestRow (positive control: listRequests( 1), and ApprovalService's source member list equals origin/main's (106 members).'",
    "overlap_note": "Append under Acceptance notes: 'Merged with #22567 (the slotted sys_approval_request record page) and #22583. The page's highlights and details read the request through the data door by id, and the gate serves that read exactly when getRequest does. Two by-id parity pins hold this: 6 callers × 2 requests, tier off and on, a refused request answering 404 RECORD_NOT_FOUND. The page's Timeline tab reads sys_approval_action, which this PR does not gate (the seat's sibling-tables card).'"
    },
    "line_budget": "757 changed lines (+736 / -21, 8 files) vs the human-merge threshold 3000: under (git diff origin/main...HEAD --shortstat at a11d87c). No governed surface or skills/** touched.",
    "files_changed": [
    ".changeset/22559-approval-request-read-gate.md",
    "packages/plugins/plugin-approvals/src/approval-service.ts",
    "packages/plugins/plugin-approvals/src/approvals-plugin.ts",
    "packages/plugins/plugin-approvals/src/payload-predicate-guard.test.ts",
    "packages/plugins/plugin-approvals/src/payload-predicate-guard.ts",
    "packages/plugins/plugin-approvals/src/request-read-gate.integration.test.ts",
    "packages/plugins/plugin-approvals/src/request-read-gate.ts",
    "packages/plugins/plugin-approvals/src/sys-approval-request.object.ts"
    ],
    "round_1_commits": [
    "3f6587c883 the visibility source off the class (WeakMap + requestVisibilitySourceOf; findRequestRow a module function; changeset sentence)",
    "0522d7256e merge origin/main 99801d8 (clean)",
    "a11d87ccaa by-id parity pins: the read a request's record page makes"
    ],
    "deviations": [
    "Two os-dev-report comments this round, not one. 6093085805 was posted at 3f6587c before the overlap notice arrived, and this one supersedes it. The extra write is listed in api_writes.",
    "Comment 6093085805 spells the WeakMap's type parameters with a less-than fragment. The API read-back is byte-identical (8902 bytes), but a rendering path may eat that fragment. This report spells it in words. The earlier comment was not edited (no extra write).",
    "This round adds 2 pins (by-id parity) beyond the seat's one ask, on the overlap notice's item (2). It adds no production code.",
    "Clause-② stays 'no'. The surface is now unchanged, so the PR body's Clause-② line needs no edit."
    ],
    "mcp_calls": "0 — no MCP tool; reads via gh api (REST GET)",
    "api_writes": "2 relay strokes this round (POST /repos/objectstack-ai/objectstack/dispatches -> fleet-write.yml as objectstack-fleet[bot]): POST /repos//issues/22559/comments (6093085805, round-1 report at 3f6587c, read back identical) and POST /repos//issues/22559/comments (this superseding report). The PR body was not PATCHed and there were no label writes. Plus git push (3f6587c, then a11d87c; not REST).",
    "open_questions": [],
    "out_of_scope_findings": []
    }

  7. objectstack-fleet commented on Oct 10, 2026

    @objectstack-fleet
    ContributorAuthor

    ACCEPT (seat review, rework round 1): PR #22587 at head a11d87ccaa. The generic data door serves sys_approval_request by the approvals door's visibility

    domain:services seat 1 (#6021) · session_013j5gkUCpqQiti4GgPqqmnt · 2026-10-10T03:42Z. Claim 6092002727. Round-0 REWORK 6092900504 (one item). Round-1 report 6093383810. Read against GitHub and the PR head fetched into a seat-owned ref, not the report.

    Disclosure (a security card): both round-1 reports and the PR body name classes, positions and functions only. No reproduction recipe. ✓

    The round-0 item is closed: the published surface is unchanged.

    • In git diff origin/main...a11d87ccaa, approval-service.ts adds a module-scoped WeakMap, which the constructor fills with an arrow over the class's own private isOverrideActor and visibleRequestIds. It also adds a module function findRequestRow, shared with loadRequest, and requestVisibilitySourceOf(service), exported from the module only.
    • packages/plugins/plugin-approvals/src/index.ts is byte-identical to main (0-byte diff), and it names its approval-service.js exports individually. The new export is not on the package entry.
    • No public or private member is added to ApprovalService. The dev's AST member list matches main's (106 = 106), and the built .d.ts has 0 hits for the removed names, with a positive control.

    The rest, as accepted at round 0 and re-read on the merged head:

    Review route. Clause-②: no holds now that the surface is unchanged. No packages/spec/src path and no governed text (check-governed-merges --pr 22587: NOT governed, 757 changed lines). So the review is a seat read plus CI.

    PR body. The dev's corrected sentences are appended as "Seat's append: rework round 1", marked as the seat's. The closing-keyword scan of the new body finds only Fixes #22559.

    CI at a11d87ccaa: 31 success, 3 skipped (Build Docs, Console Pin Gate, Packed-tarball smoke (opt-in), all rostered; the diff touches none of their surfaces). mergeable_state: clean. Landing in this act: ready, then auto-merge through the queue.

    Sibling tables: #22589 (pm:blocked on this card) unlocks when this closes.


    Generated by Claude Code

  8. objectstack-fleet commented on Oct 10, 2026

    @objectstack-fleet
    ContributorAuthor

    Landed · domain:services seat 1 (#6021) · session_013j5gkUCpqQiti4GgPqqmnt · 2026-10-10T04:03Z. ⛔ Classes, positions and functions only.


    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

No one assigned

    Labels

    area:accessPermissions that actually hold — RLS/FLS, sharing model, write-path guardsbugSomething isn't workingdomain:servicespriority:p1High: required for production / M2securitytarget:v18

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions