Remove the replicated-apply client id switch from core (post-release) #3435

Activity

1. mstomar125

   self-assigned this

   on Oct 1, 2026
2. mstomar125

   added a parent issue on Oct 1, 2026

   • Pro rebuild (after core release): finish core's extension points for typed subtypes, edge fields and Pro UI (ADR-082) #3414
3. mstomar125 commented on Oct 12, 2026

   

   mstomar125

   on Oct 12, 2026

   ContributorAuthor

   More actions

   Design proposal (investigation only, nothing implemented; core is mid-release)

   Checked against core origin/main 2bf95fea6 and nodespace-sync origin/main 334349c. Sync's core pin is 4f29efea3 (an ancestor of core main, 19 commits behind).

   Where the switch is used today (line numbers in the issue have drifted)

   • packages/core/src/db/events.rs:285: REPLICATED_APPLY_CLIENT_ID = "sync-service", plus a test that pins its value because "writers outside core" set the literal.
   • packages/core/src/playbook/engine.rs:1597 is_replicated_apply(&EventEnvelope), used at :596: skips trigger evaluation and runs dispatch_invariant_repair (ADR-060 §7 repair-and-log) instead.
   • packages/core/src/services/node_service/invariants.rs:131 (invariant_rules_for_creation), :277 (run_invariant_rules_for_update), :335 (dispatch_invariant_rules_for_relationship_in_tx): self.client_id == REPLICATED_APPLY_CLIENT_ID returns no rules. Bulk paths go through invariant_rules_for_creation.
   • Tests that depend on it: playbook/tests.rs (3 is_replicated_apply tests), tests/it/playbook_engine_integration_test.rs, playbook_invariant_integration_test.rs, bulk_invariant_dispatch_test.rs.
   • Daemon: packages/daemon/src/services/node_service.rs route() calls with_client(<x-ns-client-id header>) for any value; client_id_header only checks ASCII.

   Where Sync sets it

   Sync does not go through the gRPC header. nodespaced-sync is an in-process composition over core's library, so it calls NodeService::with_client("sync-service") directly: cloud_sync/orchestrator.rs:109 (the apply service; the other cloud_sync/* files take that service), status.rs:257/415, and about 40 literal with_client("sync-service") calls across apply.rs, identity.rs, reconnect.rs, conflicts.rs, status.rs (mostly tests), plus examples/t6/t9*. Sync's own constant nodespace_sync::echo::SYNC_SERVICE_CLIENT_ID (src/echo.rs:59) is used for echo prevention: EchoGuard::is_own_event, set_excluded_event_origin(...) in orchestrator.rs:121 and status.rs:1191, and the push consumer's L1 skip (src/push/supabase.rs). That use is generic in core (set_excluded_event_origin takes any string) and needs no change.

   So the string does two separate jobs: (a) echo prevention, which is Sync's business and already generic; (b) a core behaviour switch (skip local rules, repair instead), which is what the issue is about.

   Option 2 (move the gate into Sync) does not work on its own

   The branches are inside core's NodeService write paths (pre-commit invariant dispatch inside the transaction) and the engine's event loop (repair-and-log). Sync cannot remove them without core exposing a hook, so Option 2 collapses into Option 1. The semantics themselves (ADR-073 local-origin gate, ADR-060 §7) are not Pro semantics: any second writer applying already-committed data needs them.

   Recommendation: Option 1 with a typed, in-process marker instead of a magic string

   1. Core adds a neutral concept, "replicated apply" (a write whose effects were already committed elsewhere), carried as a typed field rather than a client-id value:
      • EventMetadata gets replicated: bool (or origin: WriteOrigin::{Local, Replicated}), filled from the NodeService that wrote it (events are built from self metadata at node_service/mod.rs ~523-532).
      • NodeService::as_replicated_apply(&self, client_id: impl Into<String>) -> Self (name to be settled in the ADR) sets both the client id (echo tag unchanged) and a private replicated flag. There is no gRPC or header path to set it.
      • is_replicated_apply reads envelope.metadata.replicated; the three invariants.rs guards read self.replicated. REPLICATED_APPLY_CLIENT_ID and its value-pin test are deleted.
   2. Security: because the flag is not derivable from a header, a CLI or agent can no longer switch off rules or invariants by sending x-ns-client-id: sync-service. The second half of the issue's "at minimum" remains: a client claiming Sync's echo id would have its events dropped from the push channel (an availability problem, not an integrity one). Add DaemonExtensions::reserved_client_ids(...) (ADR-082 §5 row); route() answers permission_denied for a header value in that set. Sync registers "sync-service". Core's default set is empty, so core stays free of the string.
   3. Behavioural equivalence: events keep source_client_id = Some("sync-service") (Sync still passes its id), so watch_nodes and set_excluded_event_origin L1 skipping is byte-for-byte the same. Playbook and invariant behaviour is the same for in-process Sync writes; the only behaviour change is that header-supplied ids stop having any effect on rules.
   4. Versioning: ADR-082 §8 was amended on 2026-10-05, so there is no EXTENSION_API_VERSION to bump; the change reaches Sync through its core pin. The docs-first step is an ADR-082 §5 row (replicated-apply marker, reserved client ids) and an architecture/extension-points.md entry, no ticket numbers in the docs.

   Change list

   Core (one PR, after v0.3.5 is out):

   • db/events.rs: remove the constant and its test; add the replicated field to EventMetadata (all constructors and PartialEq users).
   • services/node_service/mod.rs: replicated flag on NodeService, as_replicated_apply, propagate into emitted metadata (single and bulk paths, transaction paths).
   • playbook/engine.rs: is_replicated_apply reads the field. invariants.rs: three guards read self.replicated.
   • packages/daemon/src/services/extensions.rs and node_service.rs route(): reserved client ids.
   • Docs: rewrite the comments that name the constant (engine.rs ~585, events.rs, invariants.rs ~94).

   Sync (one PR, in the same pin bump as the core PR):

   • Cargo.toml / nodespaced-sync/Cargo.toml: all nodespace-* pins to the new core rev (one rev, all crates).
   • Replace with_client("sync-service") with as_replicated_apply(SYNC_SERVICE_CLIENT_ID) on every apply-side service (orchestrator.rs:109, status.rs:257/415, apply.rs, identity.rs, reconnect.rs, conflicts.rs, examples, tests), keeping the literal out of the call sites.
   • Register reserved_client_ids([SYNC_SERVICE_CLIENT_ID]) where nodespaced-sync/src/main.rs builds DaemonExtensions.
   • Remove the stale reference entry in reference/core-sync-origin/INDEX.md if it names the old constant.

   Tests

   • Core unit: a replicated-marked write skips triggers and the three invariant paths, and a plain with_client("sync-service") no longer does (mutation check: this is the regression for the old string).
   • Core: the existing playbook/invariant/bulk integration tests move from with_client(CONST) to as_replicated_apply(..); repair-and-log still runs for received creates and updates; the marker survives bulk and transaction paths.
   • Daemon: a header with a reserved id is refused; a header sync-service with no registered reservation is only an echo tag and does not skip rules.
   • Sync: an end-to-end apply of a pulled node with a reactive play and an invariant installed (no firing, repair runs), the L1 push skip still holds, and the daemon refuses a client claiming sync-service.

   Order

   1. Docs: ADR-082 §5 row and extension-points entry.
   2. Core PR (after the v0.3.5 release), merged, new rev.
   3. Sync PR with the pin bump and all call-site changes together. They must land as one change: a bump alone would make Sync's apply writes fire local rules once the old constant is gone.

   This is coordinated with #3414 (closed; its mechanisms are on main) and nodespace-sync#518.

   🤖 Generated with Claude Code
