Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

ADR-0056 的 Consequences 自相矛盾:「Capabilities 在 Setup 里作者化」与 P2/P4「在 Studio 设计、Setup 里没有 facet 可编辑」冲突 #3794

Closed

Description

@yinlianghui

在实施 #3308(退役 capability-multiselect widget 名)时通读 ADR-0056 求证组件归属,顺带发现。与 #3308 的修复面无关,单独记录。

事实

docs/adr/0056-permission-editing-studio.md 的 Consequences 首条(约 226 行):

One authoring path per concern. Objects/fields/RLS/tabs/admin-scope are authored once, in Studio, through validated editors … Capabilities are authored once, in Setup, through the sys_capability picker.

同一文件的 P2(约 203 行)说的是相反的事:

P2 — Studio: System Capabilities editor. ✅ A sys_capability multi-select (active, scope-grouped, labelled) added to PermissionMatrixEditPage … Capabilities are now designed here, not authored in Setup.

P4(约 215 行)把 Setup 侧关得更死:

With assignment on the Setup record page (P1b), … the editor is now purely a design surface, and no facet is editable as free-text JSON anywhere in Setup.

而 Decision 第 4 条(约 175 行)同样:「record view、inline edit、create/edit form 三处都把 facet 渲染成只读」。

代码站在 P2/P4 那边:sys_capability 多选(CapabilityMultiSelectField)由 Studio 的 packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx 直接渲染;Setup 侧六个 facet 一律走 permission-facet-link 只读摘要 + 深链(packages/data-objectstack/src/index.ts 的 applyFieldWidgetOverrides + packages/plugin-detail 的 PermissionFacetLink)。

所以 Consequences 里的「in Setup」是笔误或早期迭代的残留;正文其余各处与实现一致。

为什么值得记一笔(observation class)

纯文档失真,今天没有用户会撞到。但这句话出现在一份 Accepted ADR 的 Consequences 里 —— 恰恰是后来者(和 AI)用来回答「这个 concern 该在哪儿作者化」的段落,而 ADR 的全篇主张就是「one authoring path per concern」。一句反向陈述足以让下一个实现者在 Setup 侧重新长出一个能力编辑入口,也就是 ADR 本身要消灭的东西。

顺带:#3308 已经证明这份 ADR 的分期落地留下过一个同类残留(capability-multiselect widget 名),说明该文件的中间态描述值得整体过一遍,而不只是改这一句。

建议处置

把该句改为与 P2/P4 一致(「Capabilities are designed once, in Studio, through the sys_capability picker; Setup only assigns permission sets to users」),并顺带核一遍 Consequences 其余各条与 P1–P4 的实现是否同步。纯文档 PR,不搭代码改动。

参考

Activity

  1. os-project-manager commented on Aug 9, 2026

    @os-project-manager
    Collaborator

    Triage (maintainer-authorized one-off pass, 2026-08-09, registered on objectstack#6015): promoted finding → pm:queue. An ADR whose Consequences section contradicts its own P2/P4 rulings misleads every reader who stops at the summary — and this ADR governs where permission facets get authored, a question agents act on. Deliverable: reconcile the Consequences bullet to the P2/P4 direction (the later, specific rulings win over the earlier summary sentence), one docs-only PR. ⚠️ docs/adr/** edit: maintainer-merged only, per standing discipline.


    Generated by Claude Code

  2. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Aug 21, 2026
  3. self-assigned this
    on Aug 24, 2026
  4. yinlianghui commented on Aug 24, 2026

    @yinlianghui
    ContributorAuthor

    Claim: PM loop round 35
    Session: session_01CSoz9uGhaaSgiq3hshtN7L
    Branch: claude/issue-3794-adr0056-consequences
    Worktree: objectui-issue-3794
    Domain: domain:ui
    File surface: docs/adr/0056-permission-editing-studio.md — prose only, and nothing else in the repo (stop on breach; explain in the report)
    Container & model: S, mode:subagent, model: opus
    Clause-②: no — documentation prose; no schema, no runtime surface, no published API.
    Serial constraints cleared: docs/adr/** — no in-flight claim in this lane, zero claude/issue-* branches for open lane cards. Concurrent siblings #5522 / #3917 / #3880 / #5953 are disjoint by full path. ⚠️ #3917 also edits documentation, but under content/docs/**, a different tree.

    ⛔ GOVERNED SURFACE — this PR must NOT reach the merge queue

    docs/adr/** is a governed path. The standing discipline, restated by triage on this very card ("⚠️ docs/adr/** edit: maintainer-merged only"):

    • Open the PR as draft and leave it draft. ⛔ Never mark ready, ⛔ never enqueue, ⛔ never enable auto-merge.
    • Human merge is the review record. There is no machine that will stop you; this instruction is the only gate.
    • I will request review from / assign os-zhuang and list it under "awaiting a human merge" in the round report.

    Everything else about the PR is normal: Fixes #3794, a changeset if the presence gate asks for one (node scripts/check-changeset-presence.mjs — an empty frontmatter changeset is this repo's declared "releases nothing" form, and is the right answer for a docs-only diff).

    Adjudication (triage, 2026-08-09) — direction is settled

    reconcile the Consequences bullet to the P2/P4 direction (the later, specific rulings win over the earlier summary sentence), one docs-only PR.

    So: the Consequences bullet is the wrong text, and P2/P4 are right. ⛔ Do not "fix" P2/P4 to match Consequences, and ⛔ do not treat this as an open design question — the code already sits on the P2/P4 side, which the card measured:

    • sys_capability multi-select is rendered by Studio's packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx.
    • Setup's six facets all go through a read-only permission-facet-link summary + deep link.

    The card's own suggested replacement is a reasonable starting point, not a mandate:

    Capabilities are designed once, in Studio, through the sys_capability picker; Setup only assigns permission sets to users.

    The second half of the deliverable — and the part most likely to be skipped

    The card asks for more than the one sentence, and triage kept it:

    顺带核一遍 Consequences 其余各条与 P1–P4 的实现是否同步。

    #3308 already proved this ADR's phased landing left one stale residue behind (the capability-multiselect widget name), which is evidence the file has more than one. Read every Consequences bullet against P1–P4 and against the code, and report what you checked — including the bullets you found already correct. A finding of "the other five are accurate, here is how I verified each" is a complete delivery; silently fixing only the one sentence the card names is not.

    If you find a second contradiction whose correct direction is genuinely unclear, ⛔ do not guess — leave that bullet, and report it as an open question. An ADR is exactly the place where a confident wrong sentence does the damage this card is about.

    ⛔ Verify before you write: this ADR is 16 days past the card's measurement. Confirm the contradictory sentence is still there, with a control probe (something you know is present) so a zero-hit reads as real. If it has already been fixed, that is a with-evidence close, not a diff to manufacture.


    Generated by Claude Code

  5. yinlianghui commented on Aug 24, 2026

    @yinlianghui
    ContributorAuthor

    os-dev-report

    {
      "issue": 3794,
      "status": "done",
      "branch": "claude/issue-3794-adr0056-consequences",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5970",
      "premise_still_valid": true,
      "summary": "Premise re-verified before writing: the contradictory sentence was still present at docs/adr/0056-permission-editing-studio.md line 225 ('Capabilities are authored once, in Setup, through the `sys_capability` picker'), confirmed alongside two control probes that both hit (line 223 'One authoring path per concern', line 203 'P2 - Studio: System Capabilities editor'), so a zero-hit would have read as real. Reconciled it to the settled P2/P4 direction; P2/P4 untouched. The mandated audit of every other Consequences bullet found a SECOND contradiction of the same class in the same section - bullet 5 ('An admin who ... lacks `studio.access` can no longer design them ... hence P2 keeps that grant in Setup and first-class'), which is Option-B residue contradicted by four settled statements inside the same file (P2, Decision 1, Decision 3, A2-revised) and by the shipped deep-link. Fixed it as a separate self-contained hunk that a reviewer can drop on its own. Audit result over 6 Consequences claims: 2 fixed, 2 verified accurate against code, 2 deliberately left with the measurement reported (4b wording, 4c framework Q7). Diff is 1 file, +12/-6, prose only - fence honored, nothing else in the repo touched. GOVERNED SURFACE respected: PR opened draft and left draft (draft:true confirmed by read-back), never marked ready, never enqueued, no auto-merge. Issue body checked for sanitizer truncation: none found - all four sections (Fact / why-it-matters / suggested-disposition / references) close properly, blockquotes arrived as normal HTML-escaped text, and no short angle-bracket fragment is missing. The `documentation` label on the PR was applied by the repo labeler bot, not by me; no label was set by this agent (objectui has no `skip-changeset` mechanism and none was created).",
      "tests": "Docs-only diff, so no package build or test applies. Gate union re-run AT THE FINAL COMMIT c18be84e8 (git rev-parse --short HEAD from that run; `git status --porcelain` empty), each verdict quoted from the gate's own printed line, exit codes captured before any pipe: (1) `node scripts/check-changeset-presence.mjs` exit=0 -> 'No source of a released package changed in this range, so no changeset is owed.' plus '1 file(s) changed, 0 of them under the src/ of a package the release covers' - hence NO changeset, on the gate's own verdict. (2) `node scripts/check-control-bytes.mjs` exit=0 -> 'check-control-bytes: OK (scanned 4950 tracked text file(s); skipped 85 binary).' (3) `node scripts/check-doc-links.mjs` exit=0 -> 'Links are valid across 13 scan roots.' (4) `node scripts/check-doc-component-types.mjs` exit=0 -> 'Every documented component type is registered.' Plus a direct control-byte scan of the edited file: grep -naP over the C0 range exit=1 (no match, clean). DECLARED NARROWINGS, with the three required pieces of evidence: (a) `check:doc-snippets` was NOT run - proven out of scope by the script's own constant, check-doc-snippet-types.mjs:208 sets DOCS_ROOT = 'content/docs' while this diff is under docs/adr/; it also cannot run in this worktree (ERR_MODULE_NOT_FOUND: typescript, no node_modules installed for a docs-only change). (b) repo-wide eslint was NOT run - the population is read from eslint's own configuration, not from my guess: `lint:root` is `eslint . ... --ignore-pattern 'docs/**'` and `lint` is `turbo run lint` (per-package) while docs/adr belongs to no workspace package (no docs/package.json exists); file count is 1, taken from the changeset gate's own output; invariance for untouched files holds because the diff is prose inside one Markdown file - no eslint config, tsconfig or package.json changed, so no untouched file's verdict can move. CI runs the full farm regardless. EDIT-LANDED PROOF (not an editor exit code): the Python edit asserted each anchor matched exactly once before writing, then both directions were grep-counted on disk - removed text 'authored once, in Setup' = 0 and 'P2 keeps that grant in Setup' = 0; injected text 'designed once, in Studio' = 1 and 'structured editor becomes load-bearing' = 1. CODE VERIFICATION behind the audit (read, not assumed): CapabilityMultiSelectField rendered by packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx:980 bound to draft.systemPermissions; all six facets stamped permission-facet-link at packages/data-objectstack/src/index.ts:3397-3402 via applyFieldWidgetOverrides called from getObjectSchema at :3361 (sets only `widget`, never `type`); all three Setup surfaces short-circuit read-only - DetailSection.tsx:261, InlineFieldInput.tsx:248, ObjectForm.tsx:664-669; PermissionFacetLink.tsx has no onChange/input/textarea; P4 assignment removal confirmed by the comment at PermissionMatrixEditor.tsx:1057 plus RecordPermissionAssignmentsRenderer.tsx; PermissionFacetLink.tsx:71-78 builds the deep-link from the CURRENT appName via useParams, so from Setup it stays inside /apps/com.objectstack.setup/... and never cross-navigates to Studio; RowLevelSecurityPolicySchema and AdminScopeSchema are imported nowhere in the repo (only a comment at clientValidation.ts:526).",
      "open_questions": [
        {
          "question": "Consequences bullet 4b says the RLS/tab/admin-scope editors 'read RowLevelSecurityPolicySchema, tabPermissions enum, and AdminScopeSchema from @objectstack/spec'. Measured: neither symbol is imported anywhere in objectui - the only repo-wide occurrence is a comment at packages/app-shell/src/views/metadata-admin/clientValidation.ts:526 stating RowLevelSecurityPolicySchema must NOT be substituted; PermissionAdvancedFacets.tsx declares local mirror interfaces and documents the shapes as 'checked against the spec schemas rather than sampled from live data (objectstack#7130)'. Should the bullet be sharpened?",
          "options": [
            "A - leave as written: the sentence's own trailing clause ('the editors couple to those shapes') is exactly right and is its gloss on 'read', so under a fair reading the bullet is not wrong and a change would be a wording preference, not a direction correction",
            "B - sharpen to 'mirror RowLevelSecurityPolicySchema, the tabPermissions enum and AdminScopeSchema - checked against the spec schemas rather than importing them', which removes any chance a reader goes looking for an import or assumes spec-schema validation runs client-side"
          ],
          "recommendation": "A for this PR - I left it untouched deliberately rather than guess at wording where the facts, not the direction, are the only thing in question; the measurement is recorded in the PR body so the maintainer can take B in the same review with one line if they prefer it."
        },
        {
          "question": "Consequences bullet 4c (Q7) states that env-scope metadata saves do not project onto the queryable sys_permission_set data record the Setup summary reads. Is that framework follow-up still open?",
          "options": [
            "A - leave as written: it is self-labelled an open FRAMEWORK follow-up, P1-P4 are all objectui-side and none touches the projection, so nothing in this repo can confirm or falsify it and it is not a Consequences-vs-P1-P4 mismatch",
            "B - re-verify framework-side and, if the projection now tracks metadata saves, retire the bullet in a follow-up docs PR"
          ],
          "recommendation": "A for this PR - it is outside the fence and outside the audit axis. A targeted read of the framework plugin-security source found only the package publish path (draft to published sys_permission_set), not the env-scope live-save projection, so it is inconclusive from here rather than falsified."
        }
      ],
      "out_of_scope_findings": []
    }

    Generated by Claude Code


    Generated by Claude Code

  6. yinlianghui commented on Aug 24, 2026

    @yinlianghui
    ContributorAuthor

    ACCEPT — PR #5970 (R35, domain:ui)

    Reviewed against GitHub and against origin/main, not against the report's self-description.

    Checklist: draft ✅ · base main ✅ · Fixes #3794 first line ✅ · 1 file, +12/−6, prose only ✅ · fence honoured (nothing outside docs/adr/0056-permission-editing-studio.md) ✅ · mergeable_state: clean ✅ · no changeset, on the presence gate's own verdict ✅. The documentation label came from the repo labeler bot, not the agent.

    Spot-checks I ran myself, because hunk 2 is dev-initiated scope

    Hunk 1 is the sentence the card names, and it is a direct application of the settled direction. Hunk 2 is the dev's own find, so I verified its four load-bearing citations against origin/main rather than accepting them:

    claim verified
    Decision 3 says "No exception for system_permissions" ✅ line 169
    Decision 1 says "the same component, two entry points" ✅ line 162, with Setup's setup.access env-scope route at 161
    the bootstrap concern is resolved by an entry point inside Setup ✅ lines 171–173
    A2 revised — the bootstrap knot "doesn't arise" ✅ lines 51–56

    And the reading that settles it, which neither the card nor triage cited — the ADR's own Status line, line 3:

    Revised from the original Option B (which kept system_permissions editable in Setup as an exception) to the pure model below: Studio designs every facet, Setup only assigns + shows read-only summaries.

    So both edited bullets were Option-B residue in a document whose Status line already records Option B as superseded. The old bullet 5 ("hence P2 keeps that grant in Setup") did not merely sit oddly beside Decision 3 — it asserted the exception Decision 3 refuses by name. Finding it was in scope (triage kept the "核一遍其余各条" half), the fix is grounded in four statements already in the file, and keeping it as a separate self-contained hunk so a reviewer can drop it alone is the right shape for a governed surface.

    The audit reported its negative results too — bullets 2, 3, 4a verified accurate with code citations (PermissionMatrixEditor.tsx:980, applyFieldWidgetOverrides at data-objectstack/src/index.ts:3361/3397-3402, and all three Setup surfaces short-circuiting read-only). Reporting what was checked and found correct is what makes the audit half a real deliverable rather than a claim.

    Your two open questions — PM ruling

    4b (RowLevelSecurityPolicySchema / AdminScopeSchema "read … from @objectstack/spec") — option A, leave as written. Your measurement is right (imported nowhere; only a comment at clientValidation.ts:526, with local mirror interfaces in PermissionAdvancedFacets.tsx), but the bullet's own trailing clause "the editors couple to those shapes" is its gloss on "read", so this is a wording question, not a direction contradiction — and the ruled axis here is Consequences-vs-P1–P4 direction. Not worth a rework round. ✅ Declining to guess at wording where only the facts, not the direction, were in question was the right call, and putting the sharpened alternative in the PR body means the maintainer can take it with one line in the review this path requires anyway.

    4c (Q7 env-scope projection) — option A, leave as written. Self-labelled an open framework follow-up, nothing in this repo can confirm or falsify it, and it is not a Consequences↔P1–P4 mismatch. Outside the fence. Correctly left.

    ⛔ Landing: this PR does NOT enqueue

    docs/adr/** is a governed path. Per standing discipline and triage's note on this card, human merge is the review record:

    • Staying draft. ⛔ Not marked ready, ⛔ not enqueued, ⛔ no auto-merge.
    • Review requested from os-zhuang.
    • Listed under "awaiting a human merge" in the R35 round report.

    CI not yet read — the PR's checks started 10:06Z and this seat's rule is to read gates by name, once, ~11 minutes after the run's own started_at. Reading early is a discipline failure; the reading will be taken and posted on the PR. It does not gate this ACCEPT, since nothing here reaches the merge queue either way.

    #3794 stays open until the maintainer merges #5970.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Labels

domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatched

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions