Skip to content

feat(plugin-approvals): serve the approval request page through the manifest pages, and declare the thread reply as approval_comment (#22473) - #22567

Merged
objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/issue-22473-approval-request-page
Oct 10, 2026
Merged

objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/issue-22473-approval-request-page

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #22473
Clause-②: no

What this does

@objectstack/plugin-approvals now serves sys_approval_request's record page as metadata, and it declares the request thread's reply as an action. This is the backend half that the maintainer's ruling 乙 on objectstack-ai/objectui#12045 (6079807016) assigns here:

the request page is a slotted page in objectstack's plugin-approvals, registered through the manifest's pages; objectui registers the renderers and mounts RecordDetailView

The consumer of both halves is the console's approval request page, built on objectui#12045. Its page-mount step waits on this PR. This PR does not close that card.

File (under packages/plugins/plugin-approvals/) Change
src/sys-approval-request.page.ts (new) SysApprovalRequestDetailPage: a slotted page, default for every request
src/approvals-plugin.ts the manifest registers pages: [SysApprovalRequestDetailPage]
src/sys-approval-request.object.ts adds a ninth declared action, approval_comment
src/translations/pages.ts (new), src/translations/index.ts pages.sys_approval_request_detail.label in en / zh-CN / ja-JP / es-ES, with recorded source digests
scripts/i18n-extract.config.ts declares the page and the pages baseline, so the coverage ratchet counts the label
src/translations/*.objects.generated.ts regenerated for approval_comment (node scripts/check-i18n-bundles.mjs --write); the zh-CN / ja-JP / es-ES leaves are translated by hand
tests, .changeset/22473-approval-request-page.md (minor) see Evidence below

The page is module-internal: index.ts exports nothing new, and exports has only .. So Clause-② is no. Nothing changes in packages/spec or platform-objects. Outside the plugin, this PR touches one test (packages/qa/dogfood/test/approval-override-composite-pin.dogfood.test.ts, which pins this object's served actions; patch round 1 at 69fef50a2) and one docs paragraph (content/docs/automation/approvals.mdx, the list of declared actions; cross-lane declaration on #6023).

The page's slot map, as served

This was read back from GET /api/v1/meta/page/sys_approval_request_detail on the showcase dev server (pnpm dev -- --fresh, HEAD 8f30e97c3), signed in as the seeded admin:

{
  "name": "sys_approval_request_detail",
  "label": "Approval Request",
  "type": "record",
  "object": "sys_approval_request",
  "kind": "slotted",
  "isDefault": true,
  "_packageId": "com.objectstack.service.approvals",
  "slots": {
    "actions": [
      {
        "type": "record:approval_decision"
      }
    ],
    "highlights": {
      "type": "record:highlights",
      "properties": {
        "fields": [
          "process_name",
          "object_name",
          "status",
          "current_step",
          "submitter_id",
          "updated_at"
        ]
      }
    },
    "tabs": {
      "type": "page:tabs",
      "properties": {
        "tabStyle": "line",
        "position": "top",
        "items": [
          {
            "label": {
              "en": "Details",
              "zh-CN": "详情",
              "ja-JP": "詳細",
              "es-ES": "Detalles"
            },
            "icon": "file-text",
            "children": [
              {
                "type": "record:details"
              }
            ]
          },
          {
            "label": {
              "en": "Timeline",
              "zh-CN": "时间线",
              "ja-JP": "タイムライン",
              "es-ES": "Cronología"
            },
            "icon": "history",
            "children": [
              {
                "type": "record:related_list",
                "properties": {
                  "objectName": "sys_approval_action",
                  "relationshipField": "request_id",
                  "columns": [
                    "created_at",
                    "action",
                    "actor_id",
                    "step_name",
                    "comment",
                    "attachments"
                  ],
                  "sort": [
                    {
                      "field": "created_at",
                      "order": "desc"
                    }
                  ],
                  "limit": 50,
                  "showViewAll": true,
                  "title": {
                    "en": "Timeline",
                    "zh-CN": "时间线",
                    "ja-JP": "タイムライン",
                    "es-ES": "Cronología"
                  }
                }
              }
            ]
          }
        ]
      }
    },
    "discussion": []
  }
}
  • actions holds one record:approval_decision node with no properties. The type was declared on main at 7731d7018f with an empty, strict props row.
  • highlights is the object's highlightFields without record_id. This is the seat's choice on the ui seat's design input 6081889955. The object's own highlightFields (sys-approval-request.object.ts:57) is unchanged. So record_id stays in the details grid, where the console draws the target-record card for the record_id / object_name pointer pair.
  • tabs has two tabs: Details (record:details) and Timeline (sys_approval_action as a related list on request_id, newest first). The page authors tabs, not details, because the console's builder (objectui buildDefaultPageSchema) takes tabs over details when both are present. The details body therefore lives in the first tab.
  • discussion: []: there is no record:discussion node. The thread's reply is approval_comment.
  • There is no aside, because the slot map has no right-rail slot.

approval_comment beside the route's own check

The declaration (sys-approval-request.object.ts:676):

name: 'approval_comment', label: 'Reply', type: 'api', method: 'POST',
target: '/api/v1/approvals/requests/{id}/comment',
params: [comment (textarea, required), attachments (file, multiple, optional)],
visible:
  'has(record.viewer) && has(record.viewer.can_act) && record.viewer.can_act == true'
  + ' || has(record.status) && record.status == "pending"'
  + ' && has(record.viewer) && has(record.viewer.is_submitter) && record.viewer.is_submitter == true',
locations: ['record_section'], refreshAfter: true,

The route's own check:

  • packages/rest/src/rest-server.ts:12751: threadRoute('comment', ...) calls ApprovalService.comment with actorId: body.actorId ?? body.actor_id ?? context?.userId. The declared action sends no actor, so the service acts as the caller.
  • approval-service.ts:4828: loadPendingRow refuses with INVALID_STATE unless status === 'pending' (:1440).
  • :4830 and :4833: the caller is admitted as the submitter, or as the holder of a pending slot (takenSlot, which calls heldSlot).
  • :4835: any other caller is refused with FORBIDDEN, unless the context is a system one.

The viewer block (attachViewers, approval-service.ts:7027–:7028) carries two flags:

  • can_act is status === 'pending' && heldSlot(pending, caller.userId, caller). That is the same slot test, already scoped to pending.
  • is_submitter has no status test, so the predicate adds the pending test that loadPendingRow applies.

So visible is can_act OR (pending AND is_submitter). That is the route's admission for every caller the declared action can produce. The route admits two more callers, and the predicate leaves both out: a system context (its viewer block is all false), and a caller who names another identity in actorId (the action sends none). So the predicate is narrower than the route there, never wider. It has no can_override arm, because the route has none. An override admin who holds no slot gets FORBIDDEN from the route. The parity is pinned against the real service for eight caller shapes (see Tests).

Done when: evidence

1. os validate passes on the plugin with the page. node packages/cli/bin/run.js validate packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts ran at HEAD 8f30e97c3 and exited 0:

  ✓ Validation passed (456ms)

  Data: 3 Objects  45 Fields
  UI: 1 Pages

None of the findings is located on the page: grep -c 'at pages\[' counts 0. The 57 warnings are all on the three objects (field-group-undeclared 45, field-no-consumers 9, title-format-retired 3), and none of them is new in this diff. A control from a temporary defineStack config in the package (deleted afterwards; git status empty) ran the same page with the panel type misspelled record:approval_decison. It exited 1 with rule: component-type-unknown at pages[0].slots.actions[0].type, and its fix line was "Rename record:approval_decison → record:approval_decision". The correct spelling of that config exited 0 (UI: 1 Pages).

2. The metadata API serves the page for sys_approval_request. 3. approval_comment posts a reply that the timeline lists. Both were measured on the showcase dev server (fresh database, seeded admin). A retitled announcement opens a showcase_dynamic_approval request, on which the admin is both the submitter and the pending approver:

HEAD 8f30e97c3 · showcase dev server --fresh on :42987
POST /api/v1/auth/sign-in/email -> 200
GET /api/v1/meta/page/sys_approval_request_detail -> 200
GET /api/v1/meta/pages?object=sys_approval_request -> 200
  pages for sys_approval_request: ["sys_approval_request_detail"]
GET page (Accept-Language: zh-CN) -> 200 label: 审批请求
POST /api/v1/data/showcase_announcement -> 201
PATCH /api/v1/data/showcase_announcement/ID (retitle: opens showcase_dynamic_approval) -> 200
GET /api/v1/approvals/requests?object=showcase_announcement&recordId=ID -> 200
GET /api/v1/approvals/requests/REQ -> 200 {"status":"pending","viewer":{"can_act":true,"is_submitter":true,"can_override":true}}
GET /api/v1/meta/object/sys_approval_request -> 200
  served approval_comment: target /api/v1/approvals/requests/{id}/comment · params [["comment","textarea",true,false],["attachments","file",false,true]]
  served approval_comment.visible on the served request row -> true
POST /api/v1/approvals/requests/REQ/comment {comment} -> 200 {"status":"pending"}
GET /api/v1/approvals/requests/REQ/actions -> 200
  [{"action":"submit","actor_name":"Dev Admin","comment":null},{"action":"comment","actor_name":"Dev Admin","comment":"Reply posted through the declared approval_comment target."}]
GET /api/v1/data/sys_approval_action?request_id=REQ (the Timeline related list's read) -> 200
  [{"action":"comment","comment":"Reply posted through the declared approval_comment target."},{"action":"submit","comment":null}]
POST /api/v1/approvals/requests/REQ/reject -> 200
GET /api/v1/approvals/requests/REQ -> 200 {"status":"rejected","viewer":{"can_act":false,"is_submitter":true,"can_override":false}}
  served approval_comment.visible on the rejected request row -> false
POST /api/v1/approvals/requests/REQ/comment on the rejected request -> 409 {"code":"INVALID_STATE","error":"request is rejected"}

zh-CN is the one translated locale this server serves (supportedLocales: ['en', 'zh-CN']). For ja-JP and es-ES it serves the English label. The control is the platform's own sys_user_detail page and the sys_approval_request object label, which fall back to English the same way there.

Tests

The same runs at HEAD 8f30e97c3 (after git merge origin/main and a rebuild of the closure):

  • pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2: Test Files 65 passed (65) · Tests 939 passed (939), VERDICT command-exit 0.
  • pnpm --filter @objectstack/plugin-approvals typecheck: exit 0, check:test-typecheck: OK — 8 file(s) / 324 error(s). The ledger is unchanged, and no new file carries an error.
  • New, src/sys-approval-request-page.test.ts (16 cases):
    • the manifest carries the page, and the page parses;
    • the slot map: one decision node, highlights equal to highlightFields minus record_id, the two tabs, no discussion, and every named field exists;
    • validateComponentTypes, validateComponentProps and runAuthoringRules('build') are clean on the page. Each has a control that goes red: a misspelled type, a prop on the decision node, and the build rules on the misspelled type;
    • the en label equals the page literal, and every locale serves its translation.
  • New, approval-service.test.ts → "approval_comment — its visible gate is the comment route's admission". It runs eight caller shapes against the real service: shown equals admitted for each, and the admin row is a real non-arm. A reply lands on the timeline with its attachments.
  • Updated: the sys-approval-request.object.test.ts roster, plus comment in its route verbs and the declaration pins; the action-predicate-sparse-face.test.ts count (8 → 9), plus the reply's pending-only submitter arm.

Ablations each ran through node scripts/ablation-replace.mjs. Each landed (anchor 1 → 0, blob moved), and each was restored with blob == HEAD and an empty git diff HEAD:

Mutation Result
visible gains a can_override arm the parity matrix goes red on exactly pending · an override admin on no slot (shown true, route FORBIDDEN), and so does the declaration pin
the submitter arm loses its pending test red on exactly approved · the submitter and recalled · the submitter (shown true, route INVALID_STATE). The first attempt's anchor missed, so the tool refused it and wrote nothing; it was re-run with the corrected anchor
the en page label is edited the served es-ES label becomes the edited English source. The recorded digests are consulted, and both translation pins go red
the zh-CN pages entry is dropped from the extract config check:i18n-coverage reports "untranslated declared strings grew 0 → 1" for this config, so the ratchet counts the label

Gates

The gates were derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack on 8f30e97c3. That is 65 commands, plus pnpm check:i18n-coverage because the extract config moved. All 66 exited 0 at 8f30e97c3. The reconciliation --ran read: "65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN … a DERIVED zero — all 65 recorded an exit code".

On the pre-merge head de7b645ee, check:dual-build-cjs-loads exited 3 with PREREQUISITE NOT MET, because eight packages outside the closure had no dist/. It passed once those were built. Among the gates: check:i18n (all nine bundle sets in sync), check:i18n-stale-fill, check:engine-double-contract, check:test-source-alias, check:page-declaration-shape, check:published-files and check:nul-bytes.

Lint was narrowed to the 14 changed .ts files: eslint --no-inline-config --format json gave 14 files, 0 errors, 0 warnings. The population is eslint.config.mjs's **/*.{ts,…} set. The config never enables type-aware linting (its own note: "no parserOptions.project, no typed @typescript-eslint rules"), so this diff cannot move the verdict on any untouched file. The repo-wide pnpm lint is left to CI.

Acceptance notes

  • Translations: the PM's mechanism assumption 4 is falsified in part. The page label is hand-authored (src/translations/pages.ts), the way platform-objects carries pages.* for sys_user_detail / sys_organization_detail / sys_position_detail in its LOCALE.ts files. The action's leaves did go through the tooling. The reasons, as measured:
    • This set's extract command is the objects-only one, so it emits the objects sub-tree alone.
    • No bundle set in the repo generates pages.
    • A config that documents the stack-wide spelling (--no-objects-only) is not carried by the gate: flagsFromDocstring (scripts/i18n-bundle-surface.mjs) returns ["--locales=zh-CN","--fill=default","--no-metadata-forms","--source-hashes","--out=o"] for it. The flag is dropped, so check:i18n would check an objects-only extract against a stack-shaped bundle. This is dormant: no config documents that spelling, and no owner is named.
  • The Details tab's grouping. record:details renders the object's field groups (Target / State / System), and sys_approval_request declares no fieldGroups. os validate already reports this as 20 field-group-undeclared warnings, so those fields render in the ungrouped bucket. This is pre-existing, unchanged here, and has no owner named.
  • A public-door refusal for a non-participant is NOT MEASURED on the dev server: the showcase refuses sign-up (POST /api/v1/auth/sign-up/email → 403), so there was no second account to try. The refusal is pinned against the real service instead (pending · a stranger → FORBIDDEN, alongside the existing outsider test).
  • The console's details/tabs precedence that shapes this page also bears on platform-objects' SysUserDetailPage. That page authors both slots.details and slots.tabs, so its details override is not drawn. This is reported to the seat, not changed here.

Generated by Claude Code

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

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 25 documentable anchor(s).

31 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json fbb065fd4b52dce103776e49fefd90f0138059a7.

⛔ 9 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 38 pages)
  • 6 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 6 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 fbb065fd4b52dce103776e49fefd90f0138059a7 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 45b80a920fad598bcd22d49bfde072b24e93dae6 — the merge of head 69fef50a225effda259adbc7db0bd247c551d259 into base fbb065fd4b52dce103776e49fefd90f0138059a7, 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 45b80a920fad598bcd22d49bfde072b24e93dae6 && git checkout 45b80a920fad598bcd22d49bfde072b24e93dae6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fbb065fd4b52dce103776e49fefd90f0138059a7 69fef50a225effda259adbc7db0bd247c551d259 && git checkout -B drift-repro fbb065fd4b52dce103776e49fefd90f0138059a7 && git merge --no-ff 69fef50a225effda259adbc7db0bd247c551d259

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

⚠️ 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 fbb065fd4b52dce103776e49fefd90f0138059a7 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…dden and refused for an override-only admin

Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 10, 2026 02:13
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 10, 2026 02:13
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 10, 2026
Merged via the queue into main with commit 5495331 Oct 10, 2026
44 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-22473-approval-request-page branch October 10, 2026 02:47
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/l tests tooling

Projects

None yet

2 participants