fix(cli): align observation_source enum with API + document v0.17→v0.18 breaking change (#1841) (#1851)

markmhendrickson · ateles-agent · claude · web-flow · commit c5c4394b0e99 · 2026-06-30T18:29:01.000+02:00
* fix(cli): align observation_source enum with API + document v0.17→v0.18 breaking change (#1841) `observation_source` was tightened from arbitrary strings (v0.17) to a closed enum in v0.18.0, but the change shipped undocumented and the CLI enum drifted from the API. - CLI/API parity: add the missing `sync` value to OBSERVATION_SOURCE_CLI_VALUES so the CLI accepts exactly what the API and openapi.yaml accept (sensor, llm_summary, workflow_state, human, import, sync). - Improve the "Invalid --observation-source" error to list the valid values and tell users to put custom v0.17 labels in the free-form `data_source` field. - Update all three `--observation-source` CLI help strings (store, ingest, snapshots export) to include `sync` + a migration note. - Docs: CLI reference enum + migration link; new "Migrating observation_source from v0.17 to v0.18" subsection in fleet_onboarding.md with a custom-string → enum + data_source mapping table. - Release: document the retroactive breaking change in the v0.18.3 release supplement (Breaking changes + Fixes #1841). - Mark the enum as an intentional v0.17 breaking change in action_schemas.ts. - Test: tests/unit/cli_observation_source_flag.test.ts asserts CLI↔API enum parity, that `sync` is accepted, and that an invalid value's error carries the data_source migration hint. No back-compat shim (operator verdict P2: document, don't widen). Closes #1841 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(catalog): regenerate automated test catalog for #1841 enum/migration tests Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: ateles-agent <agent@ateles.ai> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
diff --git a/docs/developer/cli_reference.md b/docs/developer/cli_reference.md
@@ -693,6 +693,7 @@ Cross-instance peer sync. Backed by `src/services/sync/` and the HTTP `/peers` s
     - Use `--json=` (equals, no space) so the payload is parsed as entities input.
     - Bare `--json` (without `=`) remains the global output-format flag.
   - `--file <path>`: Path to JSON file containing entity array. Use for long payloads.
+  - `--observation-source <kind>`: Classify the *kind* of write. Closed enum — `sensor | llm_summary | workflow_state | human | import | sync` (default `llm_summary`). **Breaking change in v0.18.x (was open in v0.17):** arbitrary strings are rejected. Put custom v0.17 labels (e.g. `cboe_live`, `stale_cache`) in the free-form `data_source` field and map the closest enum value here. See [Migrating from v0.17 to v0.18](fleet_onboarding.md#migrating-observation_source-from-v017-to-v018).
   - `--interpretation-source-id <id>` / `--interpretation-source-ref <structured|unstructured>` plus `--interpretation-config <json>`: Opt into Source -> Interpretation -> Observation provenance for source-derived extraction. Omit for ordinary already-structured/chat-native facts.
   - Silent-failure guard (v0.5.1+): when a commit-mode store returns `entities_created=0`, no resolved entities, and is not an idempotency replay (`replayed: true`), the CLI emits a non-fatal stderr warning so agents/humans don't mistake an empty result for success. Typical root cause: fields nested under `attributes` (see v0.5.0 breaking change) or mismatched `user_id` scope.
   - Idempotency replays: `POST /store` returns `replayed: true` when a request is a deterministic re-commit of a previously committed `(user_id, idempotency_key)` tuple. Use this to distinguish "same call, no new work" from "call produced zero entities."
@@ -951,7 +952,7 @@ Fleet-general write-integrity tooling built on the canonical snapshot layer. `sn
   - `--entity-types <csv>`: Restrict to comma-separated entity types (e.g. `agent_task,agent_attempt`).
   - `--agent-sub <sub>`: Restrict to a single AAuth `agent_sub`.
   - `--attribution-tier <tier>`: Restrict to one tier — `hardware | software | unverified_client | anonymous`.
-  - `--observation-source <kind>`: Restrict to one write kind — `sensor | llm_summary | workflow_state | human | import`.
+  - `--observation-source <kind>`: Restrict to one write kind — `sensor | llm_summary | workflow_state | human | import | sync`.
   - `--since <iso>`: ISO-8601 lower bound on `last_observation_at`.
   - `--limit <n>`: Cap on returned entities (default: 500).
   - `--out <path>`: Write the export to this file instead of stdout.
diff --git a/docs/developer/fleet_onboarding.md b/docs/developer/fleet_onboarding.md
@@ -41,6 +41,9 @@ Set `observation_source` on every structured write. The value is orthogonal to A
 | `llm_summary`    | LLM-authored summaries / extractions. **Default** when `observation_source` is unspecified.       |
 | `human`          | Direct human edits, acceptances, corrections.                                                     |
 | `import`         | Batch / ETL ingestion. Ranked last by default because provenance is typically second-hand.        |
+| `sync`           | Writes replayed from a peer during fleet sync (Phase 5 loop prevention). Set automatically by the sync path; rarely supplied by hand. |
+
+`sensor`, `llm_summary`, `workflow_state`, `human`, `import`, and `sync` are the **only** accepted values — this is a closed enum (see the v0.17→v0.18 migration note below).
 
 Reducer tie-breaking proceeds `source_priority → observation_source_priority → observed_at`. Per-schema overrides (e.g. `agent_sensor_signal` locks `observation_source_priority` so sensor rows always win) live on `SchemaDefinition.reducer_config.observation_source_priority`.
 
@@ -69,6 +72,26 @@ neotoma store \
 
 See the applied rules in [`.cursor/rules/neotoma_cli.mdc`](../../.cursor/rules/neotoma_cli.mdc) (rule 14a) or [`docs/developer/mcp/instructions.md`](./mcp/instructions.md) for the agent-instruction wording.
 
+### Migrating `observation_source` from v0.17 to v0.18
+
+**Breaking change (v0.18.x, [#1841](https://github.com/markmhendrickson/neotoma/issues/1841)).** In v0.17, `observation_source` (and the `--observation-source` CLI flag) accepted arbitrary strings — e.g. `cboe_live`, `stale_cache`, `computed_greeks`. v0.18 restricts it to the closed enum above. Stores and `--observation-source` flags carrying a custom string now fail with:
+
+```
+Invalid --observation-source "cboe_live". Expected one of: sensor, llm_summary, workflow_state, human, import, sync. Custom v0.17 observation_source labels are no longer accepted — put them in the free-form "data_source" field instead.
+```
+
+`observation_source` is a *fixed classification* of the kind of write (it drives reducer tie-breaking); the descriptive label of *where* a value came from belongs in the free-form `data_source` field, which remains unconstrained. To migrate, map each custom string to the closest enum value and move the original label into `data_source`:
+
+| v0.17 custom string (examples) | v0.18 `observation_source` | `data_source` (free-form) |
+| ------------------------------ | -------------------------- | ------------------------- |
+| `cboe_live`, `*_live`, market/exchange feeds, webhooks, telemetry | `sensor` | `cboe_live` |
+| `computed_greeks`, derived/calculated state, state-machine output | `workflow_state` | `computed_greeks` |
+| `gpt_summary`, model-authored extractions / summaries | `llm_summary` | `gpt_summary` |
+| `analyst_note`, manual edits / acceptances | `human` | `analyst_note` |
+| `stale_cache`, `nightly_etl`, batch / backfill ingestion | `import` | `stale_cache` |
+
+If no enum value fits the *kind* of write, default to `import` (second-hand provenance) or `llm_summary` (the system default) and preserve the original string in `data_source`. There is no back-compat shim — the enum is enforced at the parse boundary on both the CLI and the API.
+
 ## 3. Use the fleet-general `agent_*` schemas
 
 Neotoma ships a v0.1 "lowest common denominator" schema set in the `agent_runtime` category. Store runtime data against these types so your drift reports, timeline views, and Inspector filters work without fleet-specific migrations:
diff --git a/docs/releases/in_progress/v0.18.3/github_release_supplement.md b/docs/releases/in_progress/v0.18.3/github_release_supplement.md
@@ -37,6 +37,7 @@ No OpenAPI changes. `source_storage` is an existing `StoreRequest` field; this r
 - **#1826** — `source_storage: "reference"` was unreachable over HTTP `POST /store`. The OpenAPI contract accepted it (v0.18.1) but `handleStorePost` still stored inline. The REST handler now honors reference mode end to end.
 - **#1827 (REST extension)** — the combined `entities[] + file + source_storage:reference` call returned a 500 on `observations.id` UNIQUE collision because the REST structured leg lacked the existing-observation guard that the MCP path has. Brought to parity.
 - **#1837** — the Inspector graph viewer fired `retrieve_graph_neighborhood` in an unbounded loop and never painted. Root cause was a global `refetchInterval: 5000`, not unbounded traversal. Polling disabled on the graph hooks; canvas height fixed.
+- **#1841** — the v0.18.0 `observation_source` enum restriction was undocumented, and the CLI enum had drifted from the API (missing `sync`). Documented the breaking change + migration mapping, aligned the CLI enum and error message to the API, no back-compat shim. See *Breaking changes* above.
 
 ## Tests and validation
 
@@ -52,4 +53,16 @@ Security review artifact: [docs/releases/in_progress/v0.18.3/security_review.md]
 
 ## Breaking changes
 
-No breaking changes. No OpenAPI tightening, no request-shape changes, no field promotions or removals. Both behavior changes (reference stores honored, duplicate observations replayed) are strict improvements aligning REST with MCP. Patch bump is correct per SemVer.
+This patch introduces no *new* breaking changes. No OpenAPI tightening, no request-shape changes, no field promotions or removals. Both behavior changes (reference stores honored, duplicate observations replayed) are strict improvements aligning REST with MCP. Patch bump is correct per SemVer.
+
+### Retroactively documented: `observation_source` closed enum (since v0.18.0)
+
+[#1841](https://github.com/markmhendrickson/neotoma/issues/1841) surfaced a v0.18.0 breaking change that shipped **undocumented**: `observation_source` was tightened from arbitrary strings (v0.17) to a closed enum — `sensor`, `llm_summary`, `workflow_state`, `human`, `import`, `sync`. Stores carrying a custom string (e.g. `cboe_live`, `stale_cache`) now hard-fail at the parse boundary on both the CLI and the API. There is no back-compat shim (operator verdict P2: document, don't widen).
+
+This release does **not** change that enforcement; it documents and de-drifts it:
+
+- The CLI `--observation-source` enum now matches the API/OpenAPI exactly (added the missing `sync`); previously the CLI rejected a value the API accepts.
+- The invalid-value error message now tells users to put custom v0.17 labels in the free-form `data_source` field.
+- A migration guide (custom v0.17 strings → closest enum + `data_source`) was added to [`docs/developer/fleet_onboarding.md`](../../developer/fleet_onboarding.md#migrating-observation_source-from-v017-to-v018) and the [CLI reference](../../developer/cli_reference.md).
+
+**If you upgraded from v0.17 and used custom `observation_source` strings:** map each to the closest enum value and move the original label to `data_source`. See the migration guide for the mapping table.
diff --git a/docs/testing/automated_test_catalog.md b/docs/testing/automated_test_catalog.md
@@ -69,7 +69,7 @@ flowchart TD
 ### Suite counts
 | Suite | Files |
 |---|---:|
-| Vitest unit tests | 145 |
+| Vitest unit tests | 146 |
 | Vitest service tests | 35 |
 | Source-adjacent tests | 64 |
 | Vitest integration tests | 148 |
@@ -109,7 +109,7 @@ flowchart TD
 **Runner:** `vitest`
 **Command:** `npm test -- tests/unit`
 **Requirements:** Basic `.env` if required by the module under test.
-**Files (145):**
+**Files (146):**
 - `tests/unit/aauth_admission.test.ts`
 - `tests/unit/aauth_attestation_apple_se.test.ts`
 - `tests/unit/aauth_attestation_revocation.test.ts`
@@ -142,6 +142,7 @@ flowchart TD
 - `tests/unit/cli_aauth_tpm2_attestation.test.ts`
 - `tests/unit/cli_aauth_yubikey_attestation.test.ts`
 - `tests/unit/cli_bug_fixes.test.ts`
+- `tests/unit/cli_observation_source_flag.test.ts`
 - `tests/unit/cli_schema_register_reducer_config.test.ts`
 - `tests/unit/client_diagnose.test.ts`
 - `tests/unit/client_helpers.test.ts`
diff --git a/src/cli/index.ts b/src/cli/index.ts
@@ -132,21 +132,29 @@ function inferMimeTypeForPath(filePath: string): string {
   return getMimeTypeFromExtension(ext) ?? "application/octet-stream";
 }
 
-const OBSERVATION_SOURCE_CLI_VALUES = [
+// Must stay in lockstep with OBSERVATION_SOURCE_VALUES in
+// src/shared/action_schemas.ts and the `observation_source` enum in
+// openapi.yaml. `sync` is API-accepted (Phase 5 peer-sync loop prevention),
+// so the CLI must accept it too. This enum was tightened from arbitrary
+// strings in v0.17 — an intentional breaking change (see issue #1841);
+// custom v0.17 labels now belong in the free-form `data_source` field.
+export const OBSERVATION_SOURCE_CLI_VALUES = [
   "sensor",
   "llm_summary",
   "workflow_state",
   "human",
   "import",
+  "sync",
 ] as const;
 
 type ObservationSourceFlag = (typeof OBSERVATION_SOURCE_CLI_VALUES)[number];
 
-function normalizeObservationSourceFlag(raw: string): ObservationSourceFlag {
+export function normalizeObservationSourceFlag(raw: string): ObservationSourceFlag {
   const normalized = raw.trim().toLowerCase().replace(/-/g, "_");
   if (!(OBSERVATION_SOURCE_CLI_VALUES as readonly string[]).includes(normalized)) {
     throw new Error(
-      `Invalid --observation-source "${raw}". Expected one of: ${OBSERVATION_SOURCE_CLI_VALUES.join(", ")}.`
+      `Invalid --observation-source "${raw}". Expected one of: ${OBSERVATION_SOURCE_CLI_VALUES.join(", ")}. ` +
+        `Custom v0.17 observation_source labels are no longer accepted — put them in the free-form "data_source" field instead.`
     );
   }
   return normalized as ObservationSourceFlag;
@@ -14281,7 +14289,7 @@ program
   )
   .option(
     "--observation-source <kind>",
-    "Kind of write: sensor, llm_summary (default), workflow_state, human, or import"
+    "Kind of write: sensor, llm_summary (default), workflow_state, human, import, or sync (custom v0.17 values → use data_source)"
   )
   .option("--idempotency-key <key>", "Idempotency key to prevent duplicate stores")
   .option(
@@ -14534,7 +14542,7 @@ program
   )
   .option(
     "--observation-source <kind>",
-    "Kind of write: sensor, llm_summary (default), workflow_state, human, or import"
+    "Kind of write: sensor, llm_summary (default), workflow_state, human, import, or sync (custom v0.17 values → use data_source)"
   )
   .option(
     "--source-upload",
@@ -15758,7 +15766,7 @@ snapshotsCommand
   )
   .option(
     "--observation-source <kind>",
-    "Restrict to one write kind: sensor | llm_summary | workflow_state | human | import"
+    "Restrict to one write kind: sensor | llm_summary | workflow_state | human | import | sync (custom v0.17 values → use data_source)"
   )
   .option("--since <iso>", "ISO-8601 lower bound on last_observation_at")
   .option("--limit <n>", "Cap on returned entities (default: 500)")
diff --git a/src/shared/action_schemas.ts b/src/shared/action_schemas.ts
@@ -439,6 +439,12 @@ export const ObservationsQueryRequestSchema = z.object({
  * is applied by the write path, not at parse time, so MCP callers that
  * omit the field remain LLM-driven by construction without breaking
  * idempotency hashes or contract-test fixtures.
+ *
+ * NOTE (v0.17 → v0.18 breaking change, issue #1841): this is a CLOSED enum.
+ * v0.17 accepted arbitrary `observation_source` strings; v0.18 restricts them
+ * to the values below — an intentional breaking change, not a regression.
+ * Custom v0.17 labels (e.g. `cboe_live`, `stale_cache`) now belong in the
+ * free-form `data_source` field; map the closest enum value here.
  */
 export const OBSERVATION_SOURCE_VALUES = [
   "sensor",
diff --git a/tests/unit/cli_observation_source_flag.test.ts b/tests/unit/cli_observation_source_flag.test.ts
@@ -0,0 +1,65 @@
+/**
+ * Unit tests for the CLI `--observation-source` flag validator
+ * (`normalizeObservationSourceFlag` in src/cli/index.ts).
+ *
+ * Issue #1841: `observation_source` was tightened from arbitrary strings
+ * (v0.17) to a closed enum (v0.18.x). This test pins:
+ *   1. The CLI enum matches the API/OpenAPI enum EXACTLY, including `sync`
+ *      (the value that previously drifted — CLI rejected it, API accepted it).
+ *   2. An invalid value's error message lists the valid values AND tells the
+ *      user to put custom v0.17 labels in the `data_source` field.
+ */
+
+import { describe, expect, it } from "vitest";
+
+import {
+  OBSERVATION_SOURCE_CLI_VALUES,
+  normalizeObservationSourceFlag,
+} from "../../src/cli/index.ts";
+import { OBSERVATION_SOURCE_VALUES } from "../../src/shared/action_schemas.ts";
+
+describe("#1841 CLI --observation-source enum", () => {
+  it("CLI enum matches the canonical API/schema enum exactly", () => {
+    // The canonical enum lives in action_schemas.ts and openapi.yaml.
+    // CLI parity is the whole point of the fix — drift here re-breaks #1841.
+    expect([...OBSERVATION_SOURCE_CLI_VALUES].sort()).toEqual(
+      [...OBSERVATION_SOURCE_VALUES].sort()
+    );
+  });
+
+  it.each([...OBSERVATION_SOURCE_VALUES])(
+    "accepts the API-accepted value %s",
+    (value) => {
+      expect(normalizeObservationSourceFlag(value)).toBe(value);
+    }
+  );
+
+  it("now accepts `sync` (was the CLI/API drift in #1841)", () => {
+    expect(OBSERVATION_SOURCE_CLI_VALUES).toContain("sync");
+    expect(normalizeObservationSourceFlag("sync")).toBe("sync");
+  });
+
+  it("normalizes case and hyphens to the canonical underscore form", () => {
+    expect(normalizeObservationSourceFlag("LLM-SUMMARY")).toBe("llm_summary");
+    expect(normalizeObservationSourceFlag(" workflow-state ")).toBe(
+      "workflow_state"
+    );
+  });
+
+  it("rejects a custom v0.17 string with a data_source migration hint", () => {
+    let caught: Error | undefined;
+    try {
+      normalizeObservationSourceFlag("cboe_live");
+    } catch (err) {
+      caught = err as Error;
+    }
+    expect(caught).toBeDefined();
+    const message = caught!.message;
+    // Lists the value that was rejected and the valid values.
+    expect(message).toContain('"cboe_live"');
+    expect(message).toContain("sync");
+    expect(message).toContain("llm_summary");
+    // Tells the user where custom v0.17 labels now belong.
+    expect(message).toContain("data_source");
+  });
+});
