Skip to content

fix: correct idempotency cache scope in docs + smoke#18

Merged
chenliuyun merged 1 commit intomainfrom
fix/idempotency-doc-drift
Apr 20, 2026
Merged

fix: correct idempotency cache scope in docs + smoke#18
chenliuyun merged 1 commit intomainfrom
fix/idempotency-doc-drift

Conversation

@chenliuyun
Copy link
Copy Markdown
Collaborator

Summary

The second real-device smoke run (24649650541) surfaced a doc/implementation drift: src/lib/idempotency.ts is an in-memory Map — its header comment even says "Process-local only — not shared across replicas" — but CHANGELOG + capabilities.idempotencyContract.keyStorage both claimed "keys are SHA-256-hashed on disk". Agents reading capabilities would assume cross-CLI-invocation replay works. It does not: each fresh node process starts with an empty cache, so the `replayed:true` flag never lit up on the second smoke invocation.

Fix (docs match code; no behavior change)

  • CHANGELOG.md — replace the "on disk" line with accurate in-memory wording + enumerate the long-lived surfaces where replay/conflict actually apply (MCP session, `devices batch`, `plan run`, `history replay`).
  • `src/commands/capabilities.ts` — `idempotencyContract.keyStorage` now reads `"In-memory SHA-256 fingerprint; raw key never stored, no disk persistence."`. Added a new `scope` field spelling out the process-local constraint so agents don't have to infer it from the test suite.
  • `.github/workflows/smoke.yml` — drop the cross-invocation replay + conflict assertions (they can never pass against the real API with a process-local cache). Keep a single flag-accepted sanity check. The cross-process behavior itself is covered by `tests/lib/idempotency.test.ts`.

Follow-up (deferred, not in this PR)

If we want replay semantics to survive across shell CLI invocations, `IdempotencyCache` needs a disk-backed store (atomic write + TTL cleanup + concurrent-read safety). That's a real feature addition — 2.4.1 scope, not required for 2.4.0 correctness.

Test plan

  • `npm run build` clean
  • `npm test` — 832/832 (one test flakes under Windows parallel load but passes in isolation; unaffected by this change)
  • After merge, re-trigger `Smoke (real device)` with the same device IDs and confirm all steps green

src/lib/idempotency.ts is an in-memory Map ("Process-local only — not
shared across replicas"), but CHANGELOG and capabilities.idempotencyContract
both claimed "keys are SHA-256-hashed on disk". An agent reading the
capabilities payload would assume cross-invocation replay works and be
mismatched against reality — the second real-device smoke run
(24649650541) surfaced this: the first CLI call wrote into the in-memory
cache, the second CLI call started fresh, and the `replayed:true` flag
never appeared because each `node` process was starting from empty.

Fixes scope only (no behavior change):
- CHANGELOG: replace "SHA-256-hashed on disk" with accurate in-memory
  wording + enumerate which long-lived surfaces the cache covers (MCP,
  batch, plan run, history replay).
- capabilities.idempotencyContract: keyStorage no longer claims disk
  persistence; add `scope` field making the process-local constraint
  explicit for agents reading capabilities.
- smoke.yml: drop cross-invocation replay + conflict assertions (they
  can never pass given the process-local cache); keep a single flag-
  accepted check against the real API. Cross-process behavior is covered
  by tests/lib/idempotency.test.ts unit tests, which the smoke workflow
  is not the right layer to re-verify.

Follow-up (deferred): if we want replay to survive across shell CLI
invocations, add a disk-backed store to IdempotencyCache (2.4.1 scope).
That is a feature addition, not required for 2.4.0 correctness.
@chenliuyun chenliuyun merged commit 7af8ef5 into main Apr 20, 2026
4 checks passed
@chenliuyun chenliuyun deleted the fix/idempotency-doc-drift branch April 20, 2026 05:48
chenliuyun pushed a commit that referenced this pull request Apr 20, 2026
C1 (#14): update --idempotency-key / --idempotency-key-prefix help text
  in devices.ts and batch.ts to mention process-local scope, per-process
  cache semantics, and that independent CLI invocations do not share cache.

C2 (#15): mcp --help "eight tools" → "eleven tools"; list all 11 by name
  including get_device_history, query_device_history, aggregate_device_history.

C3 (#17): add `scenes describe <sceneId>` subcommand. Returns sceneId,
  sceneName, stepCount:null, and a note explaining v1.1 API limitation.
  Exits 2 with scene_not_found + candidate list on unknown sceneId.
  Adds 'scenes describe' to COMMAND_META in capabilities.ts.
  Adds 2 tests (known + unknown scene).

C4 (#13): add JSDoc comment on hints field in agent-bootstrap.ts clarifying
  empty array semantics. Add 'hints' field to cliAddedFields in schema.ts.

C5 (#16): create docs/verbose-redaction.md documenting masked headers
  (authorization, token, sign, nonce, x-api-key, cookie, set-cookie,
  x-auth-token, t) and the --trace-unsafe opt-out flag.

C6 (#18): plan schema output now includes agentNotes.deviceNameStrategy
  documenting that deviceName uses require-unique resolution and plans
  should pin deviceId for determinism.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
chenliuyun pushed a commit that referenced this pull request Apr 20, 2026
Document every fix landed in this branch beyond the history-aggregate
feature: bugs #1, #4, #5, #6, #8, #9, #10, #11, #12, #13, #14, #15,
#16, #17, #18 from the OpenClaw v2.4.0 smoke-test report. Call out
the deferred items (#2, #7) explicitly so readers don't assume they
were overlooked.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
chenliuyun pushed a commit that referenced this pull request Apr 20, 2026
C1 (#14): update --idempotency-key / --idempotency-key-prefix help text
  in devices.ts and batch.ts to mention process-local scope, per-process
  cache semantics, and that independent CLI invocations do not share cache.

C2 (#15): mcp --help "eight tools" → "eleven tools"; list all 11 by name
  including get_device_history, query_device_history, aggregate_device_history.

C3 (#17): add `scenes describe <sceneId>` subcommand. Returns sceneId,
  sceneName, stepCount:null, and a note explaining v1.1 API limitation.
  Exits 2 with scene_not_found + candidate list on unknown sceneId.
  Adds 'scenes describe' to COMMAND_META in capabilities.ts.
  Adds 2 tests (known + unknown scene).

C4 (#13): add JSDoc comment on hints field in agent-bootstrap.ts clarifying
  empty array semantics. Add 'hints' field to cliAddedFields in schema.ts.

C5 (#16): create docs/verbose-redaction.md documenting masked headers
  (authorization, token, sign, nonce, x-api-key, cookie, set-cookie,
  x-auth-token, t) and the --trace-unsafe opt-out flag.

C6 (#18): plan schema output now includes agentNotes.deviceNameStrategy
  documenting that deviceName uses require-unique resolution and plans
  should pin deviceId for determinism.
chenliuyun pushed a commit that referenced this pull request Apr 20, 2026
Document every fix landed in this branch beyond the history-aggregate
feature: bugs #1, #4, #5, #6, #8, #9, #10, #11, #12, #13, #14, #15,
#16, #17, #18 from the OpenClaw v2.4.0 smoke-test report. Call out
the deferred items (#2, #7) explicitly so readers don't assume they
were overlooked.
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