[mstomar125]

Closes #3341

Carries an opaque, per-database extensions map end to end (ADR-083 §2, decision 2-A) and deletes the tray's "synced" label (ADR-083 §2 and §7 item 2). Part of the Pro/core boundary epic (#3298). Core reads no key from extensions. Nothing is removed yet: proto fields 8 and 9 and the typed binding fields stay until #3342.

What changed

• Registry (packages/daemon/src/services/database_manager.rs): DatabaseEntry.extensions: toml::Table via #[serde(flatten)]. Every key of a databases.toml entry that the registry does not define now loads into it and is written back on every save. Before, an unknown key was ignored on load and silently dropped on the next save. The three Option fields gained skip_serializing_if = "Option::is_none".
• Wire (database_service.proto): map<string, string> extensions = 10 on DatabaseInfo. listing_to_info fills it from the string-valued entries and leaves other value types (integer, boolean, array, table) in the registry, out of the response.
• Desktop DTO (packages/desktop-app/app-lib/src/commands/database.rs, the crate split has landed): DatabaseEntry.extensions: BTreeMap<String, String>, copied in to_entry.
• Frontend: DatabaseInfo.extensions: Record<string, string>, extensions: {} on IMPLICIT_BROWSER_DATABASE, and on the four fixtures that build a full DatabaseInfo.
• Tray: the synced marker is gone and the database_menu_entries doc is rewritten to match what it renders (default flag, live open state, missing-file state; the old "deliberately NOT whether open" paragraph was stale).

Existing registries stay readable

A registry written before this change may carry the typed binding fields. They still load into their typed fields (the fields are untouched) and are not duplicated into extensions; the manual check below covers a copy seeded with them.

Tests

• database_manager: registry_round_trips_entry_keys_it_does_not_define (string, nested table sorted before a scalar, native TOML datetime; defined keys are intact; save and reload keep everything; a second save writes the same bytes) and a_rename_keeps_extension_keys. The existing reload test also asserts a typed-field entry loads with empty extensions.
• database_service: listing_to_info_carries_string_extensions_only.
• app-lib commands::database: to_entry_passes_extensions_through, including the camelCase JSON key.
• tray: labels_carry_default_state (renamed), markers_render_in_a_stable_order now expects All — default · open, extension_keys_add_no_marker, and a_recorded_binding_adds_no_marker (an absence test, so its key is built from fragments) which guards the deleted label now that no fixture binds a database.

Mutation checks (each reverted afterwards):

Mutation	Tests that fail
#[serde(flatten)] -> #[serde(skip)] (today's behaviour)	registry_round_trips_entry_keys_it_does_not_define, a_rename_keeps_extension_keys, listing_to_info_carries_string_extensions_only
tray marker for non-empty extensions	extension_keys_add_no_marker
synced marker restored	a_recorded_binding_adds_no_marker
listing_to_info stringifies every value	listing_to_info_carries_string_extensions_only
to_entry drops extensions	to_entry_passes_extensions_through
remove skip_serializing_if from the three Option fields	none: toml 0.8.2 skips None in a flattened map either way, so no test can tell. Kept because the issue specifies it and it stops the save depending on that serializer detail.

Manual check (isolated NODESPACE_HOME, never ~/.nodespace)

A databases.toml seeded with the old typed binding fields, extra scalar keys, an integer key and a nested table, on two entries. A headless nodespaced from this branch started and listed both databases (nodespace database list). Over the socket, DatabaseService.List returned the typed binding fields in their own fields and extensions holding only the string keys ({"plugin_state":"keep"}, {"plugin_label":"blue"}); the integer and the table stayed out. After nodespace database rename on both entries, the file still held every key (typed fields, plugin_count, plugin_state, [databases.extra]), with the values unchanged. On the first save, extension keys are written after the typed ones in key order and arrays are laid out by the pretty serializer.

Ratchet

bun run scripts/check-pro-boundary.ts exits 0. BASELINES is not touched, per the standing decision (parallel PRs collide on those lines; the final issue tightens them), so the issue's step 3 is not applied. Counts against the ceilings: cloudBindState 203 -> 202 and tenantWording 463 -> 453; no count rose.

Deviations from the issue

• Paths: the desktop DTO lives in app-lib/src/commands/database.rs after the crate split.
• BASELINES left alone (above).
• One test beyond the issue's list, a_recorded_binding_adds_no_marker (above), so deleting the label is guarded by something that fails if it returns.
• The CLI's info_to_json is unchanged: it never printed the binding fields and the issue does not ask for extensions there.

🤖 Generated with Claude Code

[mstomar125]

Code Review Summary

Review Type: Initial Review (no prior reviews; head e3d69028)

Small, well-scoped additive change. I traced the serde/toml behaviour, every registry mutation path, the proto/DTO/frontend plumbing, the tray doc against database_menu_entries, and the ratchet, and found no correctness, security or standards problems. Recommendation: APPROVE, with one forward-looking nit for the follow-up issue. I did not run cargo or the test suite (per reviewer constraints); I relied on the author's reported green test:changed run and re-ran only bun run scripts/check-pro-boundary.ts (exit 0).

Requirements Check (issue #3341)

• An existing databases.toml loads with unknown entry keys, and every such key survives every save. Load goes through Registry::load; every mutator (rename, set_bound_tenant, set_default, remove, ...) clones the registry and edits the entry in place inside mutate_and_save, so extensions rides along. The only three DatabaseEntry { .. } literals in non-test code (database_manager.rs ~776, ~862, ~1169) build brand-new entries and set extensions: toml::Table::new(). No place rebuilds an existing entry.
• ListDatabases carries extensions (strings only) through proto, DTO and frontend type. listing_to_info is the only proto DatabaseInfo construction site in the workspace.
• No core code reads a specific key. The only consumers of .extensions are string_extensions (iterates all string entries), to_entry (copies), and tests.
• Tray shows no "synced" label; the rewritten doc matches database_menu_entries (default flag, live open marker, missing replaces both). The "rebuilt on every registry/open-set change" claim is true: spawn_tray_database_sync in main.rs re-lists on every subscribe_changes wake, and notify_changed is called from mutate_and_save and the open/close paths.
• Free-user UI: no component changed; only the store type, implicit-browser default and four fixtures.
• [~] Ratchet: check-pro-boundary.ts exits 0 and no count rose, but BASELINES is intentionally not set to the printed counts (issue step 3 / AC bullet 6). This matches CLAUDE.md's "Pro / Sync Boundary" text (baselines are ceilings; a lower count does not fail and "tightens when convenient"), and the current printout confirms the collision rationale (several other markers, e.g. proCommands 271 vs 275 and editionBranching 104 vs 113, are already below their ceilings from other PRs). Accepted deviation, not a defect.
• Build/tests/svelte-check: author reports 4120 workspace + 257 app-lib tests, clippy, fmt, svelte-check 0 errors. Not re-run here.

Verification of the areas called out

1. serde flatten + toml 0.8.2 (checked against the code, no regression found).
   • Deserialize: the derive visitor matches known keys to their typed fields and deserializes them directly. Only unmatched keys go to the flatten buffer. So DateTime<Utc>, PathBuf, the DatabaseId newtype and the #[serde(default)] Options behave as before, and a typed key cannot also land in extensions. toml::Value can represent every TOML value (including native datetimes and arrays of tables), so a previously-loadable file cannot start failing on an unknown key. The test asserts extensions.len() == 3 and is_datetime().
   • Serialize: flatten forces serialize_map; toml_edit's map serializer skips UnsupportedNone, so None fields still vanish (same bytes as before for existing files). toml::Table is a sorted map and the document formatter emits scalars before sub-tables, so a table that sorts before a scalar is fine; the test covers exactly that plus the native datetime, and asserts a second save is byte-identical.
   • Old typed binding fields stay typed and readable: set_bound_tenant_mirrors_persists_and_clears now asserts extensions is empty after a bind, and a_recorded_binding_adds_no_marker loads a file with the key and asserts it did not land in extensions.
2. Mutation paths: covered above; a_rename_keeps_extension_keys exercises the real DatabaseManager::rename path (including a Missing-status entry, which is fine for rename).
3. Proto: map<string,string> extensions = 10 is wire-additive; fields 8 and 9 untouched. string_extensions uses value.as_str()?, so integers, booleans, arrays, tables and native datetimes are skipped, never stringified.
4. Tray: verified above. Test helpers dropped their tenant param cleanly; markers_render_in_a_stable_order now expects All — default · open.
5. Tests are real guards. I checked each against the code it protects. a_recorded_binding_adds_no_marker fails if synced returns (label becomes Plain — synced). Its key is built from fragments, which is correct for an absence test under the ratchet rule (fragments are used in the test only, none in product code). extension_keys_add_no_marker is a weaker guard by nature (it only catches a future tray reading extension keys) but is what the issue specifies. The reasoning in the mutation table holds, including that flatten -> skip is caught by the mgr.list()...extensions.len() == 3 assertion rather than the on-disk equality.
6. skip_serializing_if on the three Option fields: agreed that it is a no-op on toml 0.8.2 (the struct path and the flattened map path both skip None). Keep it: it is what the issue specifies, costs nothing, and removes a dependence on a serializer detail that a toml upgrade could change. No test can distinguish it today, so no test should be added.
7. Frontend: the required extensions: Record<string, string> is satisfied at every construction site. git grep lastOpenedAt -- packages finds only the store, the implicit-browser default and the four fixtures, all updated. No dev-tools/mock constructs DatabaseInfo. Nothing here should break svelte-check.
8. Standards: no #NNNN in added lines, no #[allow(dead_code)], no deprecated/compat/migration code, no new Pro wording (added lines mention tenant only in the required bound_tenant_*: None/boundTenantCollection: null fixture literals that stay until the typed fields are removed). The DTO serializes camelCase and BTreeMap gives deterministic JSON key order.

Code Review Findings

Critical: none.

Important: none.

Suggestion (non-blocking, for the follow-up that adds set_extensions)

File: packages/daemon/src/services/database_manager.rs:107-110 (DatabaseEntry.extensions)

Because flatten writes typed fields and the extension map into the same TOML table, nothing stops a future set_extensions from inserting a key that collides with a typed key (id, name, path, created_at, last_opened_at, bound_*). That would either overwrite the typed value on save or make the save fail, and on the next load the typed field would read the extension's value. Nothing can do this today (only tests insert into the map), so it is not a defect in this PR. When the setter lands, have it reject the registry's own field names.
Principle: enforce the invariant "extensions holds only keys the registry does not define" at the write boundary, since the load path already guarantees it on the read side.

Nits

• Nit: none worth raising. The proto comment says "copied verbatim" and then "Only string values are carried"; that is the issue's own wording and reads fine.

Recommendation: APPROVE

[mstomar125]

Automated pragmatic-code-review: APPROVE (posted as a comment, GitHub blocks self-review states). Full findings in the review comment above.

[mstomar125]

Address-review summary

Reviewed: the pragmatic review of head e3d69028 (verdict APPROVE, 0 critical, 0 important, 0 nits, 1 suggestion).

Addressed: 0. Skipped: 1, deferred to the issue that owns it.

• Skipped (suggestion, DatabaseEntry.extensions in database_manager.rs): a future set_extensions could insert a key that collides with a typed key (id, name, path, created_at, last_opened_at, bound_*). This PR has no setter: nothing writes to the map outside tests, and the load path already guarantees the map only holds keys the registry does not define. The issue lists set_extensions as out of scope, so the guard belongs where the setter lands. Revisit condition: when set_extensions is added, it should reject the registry's own field names and carry a test for that.

No code changed, no commits, no push. Head stays e3d69028.

Re-Review Decision

Decision: NO RE-REVIEW NEEDED

Rationale: the review raised no defect and nothing was changed after it. The head the reviewer saw is the head the PR still has.