Skip to content

fix(devx): price the read-seam wrapper depth bound and pin it from above - #13472

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-12360-read-wrapper-depth-bound
Aug 30, 2026
Merged

os-project-manager merged 1 commit into
mainfrom
claude/issue-12360-read-wrapper-depth-bound

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

Fixes #12360

The card's framing is the whole job: the bound is documented, the number never was. MAX_READ_WRAPPER_DEPTH = 2 has carried an honest limitation notice since the day it was written — the walk "cannot DISCOVER a seam whose read runs outside the try block, or more than MAX_READ_WRAPPER_DEPTH hops away. A ratchet, not a proof." What has never been recorded is what that ratchet costs, and a constant whose price nobody has measured is held in place by inertia rather than by a reason.

This PR does not move the bound. It prices it, proves every seam behind the price is real, and makes the number impossible to raise in silence or to go stale in a comment.

Re-derived on the merged ref, not inherited

The card's figures (64 to 70 at depth 6, "+8/-2", "5 of the 8 deltas") were taken on 3ddad51b5c, five days and two recognizer changes ago. PR #13444 landed in this exact file hours before this branch was cut. Everything below was re-run on origin/main @ 71627f7b4e.

The headline number reproduces exactly: 6 real seams. The route to it does not, and both corrections matter to the next person who re-runs this census.

recognizer read seams
same-tick walk, depth 1 58
today — same-tick walk, depth 2 66
same-tick walk, depth 3 72
same-tick walk, depths 4 to 50 72 (saturated)
  • Saturation is at depth 3, not 6. Every one of the six chains is exactly three hops. The card probed 6 and could only read saturation as "at or below 6"; it is one hop past the bound, for all six.
  • The admitting step is +6 / -0, not "+8 / -2". The 2 were never seams leaving the population — they are two try lines re-attributed to a different first-matching callee. That still happens here (engine.ts:9741 and :10906 move from resolveMasterDetailParent(s) to mediaValueShapeStrictFor) but at depth 4, one level above the level that admits anything, and it moves no count.

Every admitted seam shown real before anything was proposed

The dispatch was explicit that a depth increase is the other direction of the risk #13444 closed, so each of the six was read at its call site — chain traced through the recognizer's own resolution, not assumed:

# seam (try line, first wrapper) chain to the read
1 protocol.ts:10497 getMetaItemCached getMetaItem, findDraft, lookup, engine.findOne
2 protocol.ts:13874 saveMetaItem getMetaItem, findDraft, lookup, engine.findOne
3 protocol.ts:14850 migrateStoredMetadata saveMetaItem, refuseUnmintableMetaType, metaTypeNamespaceExists, engine.findOne
4 protocol.ts:16502 publishPackageDrafts promoteDraftForPublish, lockWriteRefusal, getEffectiveLock, engine.findOne
5 protocol.ts:17565 duplicatePackage saveMetaItem, refuseUnmintableMetaType, metaTypeNamespaceExists, engine.findOne
6 lifecycle-service.ts:664 sweep reapObject, archiveObject, archivePass, hot.find

All six are unbroken await chains on the caller's own tick ending in a genuine driver read. Two are not the tails the card predicted: 3 and 5 reach a read through saveMetaItem's own precondition check, not through the getMetaItem tail assigned to them. Same verdict, and the difference is the argument for reading chains rather than assuming them — the walk stops at the first read it finds, which is not always the one a human tracing by hand would land on.

What admitting them would do to the gate: nothing. All six carry no invented answer, and every parenthetical in the rule's verdict line (8 type-discriminated, 1 pass-through, 1 answer-by-assignment, 1 baselined) is byte-identical at depth 2 and at depth 50. Only the denominator moves, 66 to 72 — and that denominator is what #5186, #6451, #9165, #8845 and #8901 are quoted against. The bound is not buying correctness here; it is holding a published number still.

Which half of "5 of the 8" survived

Decomposed rather than reported as one number, with the walkAll probe run beside the depth probe and both compared to today's 66 by try line:

set count
walkAll @ depth 2 admits 7 was 8 — #13444's guard deleted the fake seam
depth 3 admits 6
both 5 the claim, reproduced exactly
depth only 1 protocol.ts:16502 publishPackageDrafts
walkAll only 2 sys-metadata-repository.ts:883, engine.ts:9571

So the number 5 survives and its denominator does not: read it as "5 of 7", the eighth having been the fake seam PR #13444 removed. And a correction the card could not have made — walkAll is only a partial mask for this bound. publishPackageDrafts is admitted by depth and by depth alone: counting call hops reaches its read, descending lexically through nested declarations does not.

The dispatch hypothesis, measured and falsified

My reading: #13444's new contradictsWrapperResolution may already refuse some of the 6, because a deeper hop is exactly where a wrong-receiver resolution compounds.

It refuses none of them. The full checker output is byte-identical with that guard enabled and disabled at depths 2, 3, 6 and 50. Its subject is the walk, not the depth:

walk depth guard on guard off guard removes
same-tick 2 66 66 0
same-tick 3 72 72 0
same-tick 50 72 72 0
walkAll 2 73 74 1
walkAll 3 74 76 2
walkAll 50 75 77 2

Two readings, and the second is the one to carry forward. The depth axis never arms the defect #13444 closed, so a depth raise is safe from that defect. But if the walk is ever widened as well, raising the depth doubles what that guard has to catch. The two widenings cost independently and risk multiplicatively.

And a resolution hazard the guard cannot see, found while verifying the six rather than looked for. functionBodies is keyed by bare name and is last-wins. protocol.ts declares lookup three times (bodies at 6657, 6717, 7129), and the hop in seams 1 and 2 resolves the call at 6674 to the declaration at 7129 — not the one lexically enclosing it. contradictsWrapperResolution admits it and is right to: the receiver is a bare identifier and the arity matches, because the three declarations are near-copies of one another. Both seams are real anyway (all three lookup bodies read sys_metadata through this.engine.findOne), so nothing is miscounted today. But the verdict is right by luck, and each additional hop multiplies the number of names that must be unique for it to stay right. That is the strongest argument on this tree for leaving the bound alone, and no seam count can make it.

The finding that made this worth landing: the bound was pinned from below only

Measured by ablation, not assumed. Lowering MAX_READ_WRAPPER_DEPTH to 1 reddens exactly one fixture (#13444's this-rooted chain, which needs two hops). Raising it to 3, 4, 6 or 50 left all 51 read-seam fixtures green and every gate in this file green — while the census denominator moved 66 to 72 with nothing anywhere saying so. A tunable five other cards quote must not be movable in silence. That is this card's own numbers going stale twice, one level down.

What ships

  1. The measurement, in the header — THE DEPTH BOUND'S COST: the table, the six seams by name with their chains, the walkAll decomposition, the interaction matrix, the lookup hazard, and why the bound is nevertheless held.
  2. A JSDoc on the constant itself, which had none — its 18-line rationale block sits 150 lines above, beside DRIVER_READ_CALLEES. Deliberately not moved; the new block names it and adds the half that was missing (the price), so the two read as a pair rather than a duplicate.
  3. --depth-cost — a diagnostic that re-derives the price on demand and always exits 0. It prints admitted and removed seams by name per step, re-attributions per step (invisible in any count, and reading one as a removal is what produced the card's "-2"), and states its own probe ceiling rather than claiming saturation it did not test. Both stale readings of this number were produced by hand-patching the constant and re-running; this is that step, kept.
  4. Two self-test fixtures pinning the bound from above (51 to 53 cases). A three-hop ladder, one hop past the bound, plus its discrimination test — the same source at MAX_READ_WRAPPER_DEPTH + 1, which must find exactly one seam or the pin is asserting nothing.

maxDepth is threaded through isReadCall, analyzeReadSeams and a new collectReadSeams (split out of runReadSeamRule). Nothing in the audit path passes it, so the shipped verdict is the constant's.

The deliverable is a measured zero-delta

--list output is byte-identical before and after on this tree — verdicts, counts and every seam line. diff of the complete run, not just the totals.

Tests

Both legs, both quoted from the gate's own verdict lines, on final head 0f2bb1ce:

  • --self-test: ✓ self-test (log-level rule): 63 case(s) passed and ✓ self-test (read-seam invention rule): 53 case(s) passed, and the baseline offer stays marked maintainer-only (#8435)
  • main check: ✓ read-seam invention (#5186 + #6451 + #9165, 3 package roots, vocabulary find/findOne/count): 66 read seam(s), none invents an unreported answer and ✓ durability-degradation log levels: 29 durability-critical catch seam(s), all loud, rethrowing or propagating to the caller

Both new fixtures proven non-vacuous by ablation. Predictions were written first and both held; each mutation was confirmed on disk by blob hash before the reading was taken, and each restore verified by blob hash against the HEAD blob plus an empty git diff HEAD:

ablation prediction observed
MAX_READ_WRAPPER_DEPTH 2 to 3 only the pin reddens; main goes to 72 exactly that — expected violation=false seams=0, got violation=true count=1 seams=1, discrimination case still green, main prints 72
analyzeReadSeams ignores the depth option only the discrimination case reddens; main unchanged exactly that — [at maxReadWrapperDepth=3]: expected violation=true seams=1, got violation=false count=0 seams=0, pin still green, main prints 66

Gate families derived on my own final head with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (13 families, from git's own change set, not a hand-written list). All green: check:agent-test-spelling, check:bash32-floor, check:cli-command-ids, check:cross-package-test-inputs, check:durability-log-level, check:entry-guard, check:parse-guard, check:pnpm-filter-targets, check:watch-hint-literal, check-ci-filter-parity.mjs, check-cross-package-test-inputs.mjs, check-shard-attestation.mjs. Convention obligations green: pnpm check:pm-dispatch-gates (944 cases), node scripts/pm/bare-root-worklist.mjs --self-test (51 live rows, none stale), node scripts/check-self-test-wired.mjs (151 scripts), pnpm check:nul-bytes (75 assertions), plus a grep -naP control-byte sweep over the diff.

The two pin tests that read this gate's own source — sys-metadata-repository.draft-drain.test.ts and protocol.metadata-store-outage.test.ts — pass: Test Files 2 passed (2) / Tests 44 passed (44), after building the dependency closure.

Full-repo pnpm lint under the shared verify lock: 5494 files, 0 errors, 0 warnings, with the edited file confirmed present in the run (counts read from --format json, not from a summary line).

Recorded as NOT MEASURED, not as a failure: node scripts/check-test-completeness.mjs exits 3 with PREREQUISITE NOT MET — it grades a saved turbo run test log that CI tees and this run has none. Its own text says the branch is unreachable in CI.

Changeset

None. This is a CI-internal gate script that publishes nothing from any package, which .github/workflows/lint.yml calls the textbook skip-changeset case in as many words. The skip-changeset label is applied. PR #13444, on this same file hours ago, took the same route.

The decision this measurement does not take

Whether the bound should move is now a question with a price attached rather than an unknown, and it is deliberately left to the maintainer. The case for raising it: 6 verified-real seams are missing from a population that five cards quote. The case against, all measured here: admitting them changes no finding, only a number; it triggers the census re-run #8901's restart conjunct (b) reserves; and the last-wins name-resolution hazard grows with every hop. The card's own standing value is preserved either way — anyone re-running that census must read 66 as bound-dependent, and --depth-cost is now how they check the dependence instead of remembering it.


Generated by Claude Code

The bound was documented from the day it was written; what it COSTS never
was. Measured on 71627f7: 6 real read seams, all admitted at depth 3,
unchanged to depth 50, all six carrying `no invented answer` — so the
bound holds a denominator still, it does not buy a finding.

Before this change the constant was constrained from BELOW only: lowering
it to 1 reddens one fixture, raising it to 3/4/6/50 left all 51 read-seam
fixtures and every gate in this file green while the census moved 66 to
72. Two fixtures now pin it from above, and `--depth-cost` re-derives the
price on demand so it cannot go stale in a comment again.

Zero-delta: the full `--list` output is byte-identical before and after.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 30, 2026
@os-project-manager
os-project-manager marked this pull request as ready for review August 30, 2026 14:14
@os-project-manager
os-project-manager added this pull request to the merge queue Aug 30, 2026
Merged via the queue into main with commit 0e810dd Aug 30, 2026
34 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-12360-read-wrapper-depth-bound branch August 30, 2026 14:33
os-tesla pushed a commit that referenced this pull request Oct 25, 2026
…json, pnpm-lock.yaml and the root manifest resolve the workspace root upward (objectstack#25723) (#13484)

Part of #25723 (sub-issue 2b of
#25677, the objectui → objectstack `ui/`
merge). Follow-up of #13472.

## What
Adds `scripts/workspace-root.mjs` (Node builtins only, importable before
`pnpm install`) and its `.d.mts` twin: `workspaceRoot()`,
`workspaceGlobs()`, `lockfilePath()`, `rootManifest()`,
`uiTurboConfigPath()`, `uiScopedTurboConfig()`. Every reader of
`pnpm-workspace.yaml`, `pnpm-lock.yaml`, the root `package.json` and
`turbo.json` under `scripts/`, `scripts/__tests__/`, two package tests
and the five e2e specs resolves those files through it. The three ad-hoc
`packages:` parsers are unified (union of their refusals).
`check-ui-root-lookups` learns the `marker-walk` shape that let the e2e
`REPO_ROOT` IIFEs escape #13472, with self-test cases. Standalone
behaviour is byte-identical: every helper branch is the identity when
`<uiRoot>/pnpm-workspace.yaml` exists.

## Why
Once objectui is `ui/` of the objectstack monorepo, the workspace root,
lockfile and overrides live one directory up and `turbo.json` is parked
as `turbo.objectui.json` (a nested turbo.json must `extends` the root).
Readers that assumed those files at `uiRoot()` fail with ENOENT
(measured on the verification PR #25718: Type
Check UI at `check:side-effects-array`, Build & E2E UI at the first e2e
spec, 15 of 16 Test UI shards) or walk up to the wrong root (five e2e
specs). This PR makes them layout-aware ahead of the import.

## Verification
| command | standalone | merged layout (`ui/` in objectstack, scratch
copy of the verification tree) |
|---|---|---|
| `pnpm check:ui-root-lookups` / `--self-test` | 0 / 0 (17 cases) | 0 |
| `pnpm exec vitest run scripts/__tests__` | 0 (195 files, 5772 tests) |
card's failing files + every touched test: all reader reds green; 10
remaining reds are the spec-pin gates (spec is `workspace:`
post-import), the QUICK_REFERENCE script list, the vendor-chunk arm, one
dom test — none a root-file read, all carried on objectstack#25697 |
| `pnpm --filter @object-ui/cli test` | 0 (415) |
`workspace-vite.test.ts` green |
| `pnpm check:side-effects-array` | 0 | 0 |
| `pnpm check:docs-route-closure` | 0 | 0 |
| `pnpm check:lockfile-dedupe` / `check:lockfile-integrity` /
`check:installed-pin-claims` | 0 / 0 / 0 | n/a (the pin gates presuppose
the pin) |
| `pnpm type-check:scripts` / `type-check:e2e` | 0 / 0 | 0 / 0 |
| `pnpm lint:coverage`, `lint:root`, the pre-push gate list | 0 | — |
| `playwright test --list --project=chromium` | 0 (78 tests) | 0 (78
tests, 0 ENOENT) |

The same patch is on the verification branch as objectstack commit
1cade69 (run 3 of #25718).

## Landing
Tooling and tests only; no product code
(`packages/cli/src/utils/workspace-vite.ts`, `app-generator.ts`
unchanged); no governed surface touched. Changeset
`.changeset/25723-workspace-root-readers.md` with empty frontmatter —
present because two package test files under `src/` changed; releases
nothing. Draft until CI is green; then the epic PM asks for review.

Clause-②: no
Metadata-protocol: none

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01RSDL3SFtJaKzcymvCD8RsU

---------

Co-authored-by: Claude <noreply@anthropic.com>
os-tesla pushed a commit that referenced this pull request Oct 25, 2026
…ssions groups, at an objectstack pin that declares them (objectui#13478) (#13485)

<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} -->
_Requested by **Elon** · [project
thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DSu9uoz6bLHskQmxnyBE86T)_

Fixes #13478
Clause-②: yes
Metadata-protocol: none

objectstack's `Console Pin Gate` is red on main because app-shell no
longer type-checks against objectstack `47af3dde42`
(#25701). That change added the `positions`
and `permissions` groups to the per-app `TranslationDataSchema`, and
`TranslationPreview`'s `CATEGORY_DEFS` is exhaustive over that schema's
shape by `satisfies`. This PR adds the two rows and moves objectui's own
pin to `47af3dde42`, so objectui type-checks against the same spec. Once
it merges, objectstack can move its `.objectui-sha` past the merge
commit (objectstack's own card names this one as its blocker).

## What changed

- **`TranslationPreview.tsx`**: two `CATEGORY_DEFS` rows, in the
schema's order (after `flows`, before `metadataForms`): `positions` with
`Users` and `permissions` with `ShieldCheck`, the icons
`PositionPreview` and `PermissionPreview` already draw. Both are nested
groups (`flat: false`), so each card counts the group's top-level keys
and samples them with their inner key count, as `objects` and `flows`
do.
- **`metadata-admin/i18n.ts`** (the designer table, the only pack these
keys live in, `en` and `zh`):
`engine.translationPreview.category.positions` = "Positions" / "岗位" and
`engine.translationPreview.category.permissions` = "Permission Sets" /
"权限集", the words the same table already uses for the two metadata types.
- **Tests**: `TranslationPreview.categories-11765.test.tsx` gets a
spec-valid fixture for each new group (its first case requires one per
schema group). `adminPreviews.i18n-10862-s2.test.tsx` lists the two
headings among the rendered locale sites and reads coverage over 13
groups (`2/13 (15%)`, was `2/11 (18%)`).
- **Changeset**
`.changeset/13478-translation-preview-positions-permissions.md`:
`@object-ui/app-shell` patch.

## The pin

- `.objectstack-sha` moves from `e649c2d91e` (the 18.1.0 release commit)
to `47af3dde42bb4e77fd92025605b89b4a473295a4`. REST compare
`e649c2d91e...47af3dd`: `status: ahead`, `ahead_by: 51`, `behind_by:
0`; the bump script also reported "(on origin/main)". It is pinned at
the card's floor, not at objectstack main's head: the four commits after
it on objectstack main are left for the next bump.
- Path: `scripts/bump-objectstack.sh
47af3dd --no-commit` (built and packed
the 7 packages), then `pnpm install`.
- What moved besides the sha: three packed manifests in
`.objectstack-pin/` (`core`, `lint`, `spec`), each only `zod` `^4.6.5`
to `^4.6.6` (#25657). The lockfile already
resolves `zod@4.6.7`, so **`pnpm-lock.yaml` is unchanged** and `pnpm
install --frozen-lockfile` exits 0.
- **Why the rows and the pin land together** (the seat's hypothesis; it
held). At `e649c2d91e` the schema declares neither group: `grep -c '^
positions: z.record'` over that commit's
`packages/spec/src/system/translation.zod.ts` reads 0, and 1 at
`47af3dde42`; the same for `permissions`. Under `satisfies`, an extra
key in an object literal is TS2353 "Object literal may only specify
known properties" (probed with this repo's tsc 6.0.3 on a two-key
`Record`). So the rows alone fail at the old pin, and the pin alone
fails as the gate does.

## Clause-② reading (the claim asked for it)

The claim declares `yes` because the seat could not tell whether
`metadata-admin/i18n.ts` reaches app-shell's published entry, and the
two lines above are copied from the claim. Measured on the built
declaration closure (`packages/app-shell/dist`, walking the relative
imports from `index.d.ts`, the only `exports` entry with types):

- The closure holds 182 `.d.ts` files. `views/metadata-admin/i18n.d.ts`
**is** one of them (the positive control), but its face is `t(key:
string, …)`, `tFormat(key: string, …)` and the other helpers: the key
table is not exported, and no key is a type.
- Closure files that carry the string `translationPreview`: **0**.
`previews/TranslationPreview.d.ts` is not in the closure at all.

So the diff widens no published accept set and no public face of
app-shell, and the reading is `no`. The declaration is the seat's, so
the lines stay as claimed.

## Verification (local, this worktree, at the new pin)

- `pnpm exec turbo run build --filter=@object-ui/console...
--concurrency=2` (under the verify lock, 6144 MB heap): 35 of 35 tasks,
including `@object-ui/app-shell`'s `tsc` and the console's vite build,
SDUI manifest and plugin tsc. That is the build the Console Pin Gate
runs, here at objectui's own pin.
- `pnpm exec vitest run packages/app-shell/src/views/metadata-admin/
packages/types/` (under the lock): 980 files passed; 17091 tests passed,
1 skipped.
- The three TranslationPreview test files: 63 of 63. Reverse check: with
the two rows removed from `TranslationPreview.tsx` (on-disk count 0,
back to 2 after the restore), the `categories-11765` file goes red on
its order-and-coverage case and on every per-group case, the two new
groups included.
- Gates, each exit 0: `check:i18n-keys`, `check:i18n-designer-parity`,
`check:i18n-dead-keys`, `check:i18n-drift`,
`check:installed-pin-claims`, `check:spec-symbols`,
`check:icon-record-names`, `check:control-bytes`,
`check:new-line-citations` (0 new), `check-changeset-presence`,
`check-changeset-no-major`, and `eslint` on the four changed source
files.
- `check:spec-floors`: exit 1 with the same two `floor-too-low` findings
(`@object-ui/auth` `MEMBERSHIP_ROLE_EXTERNAL`, `@object-ui/types`
`navItemVisibleDataNeedsDataSource`) that #13472 recorded as
pre-existing on main. This diff touches neither package. The gate runs
on push and schedule with `--cross-check`, which is its measurement, and
that run is green on main `cd70b78`.

NOT MEASURED locally (left to CI): the rest of the workspace `vitest`
shards, `lint` and `type-check` across all packages, and e2e.

## Acceptance notes

- objectui#13477 (the label render of the same objectstack change) is a
different defect; it remains open and nothing here touches it.

- [x] Adds no product-specific widget, dialog, hook or string (see
AGENTS.md › Product-neutral renderer)

Implemented by the ui lane card thread, session
`https://claude.ai/code/session_01Usjyy1hT9FNxZHea9u7drp`; claim on the
card, comment 6391374949.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01Usjyy1hT9FNxZHea9u7drp

---
_Generated by [Claude
Code](https://claude.ai/code/session_01Usjyy1hT9FNxZHea9u7drp)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants