Repository navigation
fix(example-crm): bind sales positions to crm_sales_user permission set - #8167
Merged
os-zhuang merged 2 commits intoAug 12, 2026
Merged
Conversation
…m_sales_user The CRM example declared three positions and a crm_sales_user permission set that never met -- no sys_position_permission_set seeding, and the set was not isDefault (which would grant every user, not just these three). Every persona assigned one of the three positions resolved only the everyone baseline and was 403'd on every CRM object. Mirrors examples/app-showcase/src/security/bind-position-sets.ts: binds the three positions to crm_sales_user imperatively on kernel:bootstrapped (cannot be a declarative seed -- the seed loader runs before the security bootstrap creates the sys_position/sys_permission_set rows), wired via a new onEnable export in objectstack.config.ts. Measured with `objectstack verify --rls`: the three per-position probe personas go from 18/18 probe-blocked to 3/18 (crm_opportunity_line_item, ungranted by crm_sales_user -- filed separately as #8164, out of scope). Zero RLS holes in either state. Reverse-verified: disabling the hook restores the exact 18/18 pre-fix baseline. Fixes #8060 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
size/m
documentation
…k deferral The workspace-root tsc program (tsconfig.json at repo root) has no dom/node lib types. examples/app-crm/src/security/bind-position-sets.ts is pulled into that program transitively via scripts/analytics-reconcile/app-crm.ts, so the fallback branch's `setTimeout(...)` (unreachable in practice -- the real runtime PluginContext always provides `.hook`, and no test exercises this branch) was an undeclared global there: TS2304, +1 over the frozen @objectstack/spec-monorepo DEBT entry (80). Replaced with `void Promise.resolve().then(run)` -- a pure-ES microtask deferral needing no ambient global, equivalent in this unreachable branch. Verified in isolation (tsc --ignoreConfig under the root's exact compiler options) and via the full `pnpm check:type-check-debt` re-measure: the spec-monorepo entry is back to exactly 80. Part of #8060 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
os-zhuang
marked this pull request as ready for review
August 12, 2026 19:24
os-zhuang
deleted the
claude/issue-8060-crm-bind-positions-to-permission-set
branch
August 12, 2026 19:44
akarma-synetal
pushed a commit
to akarma-synetal/framework
that referenced
this pull request
Sep 17, 2026
… is not work (objectstack-ai#18514) Fixes objectstack-ai#18299 A card released with a remainder **no dev can execute** — a maintainer ruling, a reading only the reporter can take, a confirmation owed by another seat — wears `pm:queue` with no assignee, which the queue view and 「最老优先」 both read as ready to hand to a dev. The label cannot tell that apart from real queue work; only the card's newest seat comment can, and reading that costs a full card read per candidate. H66 is the detector the card asks for: **report-only, a listing, never a relabel and never a gate verdict**. _Authored by `session_01Gqi43smmqjJ5sUrhfoPeKu` (Claude Code). Every reading below was taken on this branch at **`301de97`**, the head this body describes._ ## The clause it enforces Two lines of `.claude/skills/pm-dispatch/SKILL.md`, quoted verbatim in the row's own sentence: - 「`pm:queue` = 有具名落点或复现的具体缺陷,或范围明确的工具/门禁修复,无可问之事。」 — the label is a **definition**, not a habit. - 「释放是显式动作:让卡离手者同笔清 assignee + `Release:` 行(会话/因/去向);下一任重新认领。」 — the release act, whose 去向 field is the only thing that can say where the card went. ⛔ The row does **not** decide the state question. Whether these cards belong in `needs-user-decision`, in `pm:awaiting-maintainer` or in a spelling that does not exist yet is left open exactly as the card leaves it; the remedy the row prints is 「judge by hand — this is a listing」. ## The measurement — the dispatch's A1 was falsified before anything was built A1 assumed the machine-readable half is the `Release:` line's third field (去向). Measured 2026-09-16 over both boards the filing card names, read-only: | corpus | threads | `Release:` lines | release announced in a heading | |:--|--:|--:|--:| | objectui#7696 · objectstack-ai#8167 · objectstack-ai#8826 · objectstack-ai#8938 (the filer's four) | 4 | **0** | **3** | | objectstack, 25 most recently updated open unassigned `pm:queue` cards | 25 | **0** | 0 | **Zero of 29.** The lit control for that zero is the prose leg read by the same fetch over the same threads: it answers **three** on the same four specimens, so the zero is a property of the board, not of a broken reader. ⇒ Both legs ship, the canonical one **first**. It is kept, not dropped, for a mechanical reason: H47 exists to report exactly that absence, and as it is answered the canonical line starts appearing — on that day it is the only reading that can tell a return-to-queue (去向 = `pm:queue`, ⛔ never a finding) from an exit, because both are written in the same announcement heading otherwise. The summary clause publishes the zero on every run. The prose leg's three anchors are frozen beside the live comment each was measured on — objectui#7696 `5664140343`, objectstack-ai#8167 `5663517223`, objectstack-ai#8938 `5668851500` — and the third anchor named by the filing card itself is labelled `measured: false`, so nobody later reads it as a measurement. Matching is **heading-anchored** (H58's fence/blockquote/table/code-span refusals, reused): the filing card, this PR and the row's own docblock all quote the spellings, and none of them may fire.⚠️ `pm:queue` is **not** read over the whole comment body, and that is measured rather than stylistic: the two live release comments on objectui#8167 and objectstack-ai#8938 both say 「`pm:dispatched` stripped, `pm:queue` added」 in their state-write paragraph, so a body-wide read would clear precisely the specimens this row exists to list. ## The bounded fetch leg — and why the first landing needed one The row first shipped buying **nothing**: one `commentCache` lookup per card. The first live sweep measured what that costs — **194 candidates, a thread in hand for 1**. The corpus is thin *by construction*, not by luck: the only row that buys a card thread for a pm-tracked card is H2's claim read, and H2 buys one only for an **ASSIGNED** card, which is this population's exact complement. A listing that cannot see 99.5% of its own population is a row whose silence means nothing, and the filing card's own criterion already asks for the fetch — 「fetch the newest comment authored by a seat」. So a **bounded buy pass** now sits at the foot of the sweep: - **One page per card, the NEWEST one**, located from the card's own `comments` count through H65's exported `h65NewestPagePlan` / `h65CommentPagePath` rather than a second copy of the arithmetic. ⛔ Never page 1 on a long thread: GitHub serves issue comments OLDEST-FIRST, so a page-less read returns the archive (the objectstack-ai#18312 measurement — five weeks stale on a long thread). - **Ordered newest-touched first** (`updated_at` descending, `number` as a total-order tiebreak; an unreadable stamp sorts **last**, never promoted ahead of a dated card). A release is a write on the card, so that is where fresh evidence lives — deliberately the opposite of the dispatch order 「最老优先」, which says which card to *work*, not where the evidence *is*. - **Capped** by a named, frozen `H66_THREAD_BUY_CAP = 100`. The standing caller is `half-state-patrol.yml` on `PATROL_CRON` = `37 1,7,13,19 * * *` — four runs a day — under the Actions token's 1000 requests/hour per repository, beside the sweep's existing label pages, listings, PR windows, H50 walks, H43 probes, H45 parent reads and seat pages. - **A cap is not a coverage claim.** Candidates past it are NOT ATTEMPTED and counted separately (H40's posture). A fetch that fails, or a carrier whose `comments` count is unreadable *and* whose page 1 came back FULL (so the newest comment cannot be located), leaves that card **UNJUDGED** — never clean, the H4 three-state contract — and is counted as such. - **Placement is mechanical**: after every row that reads `commentCache`, because a newest page is not the same object as the first page that cache holds. The write-back is guarded to the one case where the bought page **is** the whole thread (`plan.page === 1`); `seatPostRowsFor` keeps its own window out of that map for exactly this reason, and this applies the same rule to the one case where the two coincide. A card whose thread is already in hand is judged in place, free, and never deferred — so never re-bought. `seatWindowAudit` gains **`seatPurchaseSites`**. The buy adds a second located-page purchase, so rather than raise the file-wide pin from 1 to 2 and lose the argument behind it, the seat-post bound is split out and stays at **1** — the new purchase is on a disjoint population (an unassigned `pm:queue` CARD, never a `pm:seat` post), so it cannot be the 「same page bought twice」 the original bound is about. ## Disjointness (the dispatch's A3), and the one overlap stated rather than hidden - **H8** requires `pm:dispatched`; this population is `pm:queue` without it. Disjoint by label. - **H47** leg (a) requires an assignee; leg (b) is the **inverse** of this row — it fires when a `Claim:` is the newest ownership record and no `Release:` answers it. On the canonical leg `releaseAnswersClaim` is true and H47 returns null. - **H49** requires `pm:dispatched` and an assignee. Disjoint. - **H58** reads the card's own declaration of what it IS; this reads the release record of where it WENT. A card may wear both and fire both — two readings, never one restated. -⚠️ **The overlap:** on the prose leg the release was never written as a canonical line, so H47 leg (b) fires too. That is correct and is a different defect (a record missing its canonical line) from this one (a card in the dispatch pool with a remainder). Asserted as a case, not hidden. Population excludes an assigned card (H24's row) and any card carrying a second member of `PM_EXCLUSIVE_STATE_LABELS` — that pair is H3's, H25's or H29's contradiction. One vocabulary, ⛔ not a second list. ## Battery roster and floor This file printed **one total** over a flat case list — the shape AGENTS.md 「Writing a `--self-test`」 names as evidence rather than proof. The handshake (`SELF_TEST_VERDICT`) was already here; the floor was not. This PR adds the other half, copied from `scripts/check-agent-model-declared.mjs` and ⛔ never imported: | battery | registered at `301de97` | pin | |:--|--:|--:| | `H66 released queue card` | 183 | **172** | Three failure conditions, each producing a real `process.exit(1)`: a declared battery below its pin; a case registered against an undeclared battery; the roster below `SELF_TEST_BATTERY_FLOOR`.⚠️ The roster is **partial and says so**: the several hundred pre-existing `t(...)` cases stay unbatteried, and retrofitting them is a diff of its own. Cases cover every clause the dispatch named — (a) a 去向 naming a non-queue owner ⇒ listed · (b) 去向 = `pm:queue` ⇒ silent, including where an announcement heading sits beside it · (c) both prose forms ⇒ listed · (d) any later comment ⇒ silent, structurally, because the row reads the newest · (e) an assigned card ⇒ outside the population · (f) no comment at all ⇒ silent — plus the seven quoting shapes that must not fire, the anchor set's cap/freeze/provenance, the three-input-state contract, band, registry coverage, and the summary clause in both media. The buy leg adds: the cap respected when candidates exceed it, newest-touched ordering with the undated card last, a bought page judged by the same predicate as a cached one, a failed buy read as UNJUDGED, and the two source-audited guarantees below. ## Measurements — all at `301de97` **Gates** — `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived **40** commands at `301de97`; all 40 run, each exit code captured by redirect-then-`$?` and never through a pipe, **40 exit 0**. Reconciled with `--ran`: 「40 derived famil(ies) accounted for — 40 run, 0 NOT-MEASURED (a DERIVED zero — all 40 recorded an exit code and none of them is 3)」. **Lint** — the whole-repo run, not a narrowing: `pnpm lint` = `eslint . --no-inline-config` over eslint's own population of **6,798 files**, exit **0** at `301de97`. **Self-test** — `node scripts/pm/check-half-states.mjs --self-test` and `pnpm check:pm-half-states` at `301de97`: exit 0, 「4546 cases pass. Batteries: H66 released queue card 183/172.」 **Live sweep** — report-only, from an agent container at `301de97`, exit **0**: 465 half-states over 397 pm-labelled issues. The H66 clause, the new **quadruple**: > Released queue cards (H66): **197** open unassigned `pm:queue` card(s) could be spoken about, **1** with a comment thread ALREADY in hand, **100** NEWEST comment page(s) BOUGHT this run (cap 100, **96** candidate(s) NOT ATTEMPTED at that cap), **0** listed. **0** card(s) stayed UNJUDGED. ⇒ the leg works and costs what it declares: 100 pages bought, every one read, nothing unjudged. The board genuinely has no card in this shape right now — the filing card's own four specimens were all dispositioned by hand between filing and this build. Before the leg the same sweep read `194 / 1 / — / 0`, i.e. the row could see one card in two hundred. **Request count** — the run total is **NOT MEASURED**: the script exposes no request counter, and the container's `/rate_limit` proxy reported a full 15000/15000 both before and after with the quota window rolling between the two reads, so the delta is not a measurement either. What *is* exposed is the leg's own cost, and it is exact: `queueReleaseBought` = **100**, one request each, printed on the summary line every run. **Ablations** — three, each mutating the committed tree, proving the mutation on disk by occurrence count, restoring with `git checkout HEAD -- path` under a `trap`, and verifying the restore by blob hash against `git rev-parse HEAD:path` plus an empty `git diff HEAD`: 1. the release-announcement anchor rewritten so it can never match ⇒ **29 named cases red**, suite runs to completion, exit 1. 2. the battery pin raised above the registered count ⇒ 「the battery floor failed — battery 「H66 released queue card」 registered 183 case(s), below its pin of 200」, exit 1. 3. the defer guard removed, so a thread already in hand would be re-bought ⇒ exactly **one** named case red — 「⭐ …and that place is guarded by the cache test, so a thread ALREADY in hand is never re-bought (got 0, want 1)」 — exit 1. Control after every restore: exit 0, 4546 cases pass. ⭐ The first ablation also found two real defects in the battery it was measuring — a direct `[0].heading` and a direct field read on a three-valued helper, each of which **aborted the whole suite** with a TypeError instead of reporting a case. Both are now read through describing readers, with the measurement recorded beside them. **Renderers** — both, offline over a synthetic finding: `renderPlain` and `renderMarkdown` each render the `H66` row, the clause with its counts, and the markdown family ledger names `H66`. ## Scope `scripts/pm/check-half-states.mjs` only. ⛔ objectui's copy is byte-pinned there and is not touched. `skip-changeset`: `scripts/pm/**` is the fast lane and nothing in any package's `files[]` moves. --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8060
What
examples/app-crmdeclared three positions (sales_rep,sales_manager,finance_approver) and acrm_sales_userpermission set granting CRUD on 5CRM objects, but nothing ever bound them together — no
sys_position_permission_setseeding, andcrm_sales_userwas notisDefault. Any user assigned one of the three positions therefore resolvedonly the platform
everyonebaseline and was 403'd on every CRM object.This mirrors
examples/app-showcase/src/security/bind-position-sets.tsexactly, per the triage ruling: a new
examples/app-crm/src/security/bind-position-sets.tsbinds the threepositions to
crm_sales_userimperatively onkernel:bootstrapped(adeclarative seed can't do this — the seed loader runs before the security
bootstrap creates the
sys_position/sys_permission_setrows), wired via anew
onEnableexport inobjectstack.config.ts.crm_sales_userisdeliberately not marked
isDefault— that would auto-bind it to theeveryoneanchor and grant every user the CRM permissions, changing theexample's security story rather than completing it (explicitly ruled out).
Measured (predict-then-mutate + reverse verification)
Reproduced the issue's own acceptance test with
objectstack verify --app examples/app-crm/objectstack.config.ts --rls:Before (confirmed matches the issue exactly):
After, per persona:
objects:6, probeBlocked:1, memberVisible:5.probeBlocked(no object grant at all — the by-id-write class neverexercised) drops from 18/18 → 3/18. The one remaining
probe-blockedobjectper persona is
crm_opportunity_line_item, whichcrm_sales_userdoes notgrant at all — a separate, pre-existing gap (filed as #8164, out of this
card's scope). Zero RLS holes introduced or found, in either state.
Reverse verification: disabled the
onEnablewiring, re-ran — readoutreturned to exactly the pre-fix 18/18
probeBlockedbaseline, confirming thehook (not something else) is the lever. Re-enabled and re-confirmed the fixed
readout before pushing.
A caveat on "proven": the report's strict
provenfield (⇒rls-consistent) stayed at 0 for the position personas even after the fix —not a defect of this change. All 5 now-granted CRM objects declare
sharingModel: 'public_read_write'("everyone reads+writes"), so once theobject gate opens, the record is visible to any authenticated holder by
design — the report classifies that as
member-visible(inconclusive), whichis correct and expected for this app, not
rls-consistent. Zero holes is thesignal that matters: no security regression.
Sharing rules (PM mechanism assumption #2)
The three
sharedWith: { type: 'position', value: … }sharing rules donot start mattering after this fix — measured, not assumed. They widen
record visibility on top of the object's OWD baseline, but every CRM object
they target is already
public_read_write(org-wide read+write for anyoneholding the object grant), so there is nothing left to widen. The sharing
rules remain inert on top of an already-maximal baseline; this is a property
of the objects' declared
sharingModel, unrelated to the binding fix.Out of scope
packages/plugins/plugin-security/**untouched, per the ruling.crm_opportunity_line_itemhas no object-level CRUD grant in any app-crm permission set — the platform's own build lint already flags it #8164 filed (unassigned, unlabeled) forcrm_opportunity_line_itemhavingno object-level CRUD grant in any app-crm permission set — the platform's
own build-time lint (
security-master-detail-ungranted) already flags thisindependently of the binding gap this card fixes.
Tests
objectstack verify --app examples/app-crm/objectstack.config.ts --rls(see above) — the issue's own acceptance test, run before/after/reverted.
cd examples/app-crm && npx vitest run— 27/27 passing (unchanged; no testfile added, following the showcase's own precedent of proving the binding
via the live
verify --rlsprobe rather than a unit test).cd examples/app-crm && npx tsc --noEmit— clean.pnpm --filter '@objectstack/example-crm...' build— clean, only thepre-existing (now separately filed)
security-master-detail-ungrantedwarning on
crm_opportunity_line_item.pnpm check:changeset-gate-self-tests,check:i18n-coverage(afterrebuilding the showcase closure to clear a stale-dist false positive
unrelated to this diff),
check:objectui-changeset,check-changeset-no-major.mjs,check:nul-bytes— all green. Notecheck:objectui-pin-fresh/check-objectui-pin-fresh.mjsare red, butpre-existing and unrelated (ambient objectui pin staleness, independent of
this diff's content).
Generated by Claude Code