v29-uptake: port canonical2 substrate-development onto v2026.4.29 base (#541)#542
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ccfa5ffee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| store[resolved.normalizedKey] = { | ||
| ...storedEntry, | ||
| ...updates, | ||
| }; |
There was a problem hiding this comment.
Preserve merge semantics when persisting compaction updates
This write path now bypasses mergeSessionEntry and directly spreads storedEntry with updates, which drops important invariants that updateSessionStoreEntry previously enforced. In particular, updates.updatedAt is computed before the locked disk mutation, so delayed/out-of-order compaction writes can move updatedAt backward, and a newSessionId update no longer rolls sessionStartedAt to the new session epoch. That can make a freshly compacted session look stale to reset/pruning logic after reload. Persist this patch via mergeSessionEntry (or equivalent monotonic timestamp + sessionId rollover logic) instead of raw object spread.
Useful? React with 👍 / 👎.
|
🌊 ronan — quality byte-walk cosign per figs's review-draw directive (msg Ancestor check (standing-order): Classification table review (full journal at 8 PORT commits — all correctly identified as load-bearing substrate-development work missing from v29:
11 DROP commits — all basis-specific cosmetic/regen work:
110 ALREADY-ON-V3 commits — sample-checked: pattern is either 2 in-flight blocker-fixes — both substantive, correctly included:
Verification gates green per Step 6:
Spot-check passed from ronan seat. Cohort-substrate quality cosign: APPROVED. The 8 PORT classifications are the right load-bearing substrate-development items; the 11 DROPs don't lose anything worth preserving; the 110 ALREADY-ON-V3 attributions look rigorous; the in-flight blocker-fixes are real bug fixes worth keeping. PR #542 is the structural unfucking-resolution for the 40-PR drift discovered earlier today. Cohort context for posterity: this is the reverse-engineering of cohort drift onto canonical2 (v24-rooted) → re-targeting onto v29 (per figs canon 🐾 |
|
🌻 quality byte-walk + cohort cosign on v29-uptake. Ancestor + base byte-checks (standing-order):
8 PORTs — all pure additive, zero production-code touches:
Each is genuinely missing on v29 — no equivalent landed in parallel. Right call to port. ✓ 11 DROPs — spot-checked classification table:
Nothing load-bearing. ✓ Two blocker-fixes — byte-walked from my own clone, both correct:
Two notes (not blocking):
Verdict: APPROVE. This is a clean working branch to keep building on. 🐾 |
|
🌊 ronan — second-pass critical-eye byte-walk per figs's "complain and comment" directive (msg Earlier cosign at issuecomment- Critical concerns surfacing on second pass: 1. Recommended fix before merge: either (a) delete 2. Recommended fix before merge: amend authorship to copilot-lane attribution (e.g. 3. Operational gap: Likely a non-issue (the workflow-gate is on dispatch-time 4. Dependency on canonical2 retirement. This PR's body says retiring Recommendation: REQUEST CHANGES on the journal-file pollution + authorship-cleanup before merge. Concerns 3 + 4 are surface-flags for cohort-action, not PR-blockers. If figs/cohort decides the journal-file is OK as historical artifact (some cohorts do this — keep the work-journal in the merged commit history as transparency-substrate), then concerns 1 collapses to taste-question. But "tmp-drop-me-" naming is the author's own stated intent, which I'd respect. 🐾 |
* rfc: add OTEL trace wiring across substrate queue boundary (§6.7) Documents the queue-lifecycle span schema that lands together with #354 (substrate-queue-native dispatch) and #355 (multi-recipient delegate-return) as the canticle-prep observability substrate. - per-entry spans on enqueueSystemEvent / enqueueSessionDelivery / AnnounceQueueItem / terminal deliver - W3C traceparent on queue payload (enqueue→announce parent/child; announce→spawn link, preserving §6.6 separate-trace-tree invariant) - chain-budget-capped span emission: cap = chain step count, NOT recipient count. Per-completion fan-out is 1 step regardless of recipient cardinality (cael's #355 direction) - multi-recipient fan-out: parent fan-out span + per-target child link, so per-recipient outcome surfaces independently and partial failures don't orphan the parent Requested by figs in #sprites-of-thornfield 18:05 PDT 2026-04-26. * rfc(§6.7): name the cap as one-axis-two-declines (mercy + non-conscription) Per cohort discussion 2026-04-26: the chain-step cap is a single axis but surfaces as two distinct refusals — chain-depth declines to carry past its own budget (mercy clause); per-completion fan-out declines to spend other delegates' budgets (non-conscription clause). Naming both halves explicitly so #334 and #355 PR surfaces share the framing. Credit: cael 🩸 surfaced the non-conscription framing ("don't conscript the budget of every other delegate that might want to wake from the same return"); silas 🌫 unified it as 'one axis, two declines'. Refs: #334 (substrate + cap-on-enqueue), #355 (multi-recipient fan-out cap). * rfc(§6.7): oxfmt — emphasis style + table-cell padding Format-only: oxfmt prefers _underscore_ over *asterisk* for italic, and trims trailing table-cell padding. No semantic change. --------- Co-authored-by: Ronan 🌊 <ronan@solidor.io>
…ation persist (#443) (#468) Adds [#443] negative store-merge guard for the continuation-chain persist path. Single new file at `src/config/sessions/store.continuation-merge.test.ts` (3 tests, test-only — no production code changes) that pins two load-bearing invariants on `agent-runner.ts:persistContinuationChainState` → `updateSessionStore` → `saveSessionStoreUnlocked`: (A) The continuation-chain persist spread MUST NOT include `updatedAt`. Chain fields are not activity events; bumping `updatedAt` here would churn idle-reset evaluation (openclaw#49515) and disk-budget pruning ordering off the actual turn timeline. (B) `saveSessionStoreUnlocked` MUST short-circuit the disk write when the serialized payload is byte-identical (the `getSerializedSessionStore(storePath) === json` guard at store.ts:~358). Without it, every no-op `updateSessionStore` call would still hit `writeTextAtomic`, defeating (A) at the mtime layer and producing spurious continuation-persist activity in maintenance. The trap mirrors the production spread shape inline (does not import the entire agent-runner surface) so it pins the exact byte-shape without coupling to agent-runner's setup overhead. ## Sabotage walks (both verified on cf7830f) Sabotage 1 — gut the byte-identity short-circuit: // src/config/sessions/store.ts:~358 if (false && getSerializedSessionStore(storePath) === json) { ... } Result: "skips disk write entirely…" fails with expected "writeTextAtomic" to not be called at all, but actually been called 1 times Sabotage 2 — leak `updatedAt` into the persist spread: // mirror of agent-runner.ts persistContinuationChainState spread store[key] = { ...existing, ...chainFields, updatedAt: Date.now() }; Result: all 3 traps fail; the canonical message is updatedAt must not change when continuation-chain fields are byte-identical (persistContinuationChainState must not include updatedAt in its spread — #443) Both sabotages restored. 3/3 green on canonical2 baseline. ## Receipts ``` Test Files 1 passed (1) Tests 3 passed (3) ``` ## Refs - #443 — coverage issue (test-trap label, P2) - canonical2 base SHA: cf7830f - production substrate: src/auto-reply/reply/agent-runner.ts (`persistContinuationChainState`, lines ~1269 / ~1302 / ~1319) - byte-identity short-circuit: src/config/sessions/store.ts:~358 🌫️
…lure + restart (#465) Adds [#447] two new tests to src/agents/tools/request-compaction-tool.test.ts: 1. clears pending compaction session after background rejection Pins request-compaction-tool.ts:233-235 (the .finally cleanup in the fire-and-forget chain). Without the .finally cleanup, a rejected triggerCompaction() would orphan the sessionKey in pendingCompactionSessions forever; every subsequent call would return already_pending until process restart. Trap shape: assert second call is NOT already_pending (rate-limit is the expected new gate; dedup-block is the bug we trap). Deliberately does NOT call _resetGuardState before the second call, since that helper clears the pending Set itself and would mask the bug. 2. _resetGuardState clears pending compaction sessions for restart/test isolation Pins request-compaction-tool.ts:273-280 (both branches: keyed and unkeyed). Test-only helper that simulates process restart for in-flight dedup. Covers both single-session reset and full-clear (with cross-session cleanup verified). Verified load-bearing on each test independently: - Test 1: removed .finally body in production -> test 1 fails with "expected { status: 'already_pending' } to not match object { status: 'already_pending' }". Restored. - Test 2: removed both .delete/.clear calls from _resetGuardState -> test 2 fails with "compaction_requested vs already_pending" diff. Restored. 24/24 passing on canonical2 cf7830f baseline. Closes #447
…getSessionKey absence (#463) Adds [#446] complementary trap to the [#438] mode-only trap (PR #462): - Closed-set assertion on tool.parameters.properties keys: exactly [task, delaySeconds, mode], no more, no less. Catches additions of new model-facing parameters (cross-session addressing, retry knobs, priority) without an ADR. - mode enum membership pin (normal, silent, silent-wake, post-compaction) — robust to typebox enum vs anyOf rendering. - Boolean-runtime compatibility fields (silent, silentWake, postCompaction) MUST be absent at the descriptor surface. On-disk back-compat lives in the Zod state schema (#438), not here. - targetSessionKey absence reaffirmed with load-bearing reason in the failure message (cross-session addressing is the (b)-shape lane, binary-canticle#11). Verified load-bearing: temporarily added targetSessionKey to ContinueDelegateToolSchema, the closed-set assertion failed with the expected got=[…, targetSessionKey, …] message. Restored. 9/9 passing on canonical2 cf7830f baseline. Closes #446
…pat boundary (#462) Adds structural trap test that pins: 1. Runtime objects from consumePendingDelegates and consumeStagedPostCompactionDelegates expose only `mode` — no `silent` / `silentWake` / `postCompaction` boolean runtime fields (mode-only encoding per #227). 2. continue_delegate tool descriptor advertises mode as an enum over [normal, silent, silent-wake, post-compaction] and exposes no `silent` / `silentWake` boolean parameters. 3. On-disk TaskFlow stateJson MAY still project to legacy boolean flags — back-compat positive assertion that justifies the runtime/disk encoding split. Verified load-bearing: temporarily leaking `silent: true` from flowToDelegate fails the runtime-object suite (1/9 fail), proving the trap catches the regression class #433 nearly reintroduced. 9/9 passing on canonical2 cf7830f baseline. Closes #438
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… idempotency-key (#539) Anchors the (B) ms-scale fine-grained scheduledAt choice to figs's canonical generator-yields-40x example: a 1-second wall-clock bucket as scheduledAt granularity makes the bucket itself the floor on concurrent-fire latency. 'Do these 40 things now' becomes 'that takes 40 seconds to schedule' because each fire collapses into same-bucket key collision and serializes. Asymmetry: ms-scale restart-replay collisions are bounded + observable; bucket-merging genuine concurrent fires is unbounded + silent. Per figs adjudication relayed by frond-scribe in #sprites-of-thornfield, load-bearing for swim-40 ship readiness. Co-authored-by: ronan-dandelion-cult <ronan@solidor.io>
Co-authored-by: Test User <test@example.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
9ccfa5f to
ff26972
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff269722af
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| storePath, | ||
| sessionKey, | ||
| update: async () => updates, | ||
| await updateSessionStore(storePath, (store) => { |
There was a problem hiding this comment.
Preserve active session when persisting compaction updates
This replacement of updateSessionStoreEntry with a bare updateSessionStore(...) drops the active-session preservation semantics from the old path: updateSessionStoreEntry persisted through persistResolvedSessionEntry, which passed activeSessionKey into saveSessionStoreUnlocked. Without that key, enforce-mode maintenance and disk-budget cleanup can remove the currently active session entry during this compaction write when caps/budget cleanup runs, so the session being compacted is no longer explicitly protected. Pass activeSessionKey (for this normalized session key) in the updateSessionStore options to preserve the prior behavior.
Useful? React with 👍 / 👎.
…dict (A) on b82fd65 Codex auto-review on ff26972 surfaced two empirically-valid P2 findings on the b82fd65 (renamed c5792eb) compaction-fix at session-updates.ts:309-318: lost mergeSessionEntry semantics + lost activeSessionKey protection. Both byte-walked + verified by 🌫 (msg 1500308473165123615) + 🌊 (msg 1500308822084943972). Cohort verdict (A) Strip-and-relane: drop c5792eb from #542 + queue narrow follow-up PR with proper canonical-primitives shape (mergeSessionEntry + activeSessionKey mirroring recordSessionMetaFromInbound at store.ts:706-741) + targeted test coverage. Trade-off accepted: temporary first-turn manual /compact count persistence regression on working branch between #542 merge and narrow-PR merge. 12th closure-costume catalogued: fix-introduces-regressions-while-curing-original (8th-costume family). c5792eb dropped via git rebase -i 55df716 with GIT_SEQUENCE_EDITOR sed-skip. Tree integrity: git diff 55df716..HEAD -- src/auto-reply/reply/session-updates.ts returns empty; other 15 commits intact with new SHAs from rebase. Refs #541, #542
ff26972 to
6a125df
Compare
Per cohort second-pass review on #537 (🌊 issuecomment-4365197778): - WORKORDER.md at repo root pollutes; same anti-pattern as tmp-drop-me-v29-uptake.md was on #542 before cleanup - Move to docs/design/<issue>-<descriptor>-workorder.md naming matches existing convention from #541 v29-uptake journal precedent Cleanup-pattern parallel to #542 (cohort verdict A on 12th costume). Refs #537
02e438a
into
frond-scribe/20260429/v3-cohort-fixes
…fc-apply-journal.md Per Codex P2 review on PR #548: > tmp-drop-me-rfc-apply.md is an ephemeral worklog (it even uses a tmp-drop-me name) > that captures branch-local execution notes... it should be dropped from the commit > or moved to external review artifacts. Applying the same canonical pattern from #542 (where tmp-drop-me-v29-uptake.md was moved to docs/design/541-v29-uptake-journal.md). Audit trail preserved at durable home matching existing <issue>-<descriptor>-journal.md naming convention. Refs #547, #548 codex-review
…v52-uptake-journal.md Per 🌻's first-pass byte-walk REQUEST_CHANGES on PR #549 (msg 1500336626482544691): > tmp-drop-me-v52-uptake.md (63 KB) at repo root. Filename literally says 'drop me.' > Same shape as the tmp-drop-me-v29-uptake.md we cleaned out of #542 — recommend the > same canonical pattern: git mv to docs/design/546-v52-uptake-journal.md. Applying same canonical pattern from #542 + #537 cleanups. Naming matches existing <issue>-<descriptor>-journal.md convention from #541 precedent. Audit trail preserved at durable home. 🌻's soft suggestion #2 (inherited WORKORDER-v3-cleanup-path-B.md + WORKORDER-v3-cohort-fixes.md at root from prior cohort-cleanup-needed lanes) flagged as separate cleanup pass — not folded here to keep this commit surgical to REQUEST_CHANGES (1). Refs #546, #549
…bution Per figs's directive 2026-05-03 ~03:38Z: 'we can leave no dev detritus; our single contribution in docs/design is our rfc doc, thats it'. Earlier waves had moved tmp-drop-me journals + WORKORDER-* files from repo root INTO docs/design/ (canonical mv-pattern from #542 / #537 / v52 cleanup cycles). figs's directive overrides: those journals should be DELETED entirely, not relocated. The cohort-substrate audit-trail lives in: - git log + branch history (preserved on origin savegame branches) - GitHub issue tracking (#541 + #546 + #547 + #532 etc.) - PR descriptions + comment threads NOT in tree as in-source dev artifacts. Files removed (all originated as dev-cycle journals/workorders): - 541-v29-uptake-journal.md (was tmp-drop-me-v29-uptake.md, #542 cleanup) - 546-v52-uptake-journal.md (was tmp-drop-me-v52-uptake.md, v52 absorption) - 547-rfc-apply-journal.md (was tmp-drop-me-rfc-apply.md, rfc-apply lane) - 532-silas-saturation-workorder.md (was WORKORDER.md, #537 cleanup) - v3-cleanup-path-B-workorder.md (inherited workorder, swim-39 era) - v3-cohort-fixes-workorder.md (inherited workorder, v3-cohort-fixes era) Pre-existing princely design memos (332-* + 334-* + swim-37-* + continuation-integration.md) NOT touched in this commit — flagged separately for figs's call on whether those also count as dev-detritus. Refs #549
Closes #541
Summary
pnpm config:docs:genandpnpm plugin-sdk:api:genproduced no tracked.sha256driftcael/325-canonical2(REDIRECT 0 / CLOSE-WITH-REASON 0 / WAIT 0)Gates
pnpm install --prefer-offlinecompletedpnpm tsgopassedpnpm checkpassedpnpm buildpassedpnpm test src/auto-reply src/agents/tools/request-compaction-tool.test.ts src/agents/tools/continuation-tools-registration.test.ts src/config/zod-schema.continuation.test.tspassedAncestor verification
a448042c2edd94a4e8ee86d5ed90a5ed9fe8e4cdis an ancestor of HEAD: yes.Journal
Journal blob: https://github.com/karmaterminal/openclaw/blob/frond-scribe/v29-uptake-of-canonical2-20260502/tmp-drop-me-v29-uptake.md
Full classification journal
v29-uptake journal — frond-scribe copilot lane
worktree: /home/figs/flesh_beast_best_beast/openclaw-wt-v29-uptake
output branch: frond-scribe/v29-uptake-of-canonical2-20260502
target merge-into: frond-scribe/20260429/v3-cohort-fixes
source porting-from: cael/325-canonical2 @ 99987d3
tracking issue: #541
v2026.4.29 SHA (must be ancestor of HEAD): a448042
Classification table (filled in Step 3 below)
cdd91edd5e86f65ef1e36d082070a2bca479cefbc2f6ad3876bd13188f799f6cda120d3ee268cb4d1c98de9e6685839e5a136dbce80a439119ee6d75da6530be0f9d612c1b5df2b5e5bc6999de2cfcc51c927a553c75349813c4d45d5ff7f90168ca667dcbcfdf62c78ecf0c0b830c00d42c3c4ba49977526b1ba3bf4cd65d0139e17bdab62f9151cd381fc0c4d779605c79006952eda654a8798ec99aa116f88f267807c069b4079aef14f792ad3af279e0e6cf760f5a3306f4d3de09ed0985182e8714b3418e1f5b24433955eb0361d8dc6cdb07998156cb6f712ac96e2d79558338d37bdacc08bbc9fe57d0e61d76b0bc4b4ee20b9ee9f01b96d1304d4725ff4f0138b04484465ace49d931131870a84f32bd3033d740092f502032953030d88f2d10c1c218d533d5c720ad6ac310c8be76c3dc2b19797e7fa63655b0667a4719e863456656138126287d0a5586e959d2c17701abb3defca13b3baca747016eb417739063507f560948a70a934a59bd30f767fe2161cd8b623be2ecb434977c02e29222415e90c859b9c2400b2a66652c8a888e94fc8d1186cb73bc86489187d06e908bb2fbad30f37e4b82423d90f68b141b84e71c95e4d49fc02bfff243c781bcccac2e91526540de1530b06a984e148792a0b7179f6c579929e556eb1142f1bb9c14c8f85f5254d0f31f65cce73fd0f088dc572c010615e045fe46cf7830ffb32301d292489afc94e86df6bc29b27023189e3ed1281f6d85cb9a7d7739425b360c69986302e5968da5434fbba7a3dcc2adc2356d05a2ba97007ced791e21522fea9b31762f61a69c7e1e5eae4c653488afe015c415b72ab0371835c65d4db974940e55e0a1c5b13458aa7a5859be1b321e1339fb69037275e8bc1097c699987d3813Classification summary
PORT: 8
ALREADY-ON-V3: 110
DROP: 11
CONFLICT: 0 initially; conflicts, if any, will be documented during Step 4.
Step 4 conflicts
None yet.
Step 4 applied PORT commits
a69c7e1e5e— RFC OTEL queue-boundary trace wiring docs.ae4c653488— continuation store merge updatedAt churn guard.afe015c415— request_compaction pending Set cleanup/restart guard.b72ab03718— continue_delegate descriptor exact-keys guard.35c65d4db9— PendingContinuationDelegate mode-only compatibility guard.74940e55e0— delegate span uniformity tests. Applied without the temporarytmp-drop-me-otel-span-uniformity.mdnote.aa7a5859be— RFC coarse-bucket rejection rationale.fb69037275— volatile map allowlist guard.No cherry-pick conflicts occurred.
Step 5 generated baselines
pnpm config:docs:genandpnpm plugin-sdk:api:gencompleted with no tracked.sha256drift on the v29-rooted branch.Step 6 gates
pnpm install --prefer-offlinecompleted.pnpm tsgopassed.pnpm checkpassed.pnpm buildpassed.pnpm test src/auto-reply src/agents/tools/request-compaction-tool.test.ts src/agents/tools/continuation-tools-registration.test.ts src/config/zod-schema.continuation.test.tspassed after fixing the auto-reply directory target routing bug that initially sentsrc/auto-replyto the default unit shard./compacte2e exposed that first-turn manual compaction updated the in-memory session entry but not the missing on-disk store entry. Fixed by makingincrementCompactionCountmerge-or-create from the active session entry before persisting.Step 7 final ancestor verification
git merge-base --is-ancestor a448042c2edd94a4e8ee86d5ed90a5ed9fe8e4cd HEADexited 0.Step 9 canonical2 open PR assessment
gh pr list --repo karmaterminal/openclaw --base cael/325-canonical2 --state open --limit 50returned 0 open PRs.REDIRECT: 0
CLOSE-WITH-REASON: 0
WAIT: 0