Skip to content

fix(kit): decide a module's kind in one place - #42

Merged
afonsojramos merged 5 commits into
mainfrom
refactor/kit-kind-helper
Oct 1, 2026
Merged

afonsojramos merged 5 commits into
mainfrom
refactor/kit-kind-helper

Conversation

@afonsojramos

@afonsojramos afonsojramos commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Summary

Several places decided a module's kind from metadata.json on their own, and three of them disagreed with kindOfMeta (kind wins, tags is the fallback, unknown kinds are ignored):

  • kit dev's in-page push check and the stdlib boundary check called { kind: "extension", tags: ["theme"] } a theme; the push check also did so for { tags: ["theme", "extension"] } and a string tags: "theme".
  • The Store's kindOf treated a string tags field as a list.

Now:

  • kindOfMeta (packages/kit/src/vault-metadata.ts) is the one rule for the kit and scripts. The push script runs in Spotify's page and can't import, so it gets KIND_OF_META_SOURCE, a string form of the same rule built from the same KINDS list. stdlib-boundary.ts and scripts/preview.ts (which had its own kinds list) use kindOfMeta.
  • The Store keeps its own kindOf, so no kit code enters the client bundle, but now only accepts tags as an array. Store bumped to 1.8.1.
  • A guard test fails on any .tags read in packages/kit/src or scripts/ outside the helper.

Testing

  • 20 shared metadata shapes (packages/kit/test/kind-cases.ts) drive: the in-page rule against kindOfMeta in a VM; the full push script against a fake loader, for each shape as the pushed and the installed module; and the Store's kindOf against kindOfMeta. Restoring the old push check fails 3 of them; restoring the old boundary check fails its new test; the Store test failed on tags: "theme" before its fix.
  • The guard test fails, naming the lines, against the previous push.ts, stdlib-boundary.ts and preview.ts.
  • Kit, Store and guard tests: 254 pass. release.ts status --soft: ok.

Summary by CodeRabbit

  • Bug Fixes
    • Module categories are now determined consistently from metadata, including when tags are missing or malformed. A recognized category takes precedence over tags.
    • Preview listings and module checks now use the same category rules.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 4948d7ab-f0c9-4ae2-abc3-40ac415bcc67

📥 Commits

Reviewing files that changed from the base of the PR and between 83062f9 and 9701d37.

📒 Files selected for processing (13)
  • modules/store/catalog.test.mts
  • modules/store/catalog.ts
  • modules/store/metadata.json
  • packages/kit/src/push.ts
  • packages/kit/src/stdlib-boundary.ts
  • packages/kit/src/vault-metadata.ts
  • packages/kit/test/kind-cases.ts
  • packages/kit/test/push.test.mts
  • packages/kit/test/stdlib-boundary.test.mts
  • scripts/client-boundary.test.mts
  • scripts/kind-of-meta.test.mts
  • scripts/preview.ts
  • scripts/settings-ownership.test.mts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

Metadata kind selection is centralized for kit and script consumers. The store catalog validates tags before searching them, and tests compare its results with the shared rule.

Changes

Metadata Kind Classification

Layer / File(s) Summary
Shared metadata kind rule
packages/kit/src/vault-metadata.ts, packages/kit/test/kind-cases.ts
Adds KIND_OF_META_SOURCE and shared fixtures for metadata inputs with varied kind and tags values.
Kit and script consumer adoption
packages/kit/src/push.ts, packages/kit/src/stdlib-boundary.ts, packages/kit/test/push.test.mts, packages/kit/test/stdlib-boundary.test.mts, scripts/preview.ts, scripts/client-boundary.test.mts, scripts/settings-ownership.test.mts, scripts/kind-of-meta.test.mts
Kit and script consumers use the shared metadata kind rule. Tests check parity, push behavior, dependency checks, and source reads of tags.
Store catalog kind handling
modules/store/catalog.ts, modules/store/catalog.test.mts, modules/store/metadata.json
The catalog checks that tags are an array before searching them. Tests compare catalog results with the shared rule, and the store metadata version changes to 1.8.1.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 9701d

The change aligns metadata classification across consumers and safely handles malformed tags in the Store. No actionable blocker is identified; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 9701d

The change aligns metadata decisions without establishing a new execution capability or weaker control. Risk is low, with uncertainty remaining around loader recovery and concurrent state changes.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The inspected runtime decision can affect every previously loaded module in the connected client because push iterates that snapshot. Other inspected classification sinks are development-time stdlib checks, preview generation, and Store categorization.

Trust Boundaries and Controls

  • observed — At the inspected page-evaluation boundary, record contents and identifiers enter through JSON serialization. The classifier source contains fixed logic and a fixed kinds list, not metadata-derived executable text. Metadata influences the theme decision but is not itself authorization to access the page evaluator.

Resilience and Maintainability Implications

  • observed — The visible push wrapper has sequential installation and enablement steps without an enclosing transaction, and individual re-enable errors are swallowed. Result reporting checks the pushed module and execution stamp. Production loader guarantees were not established, so neither full recovery nor a PR-introduced failure was concluded.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 12 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: centralizing module-kind decisions for the kit and scripts.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 12 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the tags at dawn,
And sorts each kind before moving on.
The shared rule guides the way,
While tests compare each case of day.
The catalog guards its list with care,
Then hops along without a hare.

Comment @coderabbitai help to get the list of available commands.

@afonsojramos
afonsojramos merged commit 9294cfe into main Oct 1, 2026
6 checks passed
@afonsojramos
afonsojramos deleted the refactor/kit-kind-helper branch October 1, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant