Repository navigation
feat(denkraum): split pending, inbox and delivery onto the store (DR-04b) - #669
Conversation
…04b) Move pending(), inbox() and flushNotifications()/#deliver from the frozen P1 DeskStore (lines 515-569) into tools/denkraum/store/delivery.mjs as DeliveryStore extends AnswerStore. The code equals the split proof's delivery.mjs lines 108-162 after two renames: this[CHANGE]( to this.change( and this.io.rootAgentId to the injected this.rootAgentId (D2). The delivery id stays the event id until a queued delivery is older than seven days; a failed attempt keeps a fixed error code and no transport detail. On V2 every receipt lookup fails closed with 503 while no root agent id is configured. No new product logic, no HTTP, ENV or UI. Test-First: tools/denkraum/store/delivery.test.mjs::V2 pending inbox and delivery fail closed without a configured root agent id Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…69-O1) OpenAI R1 on #669 found that pending(), inbox() and flushNotifications() only failed closed once an event reached receiptFor(). On a V2 ledger without active events, or with flushNotifications('other-event'), they answered [] / empty lists / 'empty' without a configured root agent id. All three paths now check the root id on V2 right after reading the state, before any event is filtered. V1 stays unchanged (no receipts). A missing transport still returns 'not-configured' first: P1 returns it before reading the ledger, so no root-bound lookup runs (D1). Test-First: tools/denkraum/store/delivery.test.mjs::V2 reads and delivery refuse a missing root agent id even when no event needs a receipt check (R669-O1) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Cuarroc
added
the
review-ok
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc34cae381
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const ref = eligible.find(e => e.content === a).eventRef; | ||
| if (!Object.hasOwn(a, 'webhookDelivery')) a.webhookDelivery = { status: 'pending', attempts: 0, lastAttemptAt: null, queuedAt: null, lastError: null }; | ||
| if (a.webhookDelivery?.status === 'queued') { | ||
| a.webhookDelivery.status = 'pending'; a.webhookDelivery.deliveryId = randomUUID(); a.webhookDelivery.queuedAt = null; |
There was a problem hiding this comment.
Wenn zwei DeliveryStore-Instanzen oder Prozesse dasselbe seit sieben Tagen als queued markierte Ereignis gleichzeitig abarbeiten, lesen beide den alten Zustand und erzeugen hier unabhängig voneinander neue UUIDs. Dadurch gehen zwei notifyEvent-Aufrufe mit unterschiedlichen deliveryId-Werten für dasselbe Ereignis heraus; die vorgesehene Deduplizierung durch Root anhand der Zustellungs-ID kann dann nicht greifen und das Ereignis kann doppelt verarbeitet werden. Die Erneuerungs-ID muss deterministisch sein oder interprozesssicher genau einmal persistiert werden, bevor ein Sender zustellt.
Useful? React with 👍 / 👎.
| requireRoot(state, this.rootAgentId); const at = this.io.clock(); | ||
| const eligible = activeEvents(state).filter(e => (eventId === undefined || e.eventRef.eventId === eventId) && !receiptFor(state, e, this.rootAgentId)); | ||
| const a = eligible.map(e => e.content).filter(a => | ||
| (a.webhookDelivery?.status !== 'queued' || isoTime(a.webhookDelivery.queuedAt) && at - Date.parse(a.webhookDelivery.queuedAt) >= 7 * 86400000)) |
There was a problem hiding this comment.
Wenn ein gespeicherter webhookDelivery-Eintrag den Status queued, aber keinen gültigen queuedAt-Zeitstempel enthält, liefert isoTime(...) hier false und der Filter verwirft das Ereignis still. Da validateState die Outbox-Felder nicht prüft, wird das Ledger weiterhin erfolgreich gelesen und jeder spätere flushNotifications()-Aufruf meldet empty; die Antwort wird dadurch dauerhaft nicht zugestellt, statt einen 503-Fehler oder einen reparierbaren Pending-Zustand zu erzeugen. Der Queue-Eintrag sollte vor der Eignungsfilterung validiert werden.
Useful? React with 👍 / 👎.
| this.#notificationQueue = work.catch(() => {}); return work; | ||
| } | ||
| async #deliver(eventId) { | ||
| if (typeof this.io.notifyEvent !== 'function') return { status: 'not-configured' }; |
There was a problem hiding this comment.
Bei einem V2-Ledger ohne konfigurierte rootAgentId und zugleich ohne notifyEvent kehrt #deliver hier vor dem Lesen des Ledgers und vor requireRoot mit dem normalen Status not-configured zurück. Damit wird die fehlende Root-Bindung gerade in der üblichen Konfiguration mit deaktivierten Benachrichtigungen nicht wie vom D2-Vertrag in docs/PLAN.md:58-60 gefordert mit 503 abgelehnt, während derselbe Aufruf mit vorhandenem Transport korrekt fehlschlägt. Die V2-Root-Prüfung muss vor diesem Rückgabepfad stattfinden, ohne das V1-Verhalten zu ändern.
Useful? React with 👍 / 👎.
Merge Queue Status
This pull request spent 13 minutes 38 seconds in the queue, including 13 minutes 20 seconds running CI. Required conditions to merge
|
Der Denkraum kann jetzt auf dem neuen Speicherkern offene Antworten auflisten, einen Posteingang für Root bilden und Benachrichtigungen mit stabiler Zustellungskennung verschicken, genauso wie im bisherigen Stand P1.
Ohne konfigurierte Root-Kennung lehnt er diese drei Wege im neuen Datenformat mit Fehler 503 ab, statt still etwas Falsches zu zeigen oder zu senden.
Fortschritt, Ideen und die Bedienoberfläche folgen in eigenen Paketen; zwei Fremdprüfungen und Roots Abnahme stehen noch aus.
Report
Package: DR-04b (Z1), PLAN row DR-04, parent ticket
55ea21ce. Released by Orchestrator 13:28Z (msg-0muzknusg) as the serial second half of DR-04 after the 04a/04b split (Orchestrator accepted the split 13:19Z). Owner: DR-03/04 session (Claudeclaude-opus-5-5[1m]). Stacked on #666: baseclaude/dr-04-denkraum-answers@1e11bb8d45d26cca4d31974314bcb8af2763ed94. #666 is stacked on #661 @c80456a; Root retargets after those merge. Tier A (persistence, D2 trust boundary, outbound delivery).What changed (2 new files, 191 ALL incl. the R669-O1 fix)
tools/denkraum/store/delivery.mjs(69):class DeliveryStore extends AnswerStorewithpending(),inbox(),flushNotifications(eventId)and the private#deliver, plus the per-instance#notificationQueue. Moved from frozen P1store.mjs(sha256592b309d…a2c9) lines 515–569. No new product logic. Only two renames:this.#change(→this.change((DR-03 core) and the P1 constantrootAgentId→ injectedthis.rootAgentId(D2, 4 places).receiptForlookup in these three paths therefore fails closed with 503 without a configured root id.dc34cae, the only behaviour change against P1): a module helperrequireRootchecks the root id on V2 right after the state read inpending(),inbox()and inside the#delivertransaction, before any event is filtered. So an empty V2 ledger andflushNotifications('other-event')also answer 503. P1 could not show this case because its root id was a constant.receiptForreturnsfalsefor V1 before the root check, so V1 pending/inbox read the legacyackand need no root id.tools/denkraum/store/delivery.test.mjs(122): 8 store-level tests, synthetic ids only.pendingwith the fixed codetransport-failed; the transport detail is not persisted. The retry reuses the event id as delivery id. The payload carries only immutable refs (type,eventId,contentRevision,deliveryId). Aqueuedevent is not sent again and is not a receipt.queueddelivery older than 7 days gets a new delivery id for the same event; a failed renewal keeps the new id; nothing is sent after the Root receipt.pending/inbox/flushNotificationsfail with 503, write nothing and never call the transport.flushNotifications('other-event'); without a transport the result staysnot-configured.Relation to the Planungsarchitekt DR-04 draft (ticket
55ea21ce, 01:18:48Z)dr02-proof-20261008/after/store/delivery.mjs(sha25618f4d5f3…dae7) lines 108–162 withthis[CHANGE](→this.change(andthis.io.rootAgentId→this.rootAgentIdapplied,diffagainstdelivery.mjslines 10–64: exit 0, no hunk. The#notificationQueueline (proof 8 ↔ 9): exit 0.CHANGE/PROGRESSsymbols and ENV default; nostore.mjsre-export yet; no C-3b exception (Chief 13:19Z).Evidence
1e11bb8node --test --test-name-pattern='V2 pending inbox and delivery fail closed' tools/denkraum/store/delivery.test.mjsERR_MODULE_NOT_FOUNDfordelivery.mjs(new feature module, same red pattern as DR-03 core); log sha256cdd7b979…db0dnode --test tools/denkraum/store/delivery.test.mjs;npm run test:denkraum(Windows)sender.test.mjs+ P1ideas.test.mjs+ thismodel/core/answers/delivery+ remaining P1 ops (264–415) asDeskStore extends DeliveryStorewith injected root id; P1 515–569 removed from the shim;node --test --test-concurrency=1 *.test.mjsCARGO_TARGET_DIR=~/targets/slot5,CARGO_BUILD_JOBS=2, foregroundssh_exec144f12fprecommit exit 0 (13:38:14Z, incl. secrets, typecheck, denkraum, architecture-drift)bash scripts/ci/gates.sh lane prepushon144f12fgruen, rust-suite nextest 1873 run / 1873 passed / 7 skipped; log sha256f264d398…a0e. An earlier identical run (13:38:52Z) also finished green but its exit code was not captured because the SSH session closed; it does not count as evidence.bash scripts/ci/red-first.sh7e3a7af; all 6 trailers in the stack red/absent at the merge base and green at the head144f12fnode --test tools/denkraum/store/delivery.test.mjsMissing expected rejection; log sha2568261ef883824f6c0…dc34caegruen, nextest 1873 run / 1873 passed / 7 skipped; log sha2560ee97b5f6bc19242…; localnpm run test:denkraum(Windows) exit 0, 53/53, hygiene 0 findingsbash scripts/ci/red-first.sh;git push7e3a7afand green at the head. First push 14:26:51Z was rejected by the remote (reason not captured:-q); the retry reported144f12f..dc34cae, exit 0.git ls-remote14:28:55Z =dc34cae381e0e113a7ed406d81b018f0e26d1ceegit push+git ls-remotePA_PREPUSH=light git push -uafter the full prepush on the same head, exit 0;git ls-remote13:44:15Z =144f12faa3dbebabbdeb0c4d743b05a87253537c; #666 branch unchanged at1e11bb8, #661 branch atc80456aLimits (unchanged, not solved here)
#notificationQueueserializes deliveries per store instance only. Two instances or processes can each deliver once; Root dedupes bydeliveryId. Same class as R661-02.activeEventsis quadratic and runs inpending,inboxand every flush.Review
Tier A. OpenAI R1 on
144f12f: approve with conditions (R669-O1). OpenAI R2 delta ondc34cae: approve with conditions — R669-O1 fixed except the no-transport case (tools/denkraum/store/delivery.mjs:35: a rootless store without transport returnsnot-configuredbefore the root check; no write, no delivery, ledger bytes unchanged). Google R1 (Gemini 3.1 Pro) ondc34cae: approve; R669-G1 low on the same line, judged not a real gap;npm run test:denkraumexit 0.Root acceptance: two foreign vendors approve at the current head; the remainder is a tracked follow-up.
receiptFor. An empty V2 ledger gavepending()[],inbox()empty lists andflushNotifications()empty.flushNotifications('other-event')also gaveempty.dc34cae. Test:delivery.test.mjs::V2 reads and delivery refuse a missing root agent id even when no event needs a receipt check (R669-O1).not-configuredstay for a missing transport?#deliverreturnsnot-configuredbefore it reads the ledger, so no event and no root-bound lookup is touched. Moving the root check in front of it would change P1 behaviour without a security gain (D1), and on V1 the check does not apply anyway. The test pins this with a rootless store without a transport.NICHT ABGEDECKT
store.test.mjsandsender.test.mjsrun only in the scratch composition.notifyare DR-06 and not tested here.#[cfg(windows)]tests (run_in_pty, npm_shim, ConPTY, DPAPI) and the Windows arms of clippy are not covered. Browser smoke, frontend build and workflow gates run only in lanelinux. External services are not checked by any gate. The Windows verdict comes from the merge queue.🤖 Generated with Claude Code