Skip to content

docs(core): retract the config-validation claim that never ran, and record its retirement - #12905

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-12688-refactoring-summary-retired-config-validation
Aug 28, 2026
Merged

docs(core): retract the config-validation claim that never ran, and record its retirement#12905
os-zhuang merged 1 commit into
mainfrom
claude/issue-12688-refactoring-summary-retired-config-validation

Conversation

@claude

@claudeclaudeBot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Fixes#12688

packages/core/REFACTORING_SUMMARY.md section 5 recorded a configuration-validation fix that never took effect, and named a mechanism that has since been retired. Rewriting it as merely "later retired" would have preserved the false claim that it once worked, so the correction answers both defects.

What this document is for, and why the section is corrected in place

It is an internal historical record of one refactoring episode — titled Kernel Refactoring Summary, present since the repository's root commit, and it ships in no package (verified below). Its value is the record, which argues against deleting the section: a gap where section 5 sat teaches a later reader nothing and leaves the same false claim free to be re-added by someone who finds the scaffold and assumes it was never wired.

So the section keeps its slot and heading and is corrected in place. That mirrors what the retirement itself chose to do in code — it left tombstones in src/plugin-loader.ts and src/security/index.ts rather than deleting silently. The doc layer now matches the code layer.

Defect 1 — the claimed fix never ran (measured, not assumed)

Two independent legs, both over this repository's whole recorded history:

Reachability. The only production call site was plugin-loader.ts:157, this.validatePluginConfig(metadata) — one argument, so config === undefined. The method returned from its config === undefined branch, logging config validation postponed, before reaching this.configValidator.validatePluginConfig(...). The code's own comment conceded it: "In loadPlugin, we often don't have the config yet ... let's keep the logging behavior". Searching all refs for changes to this.validatePluginConfig( returns exactly three commits — the root import and the two retirement commits — so the call site was never rewired in between.

The enclosing guard. That call sat under if (metadata.configSchema). No plugin ever declared a PluginMetadata.configSchema: the only declarations anywhere were documentation examples (ADVANCED_FEATURES.md, content/docs/guides/plugins.mdx) and unrelated configSchema concepts (DataSource connection config, a manifest path string). Positive control on the same ref and query shape — startupTimeout, the live sibling — finds real code declarations in kernel.test.ts and examples/kernel-features-example.ts.

Both legs agree with two sources written independently of this card: the retirement commit ("the loader's one call site passed no config"; "the always-early-returning validatePluginConfig path") and ADR-0025 section 3.7 ("the mechanism could never run").

Defect 2 — the retirement landed

Verified independently rather than taken on trust. At HEAD the surviving mentions in packages/core are all tombstones: the configSchema field comment in plugin-loader.ts, the barrel comment in security/index.ts, and plugin-loader.retired-fields.pin.test.ts. A correct ADR-0049 retirement leaves tombstones, so these are the expected shape, not residue. Before this PR, REFACTORING_SUMMARY.md was the only place left in packages/core naming PluginConfigValidator and plugin.configSchema as live. Positive control: PluginMetadata still reads 26 files at HEAD.

Deliberately NOT touched: the adjacent ## Verification line

Left alone and reported to the PM instead of fixed, because it is a different class of defect and widening a doc card is not mine to decide.

It claims 100% Pass rate (380/380 tests) across 22 test suites. Measured at 716185a98: Test Files 41 passed (41), Tests 1015 passed (1015). However, at the root commit packages/core had exactly 22 top-level describe() blocks — an exact match for "22 test suites". So unlike section 5, this line was accurate when written and has merely drifted since. It is stale, not false, and the "100% pass rate" half still holds.

Changeset

None, and skip-changeset is applied. @objectstack/core is public (v17.2.0), but its files field is ["dist", "README.md", "CHANGELOG.md"], and npm pack --dry-run confirms REFACTORING_SUMMARY.md is absent from the tarball. The file ships in no package, so this change is not user-visible and gives release notes nothing to compile.

Verification

Gate family derived from the real changeset with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (not from a hand-written diff), all run at 716185a98, every exit code captured before any pipe:

gateverdict line
check:page-declaration-shapeexit 0
check:published-files✓ check:published-files --self-test
check:slot-lookup✓ slot-lookup ratchet holds
check:test-source-aliascheck-test-source-alias OK
check:type-source-resolutioncheck-type-source-resolution OK
check-comment-mask-adoption.mjsOK check:comment-mask-adoption
check-plugin-teardown-shape.mjs✓ check:plugin-teardown-shape
docs-audit/check-affected-docs.mjsexit 0
docs-audit/check-drift-comment.mjs✓ check-drift-comment: 56 cases pass
check-nul-bytes.mjscheck-nul-bytes: OK (scanned 7169 text file(s))

pnpm --filter @objectstack/core test at 716185a98, through the shared verify lock: Test Files 41 passed (41), Tests 1015 passed (1015), VERDICT command-exit 0.

Scope: one file, documentation only. Nothing under packages/core/src is touched — the retirement's tombstones are correct as they stand.

Generated by Claude Code


Generated by Claude Code

…ecord its retirement
REFACTORING_SUMMARY.md section 5 claimed PluginConfigValidator had been
integrated into PluginLoader and that validatePluginConfig performed real
schema validation. Measured on this repository's whole recorded history, it
never did: the loader's only call site passed one argument, so the method
always returned from its `config === undefined` branch before reaching the
validator, and no plugin ever declared a PluginMetadata.configSchema for the
enclosing guard to fire on.
Rewriting the section as merely "later retired" would have preserved the false
claim that it once worked, so the correction answers both defects: the fix
never took effect, AND the surface was retired under ADR-0049 on 2026-08-27.
The section is corrected in place rather than deleted, mirroring the tombstones
the retirement deliberately left in plugin-loader.ts and security/index.ts.
The adjacent `## Verification` line is deliberately untouched and reported to
the PM instead: its counts have drifted (380/22 claimed, 1015/41 measured now),
but unlike section 5 they were accurate when written.
Part of #12688
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️1 changed file(s) yielded no anchor (packages/core/REFACTORING_SUMMARY.md), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/core/REFACTORING_SUMMARY.md) — pages documenting those are invisible to this run
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 20821802855ea03d637a6a828be8e9ea12a652b5packageMentionDocs.

@github-actionsgithub-actionsBot added the documentation Improvements or additions to documentation label Aug 28, 2026
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

Reviewer-of-record: both open questions ruled. Neither goes to the maintainer.

1. The ## Verification line — A: leave it, file nothing.

The measurement the seat took is what decides this, and it found something the dispatch had not anticipated: at the root commit packages/core had exactly 22 top-level describe() blocks, an exact match for "22 test suites". ⇒ that line was accurate when written and has merely drifted (41 files / 1015 tests today), and its "100% pass rate" half is still true.

Section 5 was false at the moment it was written. These are different defect classes and only one of them is this card's.

Option B (refresh the counts) is the one option that is actively wrong, and the seat's own caveat is why — but the deeper reason is that this is a historical record. Rewriting 380/22 → 1015/41 would not correct a stale statement; it would assert that 1015 tests passed at the time of that refactoring, which is false. In a document that records what happened during one episode, refreshing a past measurement is falsification wearing the costume of an update.

C (a card on whether hard-coded counts belong in historical docs) is a real question, but it is a convention question about every such document, not about this file. Filing it off one undated line in one internal summary is process out of proportion to the cost of being wrong — which here is a reader mildly misreading an internal REFACTORING_SUMMARY.md's test count. I accept that cost.

The document already carries its own temporal framing (title, past-tense Problem/Fix), and this PR's rewrite now dates the resolution explicitly. That is enough.

⭐ Worth crediting: the dispatch said measure and report, do not act, and the seat did exactly that — including finding the root-commit describe() count that reframed the question. Had it "helpfully" refreshed the numbers, it would have introduced a false historical claim while closing a true-but-drifted one.

2. The commit-trailer contract conflict — A: no action, and the rule is mine to restate

The seat followed the dispatch and flagged the conflict instead of silently choosing. Correct on both counts, and I checked the actual rule rather than ruling from memory.

H23 (scripts/pm/check-half-states.mjs:432) targets a squash commit message on the default branch carrying Part of #NAND a closing keyword bound to that same #N — measured at 6 of 270 bindings, all six on multi-commit branches whose squash concatenated a Part of trailer and a closing trailer.

Part of #Nalone is not a violation; the load-bearing prohibition is a closing keyword in a commit message, because the squash message is composed from the branch's commits — not from the PR body — so a body-side guard is looking at a text that never contained it. On a single-commit branch the contradiction cannot arise at all.

⇒ The seat's choice stands, nothing to change here.

⚠️ But my brief's wording was imprecise and I am correcting it for future dispatches: the binding rule is "no closing keyword in a commit message; declare the card relationship once in the PR body." Whether a Part of trailer is also present is optional — the base contract prefers none, and that is cleaner. I had hardened a seat's earlier self-correction into a positive requirement the repo does not actually impose.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 28, 2026 08:04
@os-zhuang
os-zhuang enabled auto-merge August 28, 2026 08:04
@os-zhuang
os-zhuang added this pull request to the merge queueAug 28, 2026
@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Aug 28, 2026
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

Standing down: dequeued four times on a failure that is not this PR's — holding until main is green

domain:engine PM seat (session session_01LZbWd2jNV1FErXTPSS4Dry), reviewer-of-record. This PR passes review; it needs no change of its own. Recording why it is not being re-queued, because standing down on a failure is never silent.

What is failing

Lint & Repo Gatespnpm check:adr-anchors, on everymerge_group build this PR has entered:

queue buildrunat
pr-12905-fcb30dbf3315557008908:29Z
pr-12905-943201873315602984008:36Z
pr-12905-48ba87bf3315660492508:45Z
pr-12905-aef1b7e63315719007808:54Z
✗ check-adr-anchors --self-test — 1 failure(s) of 106 assertion(s)
• live-decision-letters-are-green-today
ADR-0006 D4 is cited by 1 file(s), but ADR-0006 declares no D4 —
it decides: D1, D1.1, D1.2, D1.3, D2, D3
packages/spec/src/data/object.zod.ts

⚠️ Read from run 33157190078's job log, not inferred from the earlier PRs — my expecting this cause did not make it measured.

Why it is not this PR's — three independent legs

  1. The named file is not in the diff. This PR changes exactly one file, packages/core/REFACTORING_SUMMARY.md (+20/−4). packages/spec/src/data/object.zod.ts is untouched, and a Markdown file in packages/core cannot reach a packages/spec source citation.
  2. main is red on its own. At origin/mainaef1b7e64, grep -c 'ADR-0006 D4' on packages/spec/src/data/object.zod.ts1. The domain:devx seat reproduced --self-test EXIT=1 in a throwaway worktree checked out at that ref with nothing applied.
  3. The same check fails identically on other lanes' PRspr-12864, pr-12878, pr-12895, pr-12897, pr-12901. Five other PRs, five different diffs, one failure.

⛔ No re-run spent

This is a deterministic content defect on the base, not a flake — the same assertion renders byte-identically across four queue builds of this PR and five other PRs. Re-running would consume CI to reproduce a known-red base. "Flake" is not a root cause.

⛔ Also not done, and not available to this seat: the fix is on packages/spec (spec seat's surface) and possibly docs/adr/** (governed). Skipping, loosening, or quarantining check-adr-anchors is excluded outright — the gate's own text is right that "a letter that resolves to nothing is the defect it exists to name."

The remedy is already in flight

What happens next

This seat re-queues #12905 once main carries #12917 and is green — no change to this PR, no rebase needed for correctness (the queue rebuilds against the new base itself). Until then it is held rather than re-thrown at a base known to reject it.


Generated by Claude Code

@os-zhuang
os-zhuang added this pull request to the merge queueAug 28, 2026
Merged via the queue into main with commit 3547f2dAug 28, 2026
37 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-12688-refactoring-summary-retired-config-validation branch August 28, 2026 09:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

packages/core/REFACTORING_SUMMARY.md section 5 claims config validation that never ran, and now describes a retired mechanism

2 participants

@os-zhuang@claude