Repository navigation
fix(plugin-approvals): sys_approval_request declares its per-caller viewer block under attachedOnRead (#22387) - #22479
Conversation
…viewer block under attachedOnRead The object's 8 action `visible` predicates read `record.viewer.*`, a block `ApprovalService.attachViewers` attaches per caller on read. Declared under `ObjectSchema.attachedOnRead`, the shared build validator now resolves `record.viewer` and judges its second segment against the declared leaves. A conformance test drives the real service on a real ObjectQL over SqlDriver, for every caller shape `attachViewers` serves, through `listRequests` and `getRequest`, and holds the served keys and each value's runtime type to the declaration read from the object. Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
… the build pair and the object save door
Runs the object's own action `visible` predicates through
`validateStackExpressions`, `runAuthoringRules('build')` (the stack prepared
the way `os build` / `os validate` prepare it) and the object save door's
`runRuntimeAuthoringRules({ type: 'object' })`: every predicate that reads
`record.viewer` passes, a misspelt leaf is refused at all three naming the
declared leaves, and with the declaration taken away every reader is refused
again. `@objectstack/lint` joins the devDependencies, aliased to source in
vitest so the verdict is the current rule's.
Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ
Co-authored-by: Claude <noreply@anthropic.com>
…nt declaration Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. What this run could not see
Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 3126870c4187a03df512d1eef643f9f02fa7e38b && git checkout 3126870c4187a03df512d1eef643f9f02fa7e38b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dee7692f0bf5637c5c35609b9a27d42fe506b85d 506350e9f814735db5c2bfc195ea176fdbadeb4d && git checkout -B drift-repro dee7692f0bf5637c5c35609b9a27d42fe506b85d && git merge --no-ff 506350e9f814735db5c2bfc195ea176fdbadeb4d
node scripts/docs-audit/affected-docs.mjs --json dee7692f0bf5637c5c35609b9a27d42fe506b85d |
Fixes #22387
Clause-②: no
Part of #22211
What this does
sys_approval_requestdeclares its per-callerviewerblock (#3310) underObjectSchema.attachedOnRead(#22386, PR #22425), in the shape ruling6070963704on #22211 names:With the block declared, the shared build validator resolves
record.vieweron this object and judgesrecord.viewer.LEAFagainst the declared leaves. The object's own 8 actionvisiblepredicates pass. A misspelt leaf is still refused, and the refusal names the declared leaves. The declaration's second reader (ADR-0049) is a conformance test. It drives the realApprovalServiceand holds the served block's keys, and the runtime type of each value, to the declaration read from the object.packages/plugins/plugin-approvals/src/sys-approval-request.object.tspackages/plugins/plugin-approvals/src/sys-approval-request-viewer.conformance.test.tspackages/plugins/plugin-approvals/src/sys-approval-request-attached-on-read.test.tspackages/plugins/plugin-approvals/package.json,vitest.config.ts,pnpm-lock.yaml@objectstack/lintjoins the devDependencies, aliased to source for vitest (anchored regex, theplatform-objectsprecedent), so the pin reads the current rule and not alint/distbuild (check:test-source-alias).changeset/22387-plugin-approvals-viewer-attached-on-read.mdpatchfor@objectstack/plugin-approvalsNot touched:
approval-service.ts, so the per-callerviewersemantics (#3310) do not change. No other read attachment is declared (decision_progress,pending_approver_groups, the flow steps). Nopackages/spec,lint,formula,mcporcontent/docsedit.Reproduction
The object was judged on a stack of
{ objects: [SysApprovalRequest] }, throughvalidateStackExpressionsandrunAuthoringRules('build'). This was a one-off tsx probe, deleted after the run. Before = the branch basec512c255c; after = this branch.validateStackExpressionsunknown field `viewer` on `sys_approval_request`atobject 'sys_approval_request' · action 'NAME' visibleforapproval_approve,approval_reject,approval_reassign,approval_send_back,approval_request_info,approval_remind,approval_recall,approval_resubmitrunAuthoringRules('build')expression-invalid, the same 8 sites), 21 warningstitle-format-retired, 20field-group-undeclared, pre-existing)approval_send_backrewritten torecord.viewer.can_acttunknown field `viewer.can_actt` on `sys_approval_request` (the read attachment `viewer` declares `can_act`, `can_override`, `is_submitter`) — did you mean `viewer.can_act`?The same reading through the public door,
os validate(the built CLI on this branch). The probe was a scratchdefineStackconfig holding this object as shipped, plus one record-change flow on it (see F1); its control leg drops theattachedOnReadkey from the object, which is the base state:os validateattachedOnRead)Author-time rules failed (9 issues): the 8 actionvisiblepredicates plus the flow condition, eachunknown field `viewer` on `sys_approval_request`Validation passed, warnings onlyThe PM's mechanism assumptions, as measured: 1 holds (8 refusals on
origin/main). 2 holds (strict parse and the collision refusal pass, the 8 pass, and the misspelt leaf is refused naming the leaves). 3 holds:attachViewersemits exactlycan_act,can_override,is_submitter, allboolean, for all 5 caller shapes on both doors, and no extra key. 4 holds for the resolved type, with a nuance in the Acceptance notes. 5 is the door table below.The two field-existence doors (scope note
6075461181)Both doors were measured with a probe on a real
ObjectQLoverSqlDriver(better-sqlite3, in memory). The plugin's own objects were registered from its manifest. The probe was deleted after the run.record.*fromsys_approval_request?service-automation: the flow-registration resolver,setObjectSchemaResolver(plugin.ts:1185)registry.getObject(name).fieldsobjectName: 'sys_approval_request'is checked against it, and nothing refuses a record-change flow on this object. No shipped flow is one:git grepfinds none underpackages/orexamples/.plugin.tswires it, with the start conditionrecord.viewer.can_act == true. The door logsunknown field `viewer` on `sys_approval_request`as an advisory, before and after this PR, and registration is not refused. Therecordsuch a flow binds is a stored row. The probe checked three rows: theafterUpdatehook row the record-change trigger reads asctx.result; anengine.findrow, which is what the generic data door and atype: 'flow'action's subject load read; andgetRequest's row. The first two carry noviewerkey;getRequest's row does. Executing the flow over the hook row fails:condition failed to evaluate as CEL: No such key: viewer.recordnever carriesviewer. Threading the block would make the door accept a condition that fails on every run. The build-side disagreement this leaves is Acceptance note F1.packages/mcp: thevalidate_expressiontool (mcp-http-tools.ts:676)describeObject(name).fieldssys_object before it reads fields, unless the host registers the tools withallowSystemObjects: true. No shipped host does: the runtime's HTTP door passes onlygrantedScopes(packages/runtime/src/domains/mcp.ts:134), and the stdio plugin passes no options (packages/mcp/src/plugin.ts:656).MCPServerRuntime.handleHttpRequest, with a bridge that maps the object the way the runtime'sdescribeObjectdoes. With the shipped options it answersObject "sys_approval_request" is a system object and is not exposed via MCP. Only withallowSystemObjects: truedoes it reach field existence, answeringunknown field `viewer`(unknown-field).packages/mcpisdomain:cli's and is not edited here. See Acceptance note F2.Pins and ablations
Every ablation went through
node scripts/ablation-replace.mjs, in its wrap mode, underos-verify-lock. Each mutation was proven to land on disk by anchor count and blob change, and each restore was proven by blob equal to HEAD plus an emptygit diff HEAD. The subject is this package's ownsrc/, imported relatively.@objectstack/lintis aliased to source, so no rebuild sits between a mutation and the run.sys-approval-request-attached-on-read.test.ts(4 cases)visiblepredicates that readrecord.viewerpassvalidateStackExpressions,runAuthoringRules('build')(stack prepared withnormalizeStackInputandObjectStackDefinitionSchema, asos build/os validateprepare it) and the object save door'srunRuntimeAuthoringRules({ type: 'object' }). A misspelt leaf is refused at all three, naming every declared leaf. Control: with the declaration taken away, every reader is refused again.attachedOnReadblock deleted from the object (anchor 1 → 0, blobc4b45c28b39d→fc171e176107)unknown field `viewer` on `sys_approval_request`issues, where it expects[]. The misspelt-leaf case expects 1 refusal and gets 8. The population case gets 0 declared leaves. The control stays green, as it must: its verdict is the same either way.c4b45c28b39d== HEAD,git diff HEADemptysys-approval-request-viewer.conformance.test.ts(7 cases)getRequestandlistRequests: the servedviewerkeys equalObject.keys(SysApprovalRequest.attachedOnRead.viewer); each value passes the runtime test for its declared type; each shape is marked by its one true leaf; and across the battery every boolean leaf is observed bothtrueandfalse.attachViewersservescan_actasheldSlot(...), dropping!== undefined(blob9a1822845a2f→86ee0cb9a41a)a current pending approver via getRequest: `can_act` declared boolean, served "u_app"andcan_act across the battery: expected [ false, 'u_app', undefined ]9a1822845a2f== HEAD,git diff HEADemptyattachViewersalso emitscan_comment: trueemitted keys: expected [ 'can_act', 'can_comment', … ] to deeply equal [ 'can_act', 'can_override', … ]9a1822845a2f== HEAD,git diff HEADemptyThe first attempt at the extra-key ablation was refused by
ablation-replacebefore it measured anything: its replacement contained the anchor, so the anchor count did not drop. The tool restored the file to HEAD. The rerun above used a replacement that does not contain the anchor.Tests
All runs below are at HEAD
506350e9f, the last commit on this branch. Heavy runs went throughos-verify-lock; itsVERDICT command-exitline is quoted.pnpm --filter '@objectstack/plugin-approvals^...' --filter '@objectstack/lint...' buildandpnpm --filter @objectstack/plugin-approvals buildboth gaveVERDICT command-exit 0. The dist-reading gates ran afterturbo run build --filter=!@objectstack/docs, which built 72 tasks with 71 cached (VERDICT command-exit 0).pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2:Test Files 64 passed (64),Tests 916 passed (916),VERDICT command-exit 0. The two new files contribute 4 and 7 cases.pnpm --filter @objectstack/plugin-approvals typecheck:VERDICT command-exit 0, withcheck:test-typecheck: OK — … 8 file(s) / 324 error(s) / 27 pinned signature(s) held in test-typecheck-debt.json. The ledger is unchanged, and the two new test files compile with zero errors.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderives 77 families from this diff. All 77 ran, each with its exit code captured before any pipe, and all exited 0.check:i18nandcheck:dual-build-cjs-loadsfirst exited 3 (PREREQUISITE NOT MET, nodist/) and exited 0 when rerun after the build. Reconciliation (--ran):77 derived famil(ies) accounted for — 77 run, 0 NOT-MEASURED (a DERIVED zero — all 77 recorded an exit code and none of them is 3). Among them:check:test-source-aliasOK (the new alias),check:engine-double-contractexit 0 (no engine double was added; the conformance test runs on a realObjectQL),check:workspace-manifest-cyclesOK (517 edges, no cycle),check:type-check-debtOK, andcheck:nul-bytesOK.pnpm lint(eslint . --no-inline-config) is CI's run. Locally I ran the same binary and flags over the diff's lintable files. (1) Which files are lintable comes from eslint itself: the 4 changed.tsfiles are lintable, whilepackage.json,pnpm-lock.yamland the changeset answerFile ignored because no matching configuration was supplied. (2) The--format jsonoutput reportsfiles 4, errors 0, warnings 0. (3) This diff cannot change any untouched file's verdict:eslint.config.mjsenables no type-aware linting (noparserOptions.project, no typed rules) and no cross-fileimport/rules.turbotest sweep belongs to CI.mainhas moved 4 commits since the basec512c255c(rest, auth, fleet-write). None touches this diff's packages, so the branch was not merged.Acceptance notes
record.viewer.*in a flow condition onsys_approval_request, and that flow'srecordnever carries the block.@objectstack/lintapplies an object's declared blocks to every expression site bound to that object, including a flow's start condition (scopeflattened). Measured at the public door:os validateover adefineStackconfig holding this object plus a record-change flow on it (requires: ['automation', 'triggers'], start conditionrecord.viewer.can_act == true) answersValidation passed, exit 0. With the object'sattachedOnReadkey dropped, the same config answers exit 1 and names that condition withunknown field `viewer`. Meanwhile the flow-registration door still warns, and the flow fails at runtime withNo such key: viewer. The lint test written for spec(data):ObjectSchema.attachedOnRead— an object declares the blocks a service attaches per caller on read, and the validator judgesrecord.<block>.<leaf>against it (#22211 ruling A, spec half) #22386 records this as a ruled boundary: the ruling places the block in the field-existence set, which every surface reads. This PR does not change it, because there is nopackages/lintedit on this card. WhetherattachedOnReadshould be judged only where the bound row is a served row (object action predicates) is a question for thedomain:specseat that owns spec(data):ObjectSchema.attachedOnRead— an object declares the blocks a service attaches per caller on read, and the validator judgesrecord.<block>.<leaf>against it (#22211 ruling A, spec half) #22386. A field formula or a validation rule on this object reads a stored row as well. No flow in the repository readsrecord.viewer. Both spec(data):ObjectSchema.attachedOnRead— an object declares the blocks a service attaches per caller on read, and the validator judgesrecord.<block>.<leaf>against it (#22211 ruling A, spec half) #22386's changesets and this one are still pending release, so nothing has shipped this behaviour yet.validate_expressiontool. It is unreachable forsys_approval_requestin every shipped composition (table above). A host that setsallowSystemObjects: truewould getunknown field `viewer`forrecord.viewer.can_actat thevalidationsite. That is the same expression the build now accepts on this object's action predicates. The tool has no action-predicate site, and its field set readsfieldsonly. Not threaded here, because it isdomain:cli's file.SysApprovalRequestis unchanged.ObjectSchema.createreturnsServiceObjectwithOmitapplied tofields, intersected withPickof the literal'sfields, so onlyfieldscomes from the literal. The emitted declaration text does change. A declaration-onlytscemit of the object file, with and without the block, differs by 7 lines (readonly attachedOnRead: { readonly viewer: { … } }). Those lines sit inside the literal thatPickkeeps onlyfieldsof (lines 1017–4776 of the emitted file), soPickdiscards them. The served object definition, which is a value, gains theattachedOnReadkey. Themetadata-corefield-level-security masker passes that key through unchanged (OBJECT_REFERENCE_POSITIONS.attachedOnRead, spec(data):ObjectSchema.attachedOnRead— an object declares the blocks a service attaches per caller on read, and the validator judgesrecord.<block>.<leaf>against it (#22211 ruling A, spec half) #22386).packages/spec/liveness/object.json(theattachedOnReadrow's note) says the leaf types are "a declaration awaiting its named reader" until this test lands.packages/lint/src/authoring-rules.ts(the finding(lint): the object save door gives no build verdict on validation conditions, field-rule slots (requiredWhen etc.), option visibleWhen or action predicates; os build refuses them, a metadata save stores them (#22019's sibling) #22032 pass-4 comment) records "the build refuses 8 … the producer fix is plugin-approvals: sys_approval_request's 8 actionvisiblepredicates readrecord.viewer, a block the service attaches on read, and the shared expression validator refuses all 8 as an undeclared field #22211" as a measurement. Both aredomain:specfiles, so they are not edited here.title-format-retired, and 20field-group-undeclaredfor groups the object never declares infieldGroups) are pre-existing and unchanged. They are not this card's.visiblepredicates readrecord.viewer, a block the service attaches on read, and the shared expression validator refuses all 8 as an undeclared field #22211 is closed at landing by the seat, not by a keyword.check:closing-target-claimrefuses a closing keyword on card plugin-approvals: sys_approval_request's 8 actionvisiblepredicates readrecord.viewer, a block the service attaches on read, and the shared expression validator refuses all 8 as an undeclared field #22211: the governingClaim:on plugin-approvals: sys_approval_request's 8 actionvisiblepredicates readrecord.viewer, a block the service attaches on read, and the shared expression validator refuses all 8 as an undeclared field #22211 isdomain:specseat 3's (6062089126, branchclaude/issue-22211-approval-viewer-declaration), and an execution seat claims only in its own lane.domain:specseat 2 recorded "It closes when plugin-approvals:sys_approval_requestdeclares its per-callerviewerblock underattachedOnRead, with a conformance test againstattachViewers(#22211 ruling A, plugin-approvals half) #22387's PR lands." (6072125030), so thedomain:servicesseat 2 will close card plugin-approvals: sys_approval_request's 8 actionvisiblepredicates readrecord.viewer, a block the service attaches on read, and the shared expression validator refuses all 8 as an undeclared field #22211 by hand, citing that comment, once this merges. F1 is filed as lint: a declaredattachedOnReadblock is accepted in a flow condition on its object, where the bound row is the stored row and never carries it (os validatepasses, the flow fails at run time) #22481 for triage (domain:specby its file).@objectstack/lintis a devDependency only. The publishedfiles(dist,README.md,CHANGELOG.md) and the runtime dependencies are unchanged.Generated by Claude Code