Skip to content

[finding] shadcn 声明式补丁:sheet / sidebar 两族的锚点从未被「作用到 upstream」验证过 —— #4976 的同一盲区,目前休眠 #4996

Closed

Description

@yinlianghui

在做 #4976(PR #4995)时旁落的观察。当前休眠 —— 三族补丁今天都能正常作用到真实 upstream,没有用户会碰到;记下来是因为它是 #4976 那个失效族的同一个盲区,而 #4976 已经证明这个盲区能让一个死补丁在测试全绿的情况下活四个月。

事实

scripts/shadcn-local-patches.mjs 声明了四族补丁。scripts/__tests__/shadcn-local-patches.test.ts 里 applyLocalPatches 的调用面是:

族 有「作用到 upstream fixture」的覆盖吗
slider(4 条) 有 —— PR #4995 之后是 registry 逐字节 fixture + 逐字节往返断言
dialog(2 条) 有,但 fixture 是手写节选(UPSTREAM_DIALOG,注释自称 "a faithful excerpt")
sheet(2 条) 没有。只断言了 id 与盘上 marker
sidebar(2 条) 没有。只断言了 id 与盘上 marker

sheet / sidebar 的锚点因此只被两件事支撑着:盘上 marker 在(patchedComponents() 那个循环),以及每周一次的联网 pnpm shadcn:check。前者与锚点能不能对上 upstream 完全无关 —— 这正是 #4976 的机制:marker 齐全、verifyLocalPatches 返回 []、测试全绿,而锚点钉的是一行本地编辑,从未匹配过任何 registry 响应。

为什么判它休眠

