refactor(extensions): single-source plugin data ownership; drop the staticDataExtensions duplicate list#166
Open
Stvad wants to merge 4 commits into
Open
refactor(extensions): single-source plugin data ownership; drop the staticDataExtensions duplicate list#166Stvad wants to merge 4 commits into
Stvad wants to merge 4 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71bb6d29b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Stvad
pushed a commit
that referenced
this pull request
Jun 18, 2026
… enable The Repo's static-runtime install is now toggle-aware (bootstrapWorkspace), so a plugin disabled at workspace-open has its workspaceBackfillsFacet contribution pruned. Enabling it mid-session goes through AppRuntimeProvider's setFacetRuntime path, which previously didn't re-schedule backfills — leaving the plugin's one-shot repair backfill unapplied until a reload. Call repo.scheduleWorkspaceBackfills after the merged-runtime install; it's marker-gated and idempotent, so already-run backfills no-op and unrelated swaps cost nothing. (Addresses Codex P2 on #166.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Me6NyHHry6WAoCVboujw5H
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea35cfa9cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Stvad
pushed a commit
that referenced
this pull request
Jun 18, 2026
…setFacetRuntime target `repo.setFacetRuntime` mutates its argument — `adoptDurableContributionsFrom` copies the previous runtime's durable `user-data` buckets (user property schemas / types) onto it. The bootstrap install was handed a runtime cached in `staticAppRuntimeCache`, so on a workspace switch the cached object accumulated the prior workspace's adopted user-data and replayed it on a later bootstrap for that workspace — before the new workspace's projectors cleared/repopulated the bucket — letting bootstrap writes/type snapshots transiently see another workspace's schemas/types (and persisting it on cache hits). Build the runtime fresh every bootstrap instead (cheap: modules are already imported, this only walks the extension array; getInitialLayout's promise cache already dedupes re-entry) and reuse the one fresh object for both the Repo install and landing resolution. (Codex P2 on #166.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Me6NyHHry6WAoCVboujw5H
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6fccdd48f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Stvad
pushed a commit
that referenced
this pull request
Jun 18, 2026
…o hand lists)
Two convention-based globs replace the hand-maintained plugin lists, so a
plugin is discovered just by living under src/plugins — no import to forget.
Plugin assembly (staticAppExtensions):
- glob `plugins/*/index.{ts,tsx}` default exports instead of ~40 explicit
imports. Each plugin index default-exports its AppExtension (or a
`({repo}) => AppExtension` factory); an optional `pluginOrder` overrides
the alphabetical default where registration order matters (app-intents
last; the settings meta-plugins first).
- core non-plugin extensions (kernel, renderers, toasts, default shortcuts)
stay an explicit list, kept first so plugins override via last-wins.
Local schema (data layer, headless):
- glob `plugins/*/localSchema.ts` default exports → PLUGIN_LOCAL_SCHEMAS
(the 2 leaf DDL modules), NOT `dataExtension.ts`. A dataExtension carries
actions/effects whose modules transitively import the React/app graph
(settings action → navigation → React + context/repo); globbing those
into the headless local-schema path (createTestDb/bench) re-imported that
graph — measured ~1.5s/import vs ~150ms for the leaf modules. (Addresses
Codex P2 on #166.)
- remove the now-unused `localSchemaFacet` + `resolveLocalSchemaContributions`:
local schema is owned by each plugin's `localSchema.ts` default export and
applied directly. Move the property-editor / CodeMirror UI out of the
settings + geo data extensions into their index (data/UI split).
Verification: tsc, full suite (301 files / 3271 tests), and `vite build` all
green; `import.meta.glob` confirmed compiling in the prod build.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Me6NyHHry6WAoCVboujw5H
…gle-aware data ownership (C)
Removes the hand-maintained plugin lists and the staticDataExtensions
duplication; a plugin is now discovered by convention from its files.
Assembly: `staticAppExtensions` globs `plugins/*/index.{ts,tsx}` default
exports (no ~40-import list). Each index default-exports its plugin (or a
`({repo}) => AppExtension` factory); optional `pluginOrder` overrides the
alphabetical default where registration order matters (app-intents last;
settings meta-plugins first). Core non-plugin extensions (kernel, renderers,
toasts, shortcuts) stay an explicit first list so plugins override via
last-wins.
Data: `pluginDataExtensions` globs `plugins/*/dataExtension.ts` default
exports — ALL plugin data, headless-ready. `dataExtension.ts` is now an
enforced convention: DATA ONLY, graph-free — no components/editors/actions
(those import React/navigation and would drag the app graph into the headless
local-schema path). Moved the settings property-editor UI + the two settings
command-palette actions, and the geo CodeMirror surface, into their indexes.
localSchema is read off this glob via `localSchemaFacet` for the pre-observer
DDL path (repoProvider / createTestDb / bench), which must stay UI-free.
Ownership: repo.tsx no longer installs a separate data list; the toggle-aware
`staticAppExtensions` runtime is installed in `bootstrapWorkspace` (built
fresh per bootstrap — setFacetRuntime mutates its arg via durable-bucket
adoption, so a cached runtime would leak the prior workspace's user-data),
fixing the secretly-enabled gap (disabled plugin ⇒ its data absent).
AppRuntimeProvider re-schedules workspace backfills after the merged-runtime
install so enabling a plugin mid-session applies its one-shot backfill.
The devtools metrics console hook moves to an appEffectsFacet effect so the
extension tree builds side-effect-free.
Verification: tsc, full suite (301 files / 3272 tests), and `vite build` green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Me6NyHHry6WAoCVboujw5H
Stvad
force-pushed
the
claude/data-wrapper-c
branch
from
fac374a to
dff608f
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ry invariants Adversarial review of the glob-discovery refactor (C) surfaced one real behavior regression and the lack of enforcement for the conventions C relies on. Addressed: - Same-tx processor order flip (references ↔ alias). The pre-glob `staticDataExtensions` list ran references' merge-retarget before alias's sync by hand; glob discovery orders contributions alphabetically-by-path (alias < references), silently reversing them. Both fire inside a single `core.merge` tx and can touch the same row, so order is load-bearing. Restored references-first with a precedence on alias's same-tx contribution (kernel + references keep the default 0, staying ahead of alias). Added a regression test that pins the execution order in the discovered runtime. - Loud failure on a missing default export. A `plugins/<name>/index.*` or `dataExtension.ts` without a default export was silently dropped from the assembled app / data layer; both globs now throw, naming the module. - Data-only / graph-free guard. New node-env test asserts the discovered `pluginDataExtensions` contribute no UI facets (actions, renderers, app-mounts, header items, codemirror, markdown, property-editor overrides) — catching the React/CodeMirror leak that the eval-safety test cannot (those import fine under Node without throwing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Me6NyHHry6WAoCVboujw5H
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Brings 335 commits of master into the glob-discovery refactor (C) and propagates the new model onto everything master added. Conflict resolutions (kept our glob model, folded in master's work): - staticAppExtensions.ts: kept the `import.meta.glob` discovery; it auto-picks up master's 8 new plugins. Preserved master's moved import paths (@/editor/*). app-intents-last / settings-meta-first / onboarding landing precedence are all preserved (pluginOrder + explicit precedence). - workspaceBootstrap.ts: kept our bootstrap-time toggle-aware `setFacetRuntime` install (built FRESH, reused for landing) and folded in master's extracted `resolveLayoutSession` + `ensureSystemPages`. Dropped master's landingRuntimeCache (unsafe once the runtime is the one we install — setFacetRuntime mutates it). - geo/dataExtension.ts: kept master's new data-only `systemPagesFacet` (Locations page) but left codeMirror in index.ts (data-only invariant). - defaultShortcuts.ts: combined our metrics-hook effect with master's dialogAppMountExtension; took master's @/paste/operations path. Propagation: - staticDataExtensions.ts stays deleted; its master additions (character-counter, geo) flow through the glob + pluginDataExtensions. - 8 new plugins got `export default` for discovery; character-counter's dataExtension is data-only. sync-status was renamed to system-status on master — removed the resurrected dir so the glob doesn't double-register. - onboarding/seed.test redirected off the deleted staticDataExtensions to the `pluginDataExtensions` glob; stale construction-time comments updated to the bootstrap-install model. - Fixed stale @/extensions/editor import paths (master moved them). Verified: tsc -b + app clean, full suite 338 files / 3693 tests green, eslint 0 errors, vite build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Me6NyHHry6WAoCVboujw5H
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Second catch-up merge (292 commits: c23f974..e5e514c) onto the glob-discovery refactor (C). Conflicts resolved (kept our glob model): - staticAppExtensions.ts: kept `import.meta.glob` discovery; it auto-picks up master's new `attachments` plugin and the settings meta-plugins (master moved those into its hand-list's core section — in our model they're globbed with pluginOrder -10/-9, so kernel-before-settings is a no-op reorder). app-intents-last preserved. - onboarding/index.ts: took master's new `({repo})` factory form (adds the "Insert tutorial" action) and kept our staticAppExtensions-model comment; the glob resolves the factory with {repo}. Kept `export default`. - References/agent-runtime/seed tests: adopted master's `createTestRepo` helper; kept our same-tx order regression + the pluginDataExtensions redirect. Dropped the now-unused kernelDataExtension/staticDataExtensions imports. Propagation: - attachments (new plugin) got `export default` for discovery. - Installed master's new `fflate` dep; rebuilt the agent-cli workspace dist (tsc -b) so vitest's dist-resolved protocol registry is current. Verified: tsc -b + app clean, full suite 399 files / 4200 tests green, eslint 0 errors, vite build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Me6NyHHry6WAoCVboujw5H
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
2 participants
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.
What
Removes the audit B1(c) compensation: plugin data was defined once (each
dataExtension.ts) but referenced in two parallel lists —staticDataExtensions(installed into the Repo pre-mount, toggle-blind) and the per-plugin bundling inside each plugin's toggle subtree. Adding a data-owning plugin meant editing both, and the toggle-blind construction install registered disabled plugins' data ("secretly enabled").Now plugin data ownership (mutators / types / queries / processors / schemas / backfills) comes from the single
staticAppExtensionstree:repo.tsxno longer installs a separatestaticDataExtensionsruntime at construction — the Repo comes up kernel-only (viainstallKernelRuntime).bootstrapWorkspaceinstalls the toggle-aware static runtime into the Repo before the bootstrap writes that need plugin data (the daily-notes landing resolver'saddTypeInTx, seedTutorial's references processor). It reuses the runtime it already builds for landing. Resolved with the workspace's toggle overrides, so a disabled plugin's data is genuinely absent — fixing the secretly-enabled gap.staticDataExtensions.tsis deleted; the 6 data-only plugins lose any separate listing.Plus: the devtools
__omniliner.metricsconsole hook moves off the extension-tree build path (it touchedwindowwhile buildingdefaultActionsExtension) intometricsConsoleHookEffectviaappEffectsFacet, so resolving the tree is side-effect-free.The one residual list:
PLUGIN_LOCAL_SCHEMASLocal schema (derived-index DDL + triggers, from daily-notes + references) keeps a focused, UI-free 2-entry list. It can't come from the tree, for three hard reasons:
block_referencestriggers (a missing table aborts the drain).repoProvider,createTestDb, andscripts/bench(a real headless@powersync/nodeconsumer) must not import the React component graph. Measured: ~1.9s of added module eval per test file if they did (91 test files importcreateTestDb).This keeps the data layer UI-free → headless works by construction (the bench proves it), with no eval-safety invariant to police.
Open design question (for review)
This is the hybrid. The alternative ("option B") is a single list everywhere — point the data layer at the tree too, deleting
PLUGIN_LOCAL_SCHEMAS. Measured cost of B: ~+30s CI (the ~1.9s/file delta above, parallelized) and the entire 40-plugin tree must stay Node-eval-safe forever (2 violations found in minutes during exploration). B trades a robust property (UI-free data layer) for an aesthetic one (one list). This PR takes the hybrid; happy to pivot if reviewers prefer B.Verification
tsc -bclean; directly-affected tests green (queryRun, agent-runtime/commands, createTestDb, daily-notes plugin, references dataExtension, shortcuts).🤖 Generated with Claude Code
Generated by Claude Code