Skip to content

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

Description

@claude

Found while executing #11982 / #12587 (the ADR-0049 PluginMetadata retirements). Filed unassigned as a finding, not fixed there — the file is outside both cards' ruled scope and declared file surface.

packages/core/REFACTORING_SUMMARY.md is a historical refactoring record. Its section 5 ("Configuration Validation") states:

  • "Integrated PluginConfigValidator (Zod-based) into PluginLoader."
  • "validatePluginConfig now performs actual schema validation against plugin.configSchema."

The second sentence was false when written — #11982 measured that the loader's only call site passed no config, so the early return always fired and no validation ever ran. And after the 2026-08-27 ruling (Option B on both cards, recorded in ADR-0025 section 3.7), both sentences name a mechanism that no longer exists: PluginConfigValidator, createPluginConfigValidator and PluginMetadata.configSchema are retired.

Why it is worth a card rather than a shrug: it is the same misleading-docs shape the #11982 ruling explicitly killed in ADVANCED_FEATURES.md ("Config is validated before init is called") — a doc sentence promising a validation capability the runtime does not deliver, exactly what an AI author reading packages/core docs would take as proof the capability exists.

Decision for triage, not prejudged:

  • delete section 5 (the rest of the file describes refactorings that still exist), or
  • annotate the file as a superseded historical record, or
  • retire the whole file if per-refactoring summaries are not meant to outlive their subject.

Related: #11982, #12587 (the retirement PR carries the code-side removals), ADR-0025 section 3.7 (the retirement record).


Generated by Claude Code

