fix(spec): regenerate drifted skill references + react-blocks contract, and gate them in CI#3138
Merged
Merged
Conversation
…t, and gate them in CI skills/*/references/_index.md and the objectstack-ui react-blocks contract are generated from packages/spec/src and committed, but nothing regenerated them in CI — no workflow referenced either generator, the spec `build` script does not run them, and neither had a `check:` mode. They drifted. Unlike the reference docs (#3134), these ship to third parties via `npx skills add`, so the drift is served straight to consumers' agents. An independent oracle over the committed indexes: 6 of 113 schema pointers named files that no longer exist — `automation/workflow.zod.ts` (deleted by #1398), `system/masking.zod.ts` (deleted), `data/dataset.zod.ts` (renamed to `data/seed.zod.ts` by #1620). After this change: 126 pointers, 0 broken. Regenerating alone was not enough. SKILL_MAP still named `data/dataset.zod.ts`, and the generator warned-and-skipped on the missing file, so a plain regen would have quietly dropped seed coverage from objectstack-data — whose own SKILL.md advertises `defineSeed()` authoring — and the new gate would then have locked that in as correct. Point the map at `data/seed.zod.ts` and make a dangling entry a hard failure in both modes: the map is authored config, so a dangling entry is always a bug in it, never a spec change to absorb. Gate mechanics follow build-docs.ts (#3134): every write goes through `emit()`, every wholesale-regenerated folder through `manageDir()`, and nothing touches disk until `flush()` — so write and --check share all generation logic and differ only in disposition. Extracted to scripts/lib/generated-output.ts rather than copy-pasted. `manageDir()` takes an ownership predicate because a skill's references/ folder is only partially generated: `_index.md` is ours, but `react-blocks.md` (sibling generator) and hand-written notes must survive. Placed in lint.yml's "TypeScript Type Check", not ci.yml's check-generated job. That job is paths-filtered on skills/** and build-skill-docs.ts but NOT on packages/spec/src/**, so it would skip exactly the spec-only PRs that cause this drift — and it is not a required check, so it could be merged past anyway. The type-check job has no paths filter and IS required. Both gates read src/ via tsx and need no build (verified with every workspace dist/ removed), so they run before the workspace build and fail in seconds. Each gate was proven to go red before being trusted (13/13): stale content, missing file, stale untracked leftover, dangling SKILL_MAP entry, missing skill dir, and a spec change the contract had not absorbed. The stale-leftover case is why manageDir exists — `git diff --exit-code` returns 0 on it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
os-zhuang
added
ci/cd
tooling
skip-changeset
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 102 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…lli-731802 # Conflicts: # .github/workflows/lint.yml
Contributor
Author
|
Merged I also re-checked my placement argument against the post-#3134 filter rather than the one I originally read, and updated the comment to match. #3134 renamed the job to Re-verified against the merged tree:
Follow-up from the merge, worth doing now that #3134 is in: |
This was referenced Jul 17, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 17, 2026
#3138 extracted the emit()/manageDir()/flush() sink to lib/generated-output.ts and moved build-skill-references + build-react-blocks-contract onto it, but left build-docs.ts carrying the inline original — #3126 was rewriting the same file and the migration would have conflicted. #3126 has landed, so collect the debt. Two copies of a check/write sink is the one duplication this design cannot afford: its whole claim is that --check and write are the same code, and that claim is per copy. A fix to one silently leaves the other behind, and the failure mode is a gate that passes on output a real run would not produce. Adds wasEmitted() to the shared sink. build-docs is the first caller whose later output depends on its earlier output — each category index links only the pages that got generated — and that question belongs to the sink, which already owns the emitted map. New export; the other two callers are unaffected. The empty-input guard moves to flush()'s guard hook, same meaning. Pure refactor, verified: byte-identical output across all 258 files; all five drift classes still fail (stale content, missing page, stale leftover, no-schema guard, in-sync green); both existing sink callers regress clean, including #3138's reverse test that a hand-written .md in skills/*/references/ keeps the gate green and survives a write. Net -58 lines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
os-zhuang
added a commit
that referenced
this pull request
Jul 17, 2026
PROTOCOL_MAP.md is hand-written and nothing regenerates it, so renames orphan rows silently. 25 of its 132 `src/…` links pointed at files that no longer exist. The reported row was misdiagnosed, and the git history says why. The prose "Virtual capabilities for analytics datasets" was NOT a half-finished rewrite left by #1620 — it has been there verbatim since the map was created (ced379b), when `data/dataset.zod.ts` held the *seed* schema (object/externalId/mode/records). It never described that file. So this is one wrong row plus one absent row, not one half-updated row: - `data/dataset.zod.ts` -> `data/seed.zod.ts`, described as seed fixtures (renamed by #1620 to free `dataset` for analytics, ADR-0021). - the analytics dataset the old prose was reaching for is a real, unlisted schema: added as `ui/dataset.zod.ts` (ADR-0021 semantic layer). The sweep found the same rot everywhere, all from deliberate refactors that left the map behind: - `src/hub/**` (9 rows) — the directory no longer exists (142703d). Survivors rehomed: license/registry-config/tenant -> src/system, plugin-registry/plugin-security -> src/kernel; composer, marketplace, space, hub-federation and api/hub were deleted outright. Section 9 is therefore gone and Kernel renumbered 10 -> 9. - `src/ai` (10 rows) — narrowed by 629a716. Repointed where a successor exists (agent-action -> tool, cost -> usage, rag-pipeline -> knowledge-source); dropped nlq/orchestration/predictive/devops-agent/ feedback-loop/plugin-development/runtime-ops. - `identity/role` -> `identity/position` (ADR-0090 D3 renamed sys_role). - `security/policy` (#2387), `system/compliance` + `system/masking` (ADR-0056 D8) — removed as dead, rows dropped. Gate: scripts/protocol-map.test.ts asserts every link resolves. Proven to go red first — it listed exactly the 25 dead links before the fix. It rides Test Core (required); `core: packages/**` covers the file and turbo's $TURBO_DEFAULT$ inputs include it, so a map-only edit still re-runs the test rather than cache-hitting to a false green. A new workflow job would not have blocked merges (main requires only 4 checks), and check-links.yml is dormant (workflow_dispatch only) and does not cover packages/spec. Deliberately asserts links only, not completeness: the map is a curated digest, not a generated index. Sibling of #3138, which fixed the identical stale `data/dataset.zod.ts` pointer in build-skill-references.ts's SKILL_MAP. That one guards generated artifacts that ship to third parties; PROTOCOL_MAP.md is hand-written and repo-internal (not in the package's npm `files`), so it needs its own check. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <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
1 participant
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.
Two committed generated surfaces under
skills/were drifted frompackages/specand completely unguarded by CI. Same class as #3134, but on surfaces that ship to third parties vianpx skills add objectstack-ai/framework— so the drift is served straight to consumers' agents.The drift was real, and user-facing
An independent oracle (parses every pointer out of the committed indexes, asserts the file exists — deliberately not the generator, so it isn't circular):
ce468c832The six dangling pointers handed agents paths to files that don't exist:
automation/workflow.zod.ts— deleted by refactor(automation): remove Workflow Rules; reclaimworkflowfor state machines #1398 ("remove Workflow Rules"), index never regeneratedsystem/masking.zod.ts— deleted from specdata/dataset.zod.ts— renamed todata/seed.zod.tsby fix(seed): complete Dataset->Seed rename + load published seed rows (locally verified) #1620Every change in the regenerated diff traces to a spec change; I checked each one. The
encryption.zod.tsdrops are becausefield.zod.tsno longer imports it; themetadata-protection/protection/visibility/public-auth-featuresadditions are new spec imports (ADR-0010, ADR-0089, #2874); theflow.zod.tsandmanifest.zod.tsdescription changes are edited JSDoc. Output is byte-identical across runs.Regenerating alone would have shipped a regression
SKILL_MAPstill nameddata/dataset.zod.ts, and the generator warned and skipped the missing file. A plain regen would therefore have quietly dropped seed coverage fromobjectstack-data— whose own SKILL.md advertisesdefineSeed()authoring — and the new gate would have locked that in as "correct".So: point the map at
data/seed.zod.ts, and make a dangling entry a hard failure in both modes. The map is authored config; a dangling entry is always a bug in it, never a spec change to absorb. That warn-and-continue is the same false-green vector the gate exists to close.Gate mechanics
Follows #3134's
build-docs.ts: every write goes throughemit(), every wholesale-regenerated folder throughmanageDir(), nothing touches disk untilflush()— so write and--checkshare all generation logic and differ only in disposition. Extracted toscripts/lib/generated-output.tsrather than copy-pasted a third time.manageDir()takes an ownership predicate, because a skill'sreferences/folder is only partially generated:_index.mdis ours, butreact-blocks.md(sibling generator) and hand-written notes must survive. Verified they do.New scripts:
check:skill-refs,check:react-blocks.Proven red before being trusted (13/13)
A gate that cannot fail is worse than no gate. Each was reproduced going red, then green again after restore:
~ out of date+ missing.zod.ts)- stalegit diff --exit-codemanageDirexistsSKILL_MAPentry (the bug that shipped)~ out of datez.toJSONSchemaerror)That last guard is why it checks data-kind props rather than
props.length:mergePropsputs the hand-authored overlay first, so a block whose schema failed to load still reports props. The data props are what vanish.Placement: lint.yml, deliberately
In lint.yml's "TypeScript Type Check" (no paths filter + required), not ci.yml's
check-generated. That job is filter-gated on:— not
packages/spec/src/**. It would skip exactly the spec-only PRs that cause this drift (the #3076 failure mode), and it isn't a required check, so it could be merged past even when it does run. Required checks confirmed as["TypeScript Type Check","Build Core","Test Core","Dogfood Regression Gate"].Both gates need no
^build— verified empirically by removing all 23 workspacedist/dirs and re-running (turbo's^buildongen:skill-refsis vestigial;pnpm --filterbypasses turbo anyway). So they run before the workspace build and fail in seconds.Note on #3134
#3134 is open and touches
lint.yml+packages/spec/package.jsontoo, insertingcheck:docsin the same step gap. Expect a trivial adjacent-block conflict; both blocks coexist. I based onmainrather than on that branch so this stands alone. Follow-up once it lands: migratebuild-docs.tsontoscripts/lib/generated-output.ts.check:react-conformancewas checked for overlap — there is none. It compares spec schemas against the objectui frontend manifest (opt-in viaMANIFEST=, exits 0 without it) and never checks generator output against committed files.Verification
check:skill-refs/check:react-blocksgreen on a clean tree; 13/13 red-proofs passtsc --noEmitclean · spec tests 254 files / 6890 passed🤖 Generated with Claude Code