Repository navigation
[finding] spec: APPROVER_ORG_SCOPED's docblock justifies team: false with "the engine never scoped it" — no longer true after the team org screen #10548
Description
Activity
Concentration-round first-touch: promoted
finding→pm:queue, Task XS,domain:spec. One-sentence justification rewrite exactly as the card drafts it — flag value staysfalse(retargetability answer unchanged), the justification stops citing an engine behaviour #10230 changed. Clause-② content limbno(docblock prose); path limb fires mechanically at the PR gate.
Generated by Claude Code
os-dev-report
{ "issue": 10548, "status": "done", "branch": "claude/issue-10548-approver-team-docblock", "pr": "https://github.com/objectstack-ai/objectstack/pull/11337", "premise_still_valid": true, "summary": "Rewrote the one drifted sentence in the APPROVER_ORG_SCOPED docblock (packages/spec/src/automation/approval.zod.ts) so it stops justifying `team: false` with 'the engine never scoped it'. The flag value is untouched — the diff is comment-only, no schema shape, no .describe() text, no accept/reject behaviour. Both halves of the premise were re-measured on today's origin/main (b9e9227e3), not recalled: the stale sentence still read verbatim as quoted at lines 258-260, and expandTeamUsers (approval-service.ts:1294) really does open with teamIsProvablyOutsideOrg (:1296), which reads sys_team.organization_id (:1350-1355) and drops out-of-org teams (:1358-1369), at both call sites (:1053 static team, :1228 expression resolveAs:'team'). One precision point worth the PM's eye: the two call sites pass DIFFERENT org arguments — :1053 passes the request's organizationId, :1228 passes directoryOrg — so the new wording says 'the REQUEST's organization', which is exact for this table row because on the team branch the two are provably equal (team is not org-scoped, so an organization declaration is refused upstream; the engine comment at :1048-1052 says so). No prose pin test exists: nothing in the tree quotes the sentence and approval.test.ts never references the constant, so there was nothing to update alongside. The generated surfaces carry only the export name, so they are byte-identical.", "tests": "All gates run with the exit code captured BEFORE any pipe (per-gate `rc=` lines from a runner script; no bare $? read through a tail). Gate families derived on the ACTUAL diff via `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (script reads its own change set from the merge base — 2 paths: the .zod.ts and the changeset); it named 22 families, all run plus check:nul-bytes = 23. UNION RE-RUN AFTER THE FINAL COMMIT and quoted from that run: at cae46c0c9, working tree clean, 23/23 rc=0. Two gates (check:doc-formula-expressions, check-dev-prereqs) were red on an EARLIER run for one environmental cause — the fresh worktree had only the spec dependency closure built; check-dev-prereqs states it outright ('The workspace is not built — 1 unmet precondition, not a list of problems', 66/67 packages missing dist) and doc-formula-expressions died on a missing @objectstack/formula/dist/index.mjs. A full `pnpm build --concurrency=2` (Tasks: 71 successful, 71 total) cleared both; neither red was attributable to this diff. Also: `pnpm --filter @objectstack/spec typecheck` rc=0, script names echoed in the output so it is not a zero-match silent pass — includes check:scripts-typecheck and check:test-typecheck ('OK — @objectstack/spec's test layer compiles under packages/spec/tsconfig.test.json'). `pnpm --filter @objectstack/spec test` rc=0 — 'Test Files 418 passed (418)', 'Tests 11128 passed (11128)', Duration 316.89s; invoked as `test -- src/automation` but the passthrough did NOT narrow vitest's selection, so this was the package's whole suite, broader than intended — reported as what actually ran, not as what was typed. No ablation was performed: this is a comment-only diff with no guard to mutate, so no rebuild/on-disk-mutation evidence applies. All heavy commands went through scripts/pm/os-verify-lock.sh; verdicts read from its VERDICT lines. Control-byte self-scan (grep -naP over the edited file, the changeset and the PR body) found no hits, and check:nul-bytes is green.", "open_questions": [], "out_of_scope_findings": [ "NOT filed as a new issue, deliberately: `pnpm install --workspace-concurrency=2` is rejected outright by pnpm 10.31.0 ('ERROR Unknown option: workspace-concurrency'), which cost this seat one round trip. Dedup found objectstack-ai/objectstack#9596 — the exact card, already CLOSED as completed, whose remedy IS in force (verified by reading .claude/agents/os-dev.md:102-103 on today's main rather than trusting the closed state; it is correctly scoped to --filter runs and claims nothing about install). A same-day sighting comment already characterizes the recurrence as a reader generalization rather than a wrong instruction. Filing a twin was therefore wrong and reopening was not mine to do; recorded a third-sighting frequency note on #9596 instead (comment 5385884074), explicitly as data for the domain:skills seat, proposing no text. Three agents in one day is the only thing it adds." ] }
Generated by Claude Code
- added a commit that references this issue
on Aug 23, 2026
Documentation drift found while implementing #10230. Observation class — no behaviour is wrong, and no code needs to change.
The stale sentence
packages/spec/src/automation/approval.zod.ts, in the docblock aboveAPPROVER_ORG_SCOPED:Two clauses, and after #10230 they no longer agree:
sys_team_membercarries no organization column" — still true, and still the reason a team's members are not individually placed.expandTeamUsersnow takes an organization and screens the team onsys_team.organization_idbefore expanding it, at both call sites.Why the flag value itself is still correct
team: falseis not wrong and should not be flipped. The table answers ADR-0105 D9 retargetability ("does anorganization:declaration apply to this type"), andteamstill consults no org-scoped directory — a declaration on it would still have no effect and is still rightly refused byresolveApproverDirectoryOrg. Only the justification drifted, by citing an engine behaviour that has since changed.The risk is the ordinary one for a load-bearing comment: the next reader deciding whether
teamneeds a screen finds a spec docblock asserting the engine has none, and concludes the work is outstanding when it has landed — or, in the other direction, reads the pairing ofteam: falsewithmanager: falseas still marking "the unscreened types", which after #10153 and #10230 it no longer does.Suggested repair
One-sentence rewrite of the justification, keeping the flag: say that
teamis org-agnostic for retargeting because it consults no directory, and note that the expansion is nonetheless screened to the request's organization on the team's ownorganization_id(#10230), so the flag is about targeting and not about tenancy.Not fixed in #10230's PR: that lane holds zero
packages/specownership by its dispatch fence.Related
teamapprover expansion is not organization-screened either — a cross-org team resolves into a request's approver slate #10230 — the team approver organization screenmanagerapprover resolvessys_user.manager_idwith no organization screen, while every sibling approver expansion is org-scoped #10153 — the same formanager, which made the pairing stale in the first placeFiled unassigned for triage.
Generated by Claude Code