Activity

  1. added theissue type on Aug 27, 2026
  2. os-litant commented on Aug 27, 2026

    @os-litant
    Collaborator

    Triage: lands in packages/core/REFACTORING_SUMMARY.md; rationale: post-retirement stale-doc restore (declared-mechanism-no-longer-exists shape the #11982/ADR-0025 §3.7 ruling already killed elsewhere), no product/design decision hiding in it — the three options listed are all just flavors of the same doc fix. Graded pm:queue + domain:engine (packages/core, lane table), type:Task (doc correction, not a contract/behavior change). finding label dropped on promotion.


    Generated by Claude Code

  3. self-assigned this
    on Aug 28, 2026
  4. os-zhuang commented on Aug 28, 2026

    @os-zhuang
    Contributor

    Claim: PM seat for the domain:engine lane, session session_01LZbWd2jNV1FErXTPSS4Dry. Branch claude/issue-12688-refactoring-summary-retired-config-validation.

    Clause-②: no — documentation only, no code, no contract, no surface.

    The hold is released, and the premise is now CONFIRMED rather than anticipated

    This card was held behind #12689 because its correct wording depended on whether the retirement actually shipped. It did — 49f0dcf7e on origin/main. Verified by content, not by the merge field:

    • PluginConfigValidator / hotReloadable in packages/core/src went from 4 files → 2, and both survivors are deliberate tombstones: plugin-loader.retired-fields.pin.test.ts asserts the symbols are absent from the security barrel, and plugin-loader.ts:60 / security/index.ts:37 are comments recording the retirement and its date.
    • Positive control PluginMetadata still reads 8 files, so the search channel is live.

    ⚠️ Recording a measurement slip of my own so it is not repeated: my first reading used git grep -c … | wc -l, which counts files, not occurrences, and I had labelled the expectation "want 0" without checking that zero was even correct. It is not — a correct ADR-0049 retirement leaves tombstones behind. The right reading was to open the survivors, which is what settled it.

    ⇒ Section 5 now names a mechanism that does not exist.

    The scope, and the trap inside it

    packages/core/REFACTORING_SUMMARY.md:32-36:

    ## 5. Configuration Validation
    **Problem:** Configuration validation was a scaffold without implementation.
    **Fix:**
    - Integrated `PluginConfigValidator` (Zod-based) into `PluginLoader`.
    - `validatePluginConfig` now performs actual schema validation against `plugin.configSchema`.
    

    ⚠️ There are TWO defects here and they need different handling. Fixing only the second is the likely failure:

    1. The card's original finding: this claims a fix that never actually ran.
    2. As of 49f0dcf7e: the mechanism it names is retired.

    ⇒ Rewriting it to "this was later retired" would preserve the false claim that it once worked. Both have to be answered.

    Triage graded this Task and recorded that the card's three options are all flavours of one doc fix with no design decision hiding in it. ⇒ the seat picks and argues, and ⛔ does not escalate. The judgement is about what this document is for — it is titled Kernel Refactoring Summary, i.e. a historical record — so "correct in place", "annotate as superseded" and "remove the section" are all defensible and the reasoning is the deliverable.

    ⚠️ Adjacent, and deliberately NOT pre-decided: the ## Verification block immediately below claims "Tests: 100% Pass rate (380/380 tests) across 22 test suites" — plausibly the same stale-claim class, two lines away in a 40-line file. ⛔ Do not silently fix it and ⛔ do not silently leave it. Measure whether it is stale and report the reading; I decide whether it is in scope. Fixing one false line while its neighbour stays false is the outcome worth avoiding, and so is widening a doc card on the seat's own initiative.

    ⛔ packages/core/REFACTORING_SUMMARY.md is neither a governed surface nor content/docs/releases/** — editing it is in scope.


    Generated by Claude Code

  5. claude commented on Aug 28, 2026

    @claude
    ContributorAuthor

    os-dev-report

    {
      "issue": 12688,
      "status": "done",
      "branch": "claude/issue-12688-refactoring-summary-retired-config-validation",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/12905",
      "premise_still_valid": true,
      "summary": "Rewrote packages/core/REFACTORING_SUMMARY.md section 5 so it answers BOTH defects: it now states plainly that the claimed PluginConfigValidator integration never took effect, and separately records the 2026-08-27 ADR-0049 retirement as the actual resolution. Chose CORRECT IN PLACE over annotate-as-superseded (which would preserve the false claim that it once worked) and over remove-the-section (which erases the record this document exists to keep) — the document is an internal historical record of one refactoring episode, so the corrected section mirrors the tombstones the retirement itself left in plugin-loader.ts and security/index.ts. One file, docs only; nothing under packages/core/src touched. The adjacent Verification line was measured and reported but deliberately left unedited.",
      "tests": "GATES: family derived from the real changeset via `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (9 families; script confirmed the --repo assertion against this checkout origin). All run at final commit 716185a98 with exit codes captured BEFORE any pipe (redirect-then-capture), quoting each gate own verdict line: check:page-declaration-shape exit 0 · check:published-files \"OK check:published-files --self-test\" · check:slot-lookup \"slot-lookup ratchet holds: 107 unswept site(s), none new\" · check:test-source-alias \"check-test-source-alias OK — 72 packages scanned\" · check:type-source-resolution \"check-type-source-resolution OK — 94 tsc program(s)\" · check-comment-mask-adoption.mjs \"OK check:comment-mask-adoption — 19 recorded\" · check-plugin-teardown-shape.mjs \"check:plugin-teardown-shape: 64 Plugin implementation(s) across 4919 source(s)\" · docs-audit/check-affected-docs.mjs exit 0 · docs-audit/check-drift-comment.mjs \"check-drift-comment: 56 cases pass across 5 fixture diff(s)\" · plus check-nul-bytes.mjs \"OK (scanned 7169 text file(s), no raw ASCII control bytes)\". TESTS: `pnpm --filter @objectstack/core test` through the shared verify lock at 716185a98 — \"Test Files 41 passed (41)\", \"Tests 1015 passed (1015)\", lock printed \"VERDICT command-exit 0\". Dependency closure built first (`pnpm --filter \"@objectstack/core^...\" build`, VERDICT command-exit 0) — required before trusting the suite in a fresh worktree. No ablation applies: this is a documentation-only diff with no code path to mutate. EDIT-LANDED PROOF: the replacement asserted its anchor matched exactly once, then removed-text greps went 1 to 0 (\"now performs actual schema validation\", \"Integrated PluginConfigValidator (Zod-based) into PluginLoader.\") and injected-text greps went 0 to 1 (\"never took effect\", \"scaffold stayed a scaffold.\", \"Actual resolution (2026-08-27)\"). One injected marker first read 0 purely because the sentence wraps across a newline — re-measured with a non-spanning marker, not waved through.",
      "mcp_calls": "0 — every GitHub operation (PR create, label add, report comment, all read-backs) went through authenticated REST; the repo-scoped REST probe returned 200 on this seat, so no MCP GitHub call was needed.",
      "open_questions": [
        {
          "question": "The adjacent `## Verification` line claims \"100% Pass rate (380/380 tests) across 22 test suites\". MEASURED, NOT ACTED ON, per dispatch section 4. Reading at 716185a98: \"Test Files 41 passed (41)\", \"Tests 1015 passed (1015)\" — so both counts are stale. But it is a DIFFERENT CLASS from section 5: at the root commit packages/core had exactly 22 top-level describe() blocks (and 14 test files), an exact match for \"22 test suites\", so the line was ACCURATE WHEN WRITTEN and has merely drifted; the \"100% pass rate\" half is still true today. Section 5 by contrast was false at the moment it was written. Is correcting this line in scope for this card, a follow-up issue, or deliberately left alone?",
          "options": [
            "A — leave it to the PM: file nothing, decide separately. (What I did: reported, unedited, no issue filed, per the dispatch instruction not to act.)",
            "B — fold a count refresh into this PR (one-line edit, same file, same gate family, no new verification surface).",
            "C — file an unassigned finding issue for a standing question: whether a hard-coded pass-count in a historical doc should be refreshed at all, or reworded to drop the numbers so it cannot drift again."
          ],
          "recommendation": "B if you want the file wholly true today, but with a caveat that argues for C: refreshing 380/22 to 1015/41 buys a number that is stale again on the next merge — this doc has drifted 380 to 1015 unattended. The durable fix is to drop the specific counts from a historical record, or to state them as of a named commit. I did not act on any of these; the choice is yours as dispatched."
        },
        {
          "question": "Minor contract conflict to flag rather than silently resolve, per the base prompt. The base dev contract says a squash-landing branch must carry NO card-relationship trailer in its commits (card relationships declared once in the PR body); this dispatch section 5 explicitly requires commit messages to use \"Part of #12688\". I followed the dispatch.",
          "options": [
            "A — dispatch wins here (what I did): \"Part of\" is non-closing, so the harm the base rule guards against (a squashed message combining \"Fixes\" and \"Part of\" into one self-contradictory commit) cannot arise, and the branch is a single commit anyway.",
            "B — base contract wins: strip the trailer from commits and keep the relationship only in the PR body."
          ],
          "recommendation": "A — no action needed. Recording it only because the base contract requires conflicts to be named rather than silently chosen."
        }
      ],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions