Skip to content

fix(objectql): a nested plugin registers jobs / emailTemplates / tools / skills — the two registration copies become one enumeration (#7049) - #7153

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-7049-nested-plugin-collections
Aug 10, 2026
Merged

fix(objectql): a nested plugin registers jobs / emailTemplates / tools / skills — the two registration copies become one enumeration (#7049)#7153
os-zhuang merged 2 commits into
mainfrom
claude/issue-7049-nested-plugin-collections

Conversation

@os-zhuang

@os-zhuang os-zhuang commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Closes #7049.

The defect

packages/objectql/src/engine.ts reaches the ADR-0010 provenance-stamping seam — registerItemapplyProtection, the only place _packageId / _provenance are written — from two entry points: a manifest (registerApp()) and a nested plugin (registerPlugin(), reached via manifest.plugins[]). Each carried its own copy of the collection list, and the two copies had drifted by four collections:

jobs, emailTemplates, tools, skills — registered from a manifest, not registered from a nested plugin.

A package shipping any of the four via a nested plugin therefore registered nothing: no refusal, no diagnostic, no provenance stamp. The measured symptom, from the reverse-verification below: expected [] to deeply equal [ 'nightly_invoice' ].

The shape decision, and the measurement behind it

The obvious fix is to copy the four names into the nested loop. That is precisely what #5870 did for capabilities — and this card exists because that patch-one-name approach left the rest of the two lists undiffed. Hand-adding four more names closes this instance while reproducing its cause.

So the two loops were measured against each other rather than chosen between by taste:

Aspect Manifest seam Nested-plugin seam Does it require different collection lists?
Object read manifest[key] plugin[key] No — a parameter
Package id stamped id ownerId (= parentId) No — both resolve to the same parent package; a nested plugin contributes under its parent's ownership
Per-key debug line yes no No — loop body
Aggregated-view expansion (ADR-0017) yes no No — loop body
Warn on non-derivable name yes no (silent skip) No — loop body
Ordering / error handling list order; no local try identical No

Every difference lives in the loop body; not one of them is expressible in, or caused by, which collections are enumerated. So the key list is not where the two seams legitimately differ — it is only where nobody was looking. The enumeration is hoisted to one module-scope METADATA_ARRAY_KEYS that both seams read, which makes this class of divergence unrepresentable rather than merely unnoticed.

Why the next divergence is caught, since "nobody diffed the two lists" is the documented root cause:

  1. There is no second list to diff — TypeScript resolves one identifier at both seams.
  2. engine-nested-plugin-collections.test.ts pins the seams' agreement as a property over every comparable collection (23 of them), not over the four that happened to diverge: ship one item through a manifest and the same item through a nested plugin, and the two registries must agree, in both directions. A collection added to one seam only cannot pass it. Collections excluded from the sweep are listed with the reason each is excluded, and that list is itself pinned.
  3. check:stack-collection-maps still reconciles the (now single) enumeration against ObjectStackDefinitionSchema in both directions.

The waiver handshake (atomic, per #6242's ratchet)

scripts/check-stack-collection-maps.mjs treats its waiver list as a ratchet (:53), and states ⛔ A waiver is NOT permission (:323). The row recording this divergence — keys: ['jobs', 'emailTemplates', 'tools', 'skills'] — is removed in this same PR, along with the now-empty second ObjectQL site. Both halves were verified to be genuinely coupled:

Result
Gate on origin/main, before ✓ 8 enumerations reconciled against 32 declared collections (19 waiver rows)
Gate after the fix ✓ 7 enumerations reconciled against 32 declared collections (16 waiver rows)
New gate script vs. pre-fix engine.ts ✗ METADATA_ARRAY_KEYS — could not extract the enumeration — red, as the handshake requires
--self-test ✓ 11 assertions over synthetic sources (unchanged)

The gate's own header prose and the sliceBody(from) docblock, which both described ObjectQL as "declaring its list twice", are updated to say what is true now and to record what the removed row recorded.

Reverse-verification

Predictions were written before the run: restoring origin/main's engine.ts under the new tests should fail exactly 13 of 34 — the 4 registers …, the 4 stamps …, registers all four at once, and the 4 property cases for jobs / tools / skills / emailTemplates; everything else green.

Measured: 13 failed | 21 passed, matching test-for-test. No deviation. With the fix restored: 34 passed. Full @objectstack/objectql suite: 166 files / 2883 tests, all passing.

Behaviour change (stated plainly in the changeset)

Packages that already ship today and declare any of the four via a nested plugin now register them. /meta/job, /meta/email_template, /meta/tool and /meta/skill begin answering for such a package; the email plugin's sys_email_template materializer (#4509) begins seeing its templates. Anything that has been compensating for the silence — a duplicate top-level declaration, a hand-seeded row — will now find the collection already registered.

Out of scope, filed rather than folded in — #7163

The manifest seam expands an aggregated view container into per-view items (ADR-0017) and the nested seam does not. Measured while closing this card: one container registers ['account', 'account.all_accounts', 'account.form'] from a manifest and ['account'] from a nested plugin. That is the same silent-under-registration class one layer in, but it is a loop-body difference this card did not measure and it changes what a nested plugin serves. Filed as #7163 and recorded in the test file's exclusion list with the measured values, so the exclusion is falsifiable rather than asserted.

The seven waivers at :406 and :523 naming overlapping key sets were left alone, per the card's scope fence.

Refs: #7049, #7032 (measurement + waiver row), #6242 (enumeration sweep), #5870 (capabilities precedent), #4509, #7163, ADR-0010, ADR-0017.

… nested plugin too (#7049)

`engine.ts` reaches the ADR-0010 provenance-stamping seam (`registerItem` ->
`applyProtection`) from two entry points -- a manifest and a nested plugin --
and each carried its OWN copy of the collection list. The copies had drifted by
four collections: `jobs`, `emailTemplates`, `tools` and `skills` registered from
a manifest and NOT from a nested plugin, so a package shipping any of them via
`manifest.plugins[]` registered nothing and stamped no provenance -- no refusal,
no diagnostic.

Hand-adding the four names was the available alternative and is exactly what
#5870 did for `capabilities`; it is what left these four undiffed. Measured the
two loops against each other instead: they differ in which object they read,
which package id they stamp (both resolve to the same parent package), a per-key
`debug` line, and the manifest seam's aggregated-view expansion plus its
warn-on-nameless-item. All four are loop-BODY differences; none is a reason for
the seams to enumerate different collections. So the enumeration is hoisted to
one module-scope `METADATA_ARRAY_KEYS` both seams read, and the divergence is
unrepresentable rather than merely unnoticed.

`check:stack-collection-maps` now pins one ObjectQL enumeration instead of two,
and the waiver row recording the divergence is removed in the same change --
#6242's ratchet handshake: 8 enumerations / 19 waiver rows -> 7 / 16.

Tests pin each of the four registering AND carrying `_packageId` /
`_provenance` from a nested plugin, plus the seams' agreement as a property over
every comparable collection rather than over the four that happened to diverge.

Refs: #7049, #7032, #6242, #5870, #4509, ADR-0010.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CXFj4BYpeUHyTX4eqirbeQ
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 1:33am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql.

14 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx (via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx (via @objectstack/objectql)
  • content/docs/deployment/vercel.mdx (via @objectstack/objectql)
  • content/docs/kernel/runtime-services/examples.mdx (via packages/objectql)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/objectql)
  • content/docs/kernel/services.mdx (via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx (via @objectstack/objectql)
  • content/docs/permissions/system-context.mdx (via packages/objectql)
  • content/docs/plugins/index.mdx (via @objectstack/objectql)
  • content/docs/plugins/packages.mdx (via @objectstack/objectql)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx (via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx (via @objectstack/objectql)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/objectql)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

…erence (#7163)

The `views` exclusion in the seam-agreement sweep is out of scope for #7049 by
measurement, not by convenience: one aggregated container registers
['account', 'account.all_accounts', 'account.form'] from a manifest and
['account'] from a nested plugin. Filed as #7163; recording the issue number and
the measured values so the exclusion is falsifiable from the test file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CXFj4BYpeUHyTX4eqirbeQ
@os-zhuang
os-zhuang marked this pull request as ready for review August 10, 2026 01:46
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit bf42e76 Aug 10, 2026
27 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7049-nested-plugin-collections branch August 10, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants