Repository navigation
feat(memory): spoke-memory optional capability family — wire, operations, engine, Connect, conformance - #114
Conversation
…nternal-error to retryable
…lformed grant timestamps
The pending candidate's `proposed` (kind + content) and the consumed shell (kind only) shared the JSON title "Proposed", so codegen merged them into one kind-only type and a legal pending candidate with `proposed.content` could not deserialize. The consumed shell is now `ConsumedProposed`; regenerated TS and Rust outputs follow, and a regression test covers the pending shape, the consumed shell, and the rejection of content on a consumed candidate. Also repairs the pre-existing `builder_smoke` assumption that `Scope.extensions` is a bare map (P1 codegen made it `Option<ExtensionMap>`).
The regenerated Connect types replaced the per-variant extension key aliases with the shared `ExtensionKey`, and payload/extension fields became newtypes (`Shared0`). Mechanical adaptation only: wrap the extension maps and request payload in their newtypes, convert the invoke payload back to `serde_json::Value` before dispatch, and follow the same renames in the loopback fixtures. No behaviour change.
…es, dual-provider fixes, QC fix rounds) into iteration/v0.15-memory
 [Critical impact] The latest changes appear safe to merge; the remaining malformed-receipt checks are non-blocking. Findings
Fix with agent prompt### Issue 1
packages/spoke-connect-ts/src/remote/remote-adapter.ts:752-755
The new memory methods call `#invokeMapped`, but none of their operations appear in `SUCCESS_SHAPES`. `isValidSuccessPayload` therefore accepts any value, including `null` or a string, and returns it as a typed memory response. Callers can then fail when reading expected fields. Add required-field checks for memory reads and receipt states, matching the existing port checks.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds the optional
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…est check
A chunker that drops an empty header.extensions (D8: omitted == empty
map) reassembled into a bundle without the key, so the sealed digest
computed over extensions: {} never verified. Normalize the omission
before hashing and cover the foreign-chunker shape with a regression
test.
…erve budget The memory serve-budget timeout wrapped service.invoke in tokio::time::timeout, whose expiry dropped the service future — cancelling admitted provider work (the receipt write could be lost) while answering OUTCOME_UNCERTAIN. The TS path leaves the provider promise running; match it by spawning the invoke and racing the deadline against a oneshot: expiry answers the budget, the detached task runs to its real completion, and polling observes it. The response vocabulary is unchanged (result if ready, else OUTCOME_UNCERTAIN + retry_class uncertain); zero-budget and non-memory timeouts are untouched. RemoteServePorts::as_memory now returns an owned Arc so the spawned task can own the face. The loopback suite gains a provider fixture whose commit is part of the invoke future (no self-detach), proving the commit lands after the deadline.
…keep get_receipt read-only Two reviewed findings on the SQLite reference provider: - recall: the cursor offset was applied AFTER engine_recall limited the page to max_items, so page 2+ was empty forever and the cursor never advanced. The engine RecallRequest now carries the offset (MEM-R026) and drops the leading ranked records before the item/byte budget; the provider passes it through. New walk-every-page test plus an engine-level offset test. - receipts.get: polling ran complete_pending through mutate_value, a write on the read path (MEM-R011). The provider worker now runs separately (run_worker); the port read only observes stored state. The conformance interpreter drives the worker explicitly (honoring the corpus hold/release vocabulary), direct poll sites settle via run_worker, and a new test asserts polling alone leaves a pending receipt pending.
Memory ops had no entry in SUCCESS_SHAPES, so invokeMapped's shape gate accepted any payload for every memory.* response (spokeOk(garbage)). Add required-field checks mirroring the existing port checks and the schemas/ops/memory-*-response success branches: read ops get their declared top-level fields (describe/spaces/subjects/candidates/grants/ records/recall/profile/reflections/exports), and every mutating op plus memory.receipts.get gets the receipt-envelope fields shared by the committed/pending/failed states. FieldKind gains boolean for recall.truncated and reflections.state.stale.
…ld comparisons) into iteration/v0.15-memory
|
@greptile-apps review |
…nonces §14.3: an admitted memory invoke that dies after invocation (Rust: the detached task panics and drops its result channel; TS: the provider throws/rejects) left the responder answering a clean INTERNAL_ERROR, which asserts no-effect the transport cannot support. Both paths now answer OUTCOME_UNCERTAIN with details.retry_class = "uncertain" and kind = "panic"; the Rust helper sits beside memory_uncertain_reject and the TS memory catch matches it. INTERNAL_ERROR stays on paths where no memory write is possible (extract, port, dispatch-denies). Tests: a loopback fixture mode whose admitted capture panics proves the Rust branch, and a TS responder test drives a throwing memory service. Hello nonces in memory_loopback / memory_conformance are now derived from the clock at runtime instead of hard-coded values (CodeQL rust/hard-coded-cryptographic-value); the nonce is opaque to the assertions.
…untime nonces) into iteration/v0.15-memory
|
Independent review of head f0ebec9 — report at /tmp/pr-114-review.md (session-side; summary below). All six Greptile findings are verified fixed at head with regression tests (transfer extensions normalization, responder deadline/panic → OUTCOME_UNCERTAIN with single-commit replay, recall cursor offset before budget, pure receipt reads, full SUCCESS_SHAPES coverage); local cargo (spoke-memory / spoke-operations / spoke-connect) and vitest (397 tests) pass at head, CI is green, and no code-scanning alerts remain open. One new blocking issue: the generated TS types keep Verdict: request changes (fix the committed-receipt |
… output json-schema-to-typescript drops required-ness for names with no declared property in the same schema: the op-pin allOf fragment required [op, idempotency_key] but declared only op, so TS consumers could build committed-receipt envelopes the schema rejects (Rust/typify kept the key non-Option all along). Declare the idempotency_key property on the pin fragment in the 18 keyed mutation response schemas (plus the same pin shape in memory.receipts.get's failed branches): the emitted intersection now carries idempotency_key as REQUIRED while Rust output stays semantically unchanged (doc lines only). Regression: a compile-level assertion in spoke-connect-ts (tsc fails if the key goes optional again) plus a runtime round-trip — see tests/schemas/receipt-requiredness.test.ts.
- recall: the §9 byte measurement no longer panics on an unserializable
content value — it returns the typed RecallError::NonFiniteContent,
mirroring the non-finite guard in spoke-operations digests (serde_json
Values cannot hold non-finite floats by construction, so the guard is
defensive parity; documented at the site). recall() is now fallible;
providers map the error to INVALID_INPUT and tests unwrap via a helper.
- partition_bundle now rejects a bundle whose header lacks extensions
(D8): partitioning such a bundle would emit chunks whose reassembly
normalizes the omission to {} and fails the bundle-digest check against
the source digest. Test added; the corpus asset already carries
extensions.
|
Reviewer round fixes pushed (29a5f7d): the blocking P2 (TS requiredness for idempotency_key on the 18 keyed receipt branches — pin-fragment declaration repair, compile-level regression added) plus P3-1 (recall serialization guard) and P3-3 (partition asymmetry reject). P3-2/4/5 recorded as hardening advisories. Requesting re-review at 29a5f7d. |
|
@greptile-apps review |
|
Round-2 re-review of head 4c5c21d (round-1 report: /tmp/pr-114-review-r2.md, session-side; summary below). The round-1 blocking P2 is properly fixed at the schema/codegen level: the keyed receipt allOf fragments now declare Verdict: ship it |
Summary
Lands the optional
spoke-memorycapability family — per-subject subjective memory (impressions, not canon) with candidate→review→retain gating, explicit-only forgetting, payload-bound idempotency, single-authority spaces, and cooperative-provider transfer — as a complete wire-to-conformance slice.memory.*op pairs; inventory 34 → 113) + regenerated TS/Rust typesMemoryPort(Rust trait + TS interface, standalone likeExtractionPort) + pure validators, grant-coverage, JCS digest helpers, 14 reject codescrates/spoke-memoryreference engine: classify / recall / reflect / transfer — deterministic, I/O-free, RFC 8785-canonical (ES6 number semantics via ryu)spoke-memory/spoke-memory-reflection/spoke-memory-transfer(Rust + TS),RemoteMemoryService+ typedMemoryPortServicedelegation, client adapters, golden vectorsMEM-R001–R027) + capability ADR + Author-Memory boundary amendments + integrator docs (EN/zh) + release-readiness verification (no version bump in this PR)Protocol source: nexus research package
01-spoke-memory-protocol.md(proposed), landed with repo-side adjustments recorded in the iteration compass (D8 chunk-header extensions carrier; semantic locks: noaffectwire field, explicit-only forgetting, flat subjects).Residual disclosure (open issues, none blocker-deferred)
Merge policy
pr_merge_policy: reviewed-auto— this PR is expected to receive a merge approval via the PR-level review loop (ship it on the current head), not a human owner gate.Verification
pnpm run verify-codegenexit 0 (113 schemas; opaque-JSON + hoist asserts)cargo +nightly check --workspaceclean;pnpm run docs:build+pnpm run verify:versiongreen