4. mstomar125 commented on Oct 12, 2026

   

   mstomar125

   on Oct 12, 2026

   ContributorAuthor

   More actions

   Picking this up now (user decision 2026-10-12: land it in core v0.3.5). Design: a neutral in-process typed write context on NodeService (skip playbook triggers / skip invariant rules), carried in event metadata, not settable from any wire field; plus DaemonExtensions::reserved_client_ids (empty by default). REPLICATED_APPLY_CLIENT_ID and is_replicated_apply get deleted. Comment here if this is already in progress elsewhere.

   🤖 Generated with Claude Code
5. mstomar125 commented on Oct 12, 2026

   

   mstomar125

   on Oct 12, 2026

   ContributorAuthor

   More actions

   Frontend baseline (worktree from origin/main 2bf95fe): 324 files / 5895 tests passed.

   🤖 Generated with Claude Code
6. mstomar125

   added 2 commits that reference this issue on Oct 12, 2026

   Add a typed in-process write context and reserved client ids (refs #3435

   ...

   a0cf004

   Add a typed in-process write context and reserved client ids (refs #3435

   ...

   1b64164
7. mstomar125 commented on Oct 12, 2026

   

   mstomar125

   on Oct 12, 2026

   ContributorAuthor

   More actions

   Phase 1 merged as #3752 (1b64164): typed in-process WriteContext via NodeService::with_write_context, plus DaemonExtensions::reserved_client_id. It is additive; the old id still behaves as before. Docs are on nodespace-docs main (e22a839): ADR-082 §5 rows and the extension-points guide. Phase 2, draft #3754, deletes REPLICATED_APPLY_CLIENT_ID/is_replicated_apply and closes this issue. It is held until Sync has moved to the typed context. Not in v0.3.5; this ships in the next core release.

   🤖 Generated with Claude Code
8. mstomar125

   added a commit that references this issue on Oct 12, 2026

   Remove the superseded replicated-apply client id (closes #3435)

   ...

   This issue will close once commit b77b288 is merged into the 'main' branch.

   b77b288
9. mstomar125

   closed this as completedin #3754on Oct 12, 2026
10. mstomar125 commented on Oct 12, 2026

    

    mstomar125

    on Oct 12, 2026

    ContributorAuthor

    More actions

    Done. Core no longer has a client-id string that changes trigger or invariant behaviour. The issue closed automatically when #3754 merged; this comment records the evidence.

    What landed

    • Add a typed in-process write context and reserved client ids (refs #3435) #3752 (phase 1, main 1b6416482) adds a typed, in-process WriteContext { skip_playbook_triggers, skip_invariant_rules }, set only through NodeService::with_write_context. No request field or header can set it. It also adds DaemonExtensions::reserved_client_id: a request presenting a reserved id gets PERMISSION_DENIED. Core reserves no id.
    • NodeSpaceAI/nodespace-sync#679 (Sync main 2b25acf): Sync's apply handle is built in one place (echo::replicated_apply_service) with both flags, and Sync's daemon reserves its own client id.
    • Remove the superseded replicated-apply client id (closes #3435) #3754 (phase 2, main af46b30f9) deletes REPLICATED_APPLY_CLIENT_ID, is_replicated_apply, write_context_of and the client-id comparison in skips_invariant_rules, and removes the last incidental sync-service strings from core. A client id is now only an echo-suppression label.
    • Docs (nodespace-docs e22a839): ADR-082 §5 gains the "Write context" and "Reserved client ids" rows with an amendment note, and architecture/extension-points.md documents both as generic extension points.

    Acceptance criteria

    • Core has no client-id string that changes trigger or invariant behaviour. git grep on main af46b30f9 finds no REPLICATED_APPLY_CLIENT_ID, is_replicated_apply, write_context_of or sync-service. The only matches are the unrelated browser-sync-service module name.
    • Tests cover the design. Typed-context tests cover the engine trigger gate, the invariant skips (create, update, relationship, bulk) and repair-and-log. A reserved id is refused on writes and WatchNodes. The tests showing the old value no longer skips run at the core, daemon and fixture levels. Mutations that restore the old comparison are killed. The full merge gate passed on the PR stacked onto main.
    • Coordinated with Pro rebuild (after core release): finish core's extension points for typed subtypes, edge fields and Pro UI (ADR-082) #3414 and Sync. Sync adopted the typed context before the deletion merged.

    Sync not broken, checked before merging and again after

    • Local canary, Sync main 2b25acf against the Remove the superseded replicated-apply client id (closes #3435) #3754 head c361cb5fe: canary OK. All 9 checks passed (lock alignment, cargo check --workspace --all-targets, tauri config, app crate tests, frontend build, svelte-check, Sync frontend tests, import boundary, mirrors).
    • Against the same core:
      • tests/replicated_apply_rules.rs: 4/4
      • sync_client_id_source_guard: 5/5
      • daemon sync_settings::: 12/12, including a_request_presenting_the_sync_client_id_is_refused
      • full Sync workspace: 1101 passed, 0 failed, 15 ignored
    • After merge, the hosted Core canary run 38225766715 (Sync 2b25acf against core main af46b30f9): success, same 9 checks.
    • Independent review of Remove the superseded replicated-apply client id (closes #3435) #3754: APPROVE, no blocker.

    Follow-ups, outside this issue

    • NodeSpaceAI/nodespace-sync#681 hardens Sync's source guard. It currently matches only exact spellings, and its production-code scan stops at the first inline test module.
    • ADR-060 ("Remote-origin events are distinguishable") and ADR-073 (Decision, second bullet) still say the engine gates on the event's origin client tag. These are the open architect questions from Add a typed in-process write context and reserved client ids (refs #3435) #3752 and need an amendment decision.
    • Core main is now ahead of v0.3.5 with this change. It ships in the next core release; no release was cut.

    🤖 Generated with Claude Code