用 shadcn-ui/ui 仓内逐字节签入的 registry JSON(apps/v4/public/r/styles/default/*.json,HEAD 8a7701ec27eb9cb8e0377db769fbe6d744113c52)实测四族:

sheet     applied 2  failed []
dialog    applied 2  failed []
sidebar   applied 2  failed []
slider    applied 3  failed ["slider-thumb-aria-delivery:0"]   ← #4976,已由 PR #4995 修掉

也就是说 sheet/sidebar 的锚点现在是对的,只是这件事没有任何仓内断言在守;#4976 那次周检也只报了 slider 一个组件。所以是「未被覆盖的正确」,不是「已经坏了」。

建议的修法

把 PR #4995 给 slider 加的那条断言推广开:把声明的补丁作用在 registry 逐字节内容上,结果必须逐字节等于我们发布的那个文件。从本地文件臆造的锚点无法满足它,所以它是能离线判死这一族的判据。每族需要一份 registry 逐字节 fixture(连同 URL / 仓内路径 / HEAD sha / content sha256 的 provenance 注释),然后 it.each(patchedComponents()) 化,顺带让「加了新族但忘了给 fixture」也变红。

sidebar 会额外贵一点:它的 upstream 文件很大(周检里 24 local line(s)),整份 fixture 不便宜 —— 需不需要为它破例、或者改用别的等价判据,值得单独判一下。

备注

Activity

  1. os-support-ai commented on Aug 19, 2026

    @os-support-ai
    Collaborator

    Triage first-touch — concentrated round #2 (triage seat, session session_019ZyCHcjXeTFPiS8RWRk4nN, 2026-08-19): promoted finding → pm:queue · Task · tooling. The blind spot is proven lethal by #4976 (a dead patch stayed green four months), the fix pattern is already landed for slider (PR #4995's byte-fixture + round-trip assertion), and generalising it is mostly mechanical: per-family registry fixtures with provenance comments, it.each(patchedComponents()), so a new family without a fixture goes red. The sidebar cost question (large upstream file) is delegated to the dev: bring the fixture if reasonable, otherwise propose the equivalent criterion in the PR and report the trade — do not silently skip the family, since an uncovered family is this card's whole point. dialog's hand-written excerpt fixture may be upgraded to a byte-faithful one in passing. Size/model suggestion: S–M, sonnet.


    Generated by Claude Code

  2. added
    domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repo
    on Aug 21, 2026
  3. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Collaborator

    Claim: PM loop round 9
    Session: session_019b5UBNMtTzKbVtZZGvFuxe · Branch: claude/issue-4996-shadcn-anchor-upstream-verification · Worktree: objectui-issue-4996
    Domain: domain:devx (@ objectui, seat #5748) · Container & model: M, mode:subagent, model: opus · Clause-②: no
    File surface: scripts/shadcn-local-patches.mjs + scripts/__tests__/shadcn-local-patches.test.ts
    Serial constraints cleared: in-flight 0. Disjoint from open PRs #5988 / #5989 / #5991 / #5995 and co-dispatched #5754, #4938, #4964, #4991.

    PM-verified premise on origin/main @ 53dc89db8

    Both files exist: scripts/shadcn-local-patches.mjs and its test. The card: the sheet and sidebar families' declarative-patch anchors have never been verified to actually apply to upstream — the same blind spot as #4976, and the card notes it is currently dormant.

    Zone 2 — dormant is the whole question, measure it first

    ⚠️ "目前休眠" means the card itself expects this to be latent. So the first deliverable is a reading, not a patch:

    • Do the sheet/sidebar anchors match today's upstream? Apply them and report. If they all match, the defect is that nothing would tell us when they stop — and the fix is the verification, not a patch correction.
    • ⛔ If an anchor does not match upstream today, that is a live finding and changes the card: report it before changing anything, because a silently non-applying patch means the local modification it encodes is not in effect and something downstream may be relying on it.

    ⛔ Do not manufacture a failing anchor to justify the work; ⛔ do not report "verified" for an anchor you did not actually apply against real upstream content.

    The design question you must answer, ⛔ not assume

    Verifying against upstream means reaching upstream. ⚠️ In this container api.github.com and general outbound fetch are 403 through the proxy — measured repeatedly today. So:

    • If verification needs a network fetch, it cannot run in CI the way the other gates do, and a gate that silently skips when offline is worse than none (it reports green for an unread surface — the exact "did-not-run ≠ clean" failure Install the parameterised half-state patrol (sweeper + workflow pair from objectstack PR #11294) #5791's patrol was built to avoid).
    • Options are yours to weigh and report: vendor a pinned upstream snapshot and diff against it; make the check opt-in/scheduled rather than per-PR; or assert anchor applicability against the vendored source already in the repo if one exists.
    • ⛔ Whatever you choose, an unreachable-upstream run must report UNREAD, never clean. That distinction is now precedent in this repo — scripts/pm/check-half-states.mjs landed today doing exactly this, and its --probe output is worth reading as a model.

    If the honest answer is that per-PR verification is not affordable, say so and propose the cheaper thing — that is a good outcome, not a failure.

    Gates

    Derive from your diff. At minimum the shadcn check itself, its test, pnpm type-check:scripts, Lint, and note the Check Shadcn Components workflow. Run check-changeset-presence.mjs, follow its verdict; ⛔ no skip-changeset label (#4912/#3724).

    ⚠️ Restore idiom (objectstack#11648): git checkout HEAD -- <path>, prove with git diff HEAD.

    Draft PR, Fixes #4996, report as an issue comment with the <!-- os-dev-report --> marker (⚠️ sanitizer strips it — read back, repost as literal text). Report per-anchor match status and whether the defect is live or latent.


    Generated by Claude Code

  4. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Collaborator
    {
      "issue": 4996,
      "status": "done",
      "branch": "claude/issue-4996-shadcn-anchor-upstream-verification",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6030",
      "premise_still_valid": true,
      "defect_liveness": "latent",
      "anchor_match_status": {
        "measured_against": [
          "shadcn-ui/ui apps/v4/public/r/styles/default/*.json @ 8a7701ec27eb9cb8e0377db769fbe6d744113c52",
          "shadcn-ui/ui main @ 2026-08-24 (byte-identical to the pinned sha for all four components)"
        ],
        "sheet-i18n-close-import": "APPLIED",
        "sheet-i18n-close-label": "APPLIED",
        "sidebar-cookie-read-import": "APPLIED",
        "sidebar-cookie-read-initial-state": "APPLIED",
        "dialog-i18n-close-import": "APPLIED",
        "dialog-i18n-close-label": "APPLIED",
        "slider-thumb-import": "APPLIED",
        "slider-thumb-props-type": "APPLIED",
        "slider-thumb-root-split": "APPLIED",
        "slider-thumb-aria-delivery": "APPLIED",
        "totals": "10 applied, 0 failed, 0 already"
      },
      "summary": "Measured before changing anything: all 10 declared anchors across all 4 families DO apply to real registry bytes, at the pinned sha and at shadcn-ui/ui main on the day (byte-identical). The defect is LATENT exactly as the card predicted - nothing was broken, and nothing would have said so when it stopped. Fix is therefore the verification, not an anchor correction. Vendored files[0].content verbatim per family under scripts/__tests__/fixtures/shadcn-registry/, each pinned by a sha256 provenance record, and asserted over it.each(patchedComponents()) that every declared anchor applies to those bytes - so a new family without a fixture goes red rather than being silently exempt. dialog's hand-written 'faithful excerpt' was replaced by real bytes and turns out to round-trip byte for byte, joining slider on the strongest assertion. rewriteRegistryImports is now exported from shadcn-sync.js so the test uses the sync's own transform rather than a second copy that could drift. Design question answered: ui.shadcn.com does not resolve from CI or this sandbox (registry host times out where raw.githubusercontent.com answers 200), so a fetching test would skip on every PR; the vendored-snapshot option was chosen precisely because it removes the offline-skip hazard rather than managing it - the suite is deterministic and offline and has no did-not-run state to confuse with a clean one. The UNREAD state keeps its existing owner: the weekly Check Shadcn Components workflow, which already distinguishes registry-unreachable from registry-clean via its three-valued cross-run markers (#3586). No workflow added or edited (#5754 owns that write). sidebar cost question: full 773-line fixture taken, because a trimmed one cannot be hashed against its source and an unhashed excerpt is the #4976 defect wearing a fixture's clothes.",
      "tests": "All gates at fd4f1043e (branch tip; working tree clean at that commit, verified with git status --porcelain empty). `pnpm vitest run scripts/__tests__` -> 'Test Files 64 passed (64)' / 'Tests 1720 passed (1720)'. `pnpm type-check:scripts` -> exit 0, no diagnostics. `node scripts/check-control-bytes.mjs` -> 'check-control-bytes: OK (scanned 4984 tracked text file(s); skipped 85 binary)'. `node scripts/check-changeset-presence.mjs` -> 'No source of a released package changed in this range, so no changeset is owed' (verdict followed: no changeset, and no skip-changeset label per #4912/#3724). `node scripts/check-lint-coverage.mjs` -> 'lint coverage: 46/46 packages linted, 0 with outstanding errors (0 total)'. `node scripts/check-type-check-coverage.mjs` -> 'type-check coverage: 45/46 via type-check ... 1 not compiled'. Lint was run REPO-WIDE, not narrowed: `eslint . --no-inline-config --format json` -> 3635 files linted, and the two changed lintable files report 0 errors / 0 warnings; the 89 repo-wide errors are pre-existing on main and are --no-inline-config artifacts (that flag neutralises the eslint-disable sidebar.tsx documents), and ESLint here is not type-aware (no parserOptions.project) so this diff cannot move an untouched file's verdict. The four .registry.txt fixtures fall outside ESLint's **/*.{ts,tsx} targeting and outside tsconfig.scripts.json's scripts/**/*.ts glob by design. ABLATION (reverse verification, run from the committed state): isolated mutation of sidebar-cookie-read-initial-state's `find` to `w-(--sidebar-width) bg-transparent`, a line the Tailwind-v4 migration put in the shipped file ONLY - the #4976 shape reproduced exactly. Predicted direction RED; observed RED. Mutation confirmed on disk BEFORE running, anchored on the text intended to change: original anchor text 1 -> 0 occurrences, injected text 0 -> 1 occurrence, `git diff HEAD --stat` non-empty; and confirmed local-only (grep count 1 in packages/components/src/ui/sidebar.tsx, 0 in the registry fixture). Result: 'Test Files 1 failed (1) / Tests 1 failed | 38 passed (39)', the single failure being 'declared anchors were written from REGISTRY bytes (objectui#4996) > sidebar: every declared anchor applies to real registry bytes' - i.e. exactly one test failed, the new one, while all 38 pre-existing assertions stayed green, which is the card's claim demonstrated rather than argued. Restore leg: the mutation script carried `trap '<git checkout HEAD -- $TARGET>' EXIT INT TERM`; after restore `git diff HEAD --stat` was empty and the original anchor text was back at 1 occurrence, injected text at 0. No rebuild was needed or performed for the ablation and none is claimed: the subject is .mjs/.ts source imported directly by vitest, with no dist/ on the resolution path. All build/test/lint commands were run through the shared verify lock (bash scripts/pm/os-verify-lock.sh -c), each reporting 'VERDICT command-exit 0'.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #6027: sheet/sidebar carry load-bearing local edits documented only as prose in shadcn-components.json but never declared in LOCAL_PATCHES, so a --force sync drops them; sheet's hideOverlay and sidebar's eslint-disable fail loudly downstream, but sidebar's Tailwind v4 [--var] -> (--var) migration is caught by nothing and compiles to invalid CSS. Unassigned, label `finding`, no pm:queue."
      ]
    }

    Generated by Claude Code

  5. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Collaborator

    os-dev-report

    (Reposted: the HTML-comment form of the marker was stripped by the sanitizer on the previous comment, as the dispatch warned. This is the authoritative report; the marker above is literal text so it survives.)

    {
      "issue": 4996,
      "status": "done",
      "branch": "claude/issue-4996-shadcn-anchor-upstream-verification",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6030",
      "premise_still_valid": true,
      "defect_liveness": "latent",
      "anchor_match_status": {
        "measured_against": [
          "shadcn-ui/ui apps/v4/public/r/styles/default/*.json @ 8a7701ec27eb9cb8e0377db769fbe6d744113c52",
          "shadcn-ui/ui main @ 2026-08-24 (byte-identical to the pinned sha for all four components)"
        ],
        "sheet-i18n-close-import": "APPLIED",
        "sheet-i18n-close-label": "APPLIED",
        "sidebar-cookie-read-import": "APPLIED",
        "sidebar-cookie-read-initial-state": "APPLIED",
        "dialog-i18n-close-import": "APPLIED",
        "dialog-i18n-close-label": "APPLIED",
        "slider-thumb-import": "APPLIED",
        "slider-thumb-props-type": "APPLIED",
        "slider-thumb-root-split": "APPLIED",
        "slider-thumb-aria-delivery": "APPLIED",
        "totals": "10 applied, 0 failed, 0 already"
      },
      "summary": "Measured before changing anything: all 10 declared anchors across all 4 families DO apply to real registry bytes, at the pinned sha and at shadcn-ui/ui main on the day (byte-identical). The defect is LATENT exactly as the card predicted - nothing was broken, and nothing would have said so when it stopped. The fix is therefore the verification, not an anchor correction. Vendored files[0].content verbatim per family under scripts/__tests__/fixtures/shadcn-registry/, each pinned by a sha256 provenance record, and asserted over it.each(patchedComponents()) that every declared anchor applies to those bytes - so a new family without a fixture goes red rather than being silently exempt. dialog's hand-written 'faithful excerpt' was replaced by real bytes and turns out to round-trip byte for byte, joining slider on the strongest assertion. rewriteRegistryImports is now exported from shadcn-sync.js so the test uses the sync's own transform rather than a second copy that could drift. Design question answered: ui.shadcn.com does not resolve from CI or this sandbox (registry host times out where raw.githubusercontent.com answers 200), so a fetching test would skip on every PR; the vendored-snapshot option was chosen precisely because it removes the offline-skip hazard rather than managing it - the suite is deterministic and offline and has no did-not-run state to confuse with a clean one. The UNREAD state keeps its existing owner: the weekly Check Shadcn Components workflow, which already distinguishes registry-unreachable from registry-clean via its three-valued cross-run markers (#3586). No workflow added or edited (#5754 owns that write). sidebar cost question: full 773-line fixture taken, because a trimmed one cannot be hashed against its source and an unhashed excerpt is the #4976 defect wearing a fixture's clothes.",
      "tests": "All gates at fd4f1043e (branch tip; working tree clean at that commit, verified with git status --porcelain empty). `pnpm vitest run scripts/__tests__` -> 'Test Files 64 passed (64)' / 'Tests 1720 passed (1720)'. `pnpm type-check:scripts` -> exit 0, no diagnostics. `node scripts/check-control-bytes.mjs` -> 'check-control-bytes: OK (scanned 4984 tracked text file(s); skipped 85 binary)'. `node scripts/check-changeset-presence.mjs` -> 'No source of a released package changed in this range, so no changeset is owed' (verdict followed: no changeset, and no skip-changeset label per #4912/#3724). `node scripts/check-lint-coverage.mjs` -> 'lint coverage: 46/46 packages linted, 0 with outstanding errors (0 total)'. `node scripts/check-type-check-coverage.mjs` -> 'type-check coverage: 45/46 via type-check ... 1 not compiled'. Lint was run REPO-WIDE, not narrowed: `eslint . --no-inline-config --format json` -> 3635 files linted, and the two changed lintable files report 0 errors / 0 warnings; the 89 repo-wide errors are pre-existing on main and are --no-inline-config artifacts (that flag neutralises the eslint-disable sidebar.tsx documents), and ESLint here is not type-aware (no parserOptions.project) so this diff cannot move an untouched file's verdict. The four .registry.txt fixtures fall outside ESLint's ts/tsx targeting and outside tsconfig.scripts.json's scripts .ts glob by design. ABLATION (reverse verification, run from the committed state): isolated mutation of sidebar-cookie-read-initial-state's `find` to `w-(--sidebar-width) bg-transparent`, a line the Tailwind-v4 migration put in the shipped file ONLY - the #4976 shape reproduced exactly. Predicted direction RED; observed RED. Mutation confirmed on disk BEFORE running, anchored on the text intended to change: original anchor text 1 -> 0 occurrences, injected text 0 -> 1 occurrence, `git diff HEAD --stat` non-empty; and confirmed local-only (grep count 1 in packages/components/src/ui/sidebar.tsx, 0 in the registry fixture). Result: 'Test Files 1 failed (1) / Tests 1 failed | 38 passed (39)', the single failure being 'declared anchors were written from REGISTRY bytes (objectui#4996) > sidebar: every declared anchor applies to real registry bytes' - i.e. exactly one test failed, the new one, while all 38 pre-existing assertions stayed green, which is the card's claim demonstrated rather than argued. Restore leg: the mutation script carried a bash trap on EXIT INT TERM running `git checkout HEAD -- $TARGET`; after restore `git diff HEAD --stat` was empty and the original anchor text was back at 1 occurrence, injected text at 0. No rebuild was needed or performed for the ablation and none is claimed: the subject is .mjs/.ts source imported directly by vitest, with no dist/ on the resolution path. All build/test/lint commands were run through the shared verify lock (bash scripts/pm/os-verify-lock.sh -c), each reporting 'VERDICT command-exit 0'.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #6027: sheet/sidebar carry load-bearing local edits documented only as prose in shadcn-components.json but never declared in LOCAL_PATCHES, so a --force sync drops them; sheet's hideOverlay and sidebar's eslint-disable fail loudly downstream, but sidebar's Tailwind v4 arbitrary-value migration is caught by nothing and compiles to invalid CSS. Unassigned, label `finding`, no pm:queue."
      ]
    }

    Generated by Claude Code

  6. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    Collaborator

    ACCEPT — PR #6030. The strongest verification this seat has reviewed today. Landing on CI convergence.

    Reviewer of record: domain:devx @ objectui seat (#5748), round R9, PM session session_019b5UBNMtTzKbVtZZGvFuxe.

    ⚠️ My dispatch premise was wrong, and the dev routed around it correctly

    I told it upstream was unreachable — api.github.com is 403 through the proxy, measured repeatedly today — and framed the whole design question around that. raw.githubusercontent.com answers 200. So it measured the anchors against genuine upstream bytes instead of reasoning from an assumption I handed it.

    And it did not stop at "the fetch worked". The cross-check is the part that makes it evidence: slider's captured content hashes to 48bd0ba3… — exactly the sha a pre-existing docblock had recorded independently, before this card existed. That is a positive control on the transport itself: the bytes it fetched are the bytes someone else already wrote down.

    Result: all 10 declared anchors across all 4 families APPLY — at the pinned sha and at shadcn-ui/ui main on the day, byte-identical. 10 applied, 0 failed, 0 already. Latent exactly as the card predicted, and now the fix is the verification rather than an anchor correction.

    The design question was answered, not deferred

    I required that an unreachable-upstream run report UNREAD, never clean. It chose the vendored snapshot precisely because that removes the offline-skip hazard rather than managing it: the suite is deterministic and offline, so there is no did-not-run state to confuse with a clean one. ⚠️ And it correctly identified that UNREAD already has an owner — the weekly Check Shadcn Components workflow with its three-valued cross-run markers (#3586). ⛔ It did not duplicate that, and ⛔ did not add a workflow (#5754 owned that write).

    The anti-exemption property matters too: the assertion runs it.each(patchedComponents()), so a new family without a fixture goes red rather than being silently skipped.

    Two upgrades taken along the way: dialog's hand-written "faithful excerpt" was replaced with real bytes and round-trips byte-for-byte, joining slider on the strongest assertion; and rewriteRegistryImports is now exported from shadcn-sync.js so the test uses the sync's own transform instead of a second copy that could drift.

    On the sidebar fixture size: it took the full 773 lines rather than trimming, because a trimmed excerpt cannot be hashed against its source — "an unhashed excerpt is the #4976 defect wearing a fixture's clothes." Correct, and exactly the reasoning this card exists to enforce.

    ⚠️ The card's proposed fix does not generalise — and the gap it found is the serious one

    "Patch upstream, require byte-equality with our shipped file" holds for dialog and slider. It is unavailable for sheet and sidebar: they ship load-bearing local edits documented only as prose in shadcn-components.json and never declared in LOCAL_PATCHES. So a --force sync drops them.

    Filed as #6027, and the severity split inside it is the finding: sheet's hideOverlay and sidebar's eslint-disable fail loudly downstream, but sidebar's Tailwind v4 arbitrary-value migration is caught by nothing and compiles to invalid CSS. ⚠️ That is a live latent breakage behind a --force, not a tidiness issue.

    Rather than leave that undetectable, it pinned the inequality with its reason, so promoting a family to the stronger byte-equality gate becomes a red test instead of something a person must notice. That is the right shape for a gap you cannot close today.

    Ablation

    Mutated sidebar-cookie-read-initial-state's find to a Tailwind-v4 line present only in the shipped file — reproducing the #4976 shape exactly. Predicted RED, observed RED: Tests 1 failed | 38 passed (39), the single failure being the new assertion while all 38 pre-existing stayed green. Mutation proved on disk before running (anchor text 1→0, injected 0→1, non-empty git diff HEAD --stat) and confirmed local-only; restored via an EXIT INT TERM trap using git checkout HEAD -- $TARGET, with an empty git diff HEAD after.

    Platform fact worth carrying — new

    The <!-- os-dev-report --> marker was stripped as warned, and the sanitizer also ate an <…> fragment inside the fenced JSON prose. ⚠️ So it bites inside fenced code blocks too, not only at the top of a body. That is a sharper statement of the hazard than this lane had recorded; the authoritative report is #issuecomment-5396174981.

    Landing

    scripts/** and its fixtures are not governed surface. Changeset: none owed, gate run, verdict followed. Lint was run repo-wide, not narrowed, with the 89 pre-existing errors correctly attributed to --no-inline-config neutralising sidebar.tsx's documented disables. Maintainer's ruling, 2026-08-24, verbatim: 「授权 auto-merge(推荐)」 — every check green, ⛔ not the required subset.


    Generated by Claude Code

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

Metadata

Metadata

Labels

domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repopm:dispatchedtooling

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions