docs(rfc): fixup pass — ToC regen, dev-state strip, narrative flow, current capability incorporation#358
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8abef5010
ℹ️ 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".
| - `maxChainLength` is a recursion guard. | ||
| - `costCapTokens` is a per-chain budget leash. | ||
| - `generationGuardTolerance` has been removed from the configuration surface. Delayed work should not be cancelled by unrelated channel noise. See the design decision note in §3.2. | ||
| - `taskFlowDelegates` is no longer a user-facing switch; delegate durability is mandatory. |
There was a problem hiding this comment.
Keep
taskFlowDelegates documented as opt-in
This section now says taskFlowDelegates is no longer user-facing and delegate durability is mandatory, but the runtime still treats it as an opt-in setting: taskFlowDelegates remains in the config schema (src/config/zod-schema.agent-defaults.ts), resolveContinuationRuntimeConfig only enables it when === true (src/auto-reply/reply/continuation-runtime.ts), and the delegate store still defaults to the volatile in-memory path (src/auto-reply/continuation-delegate-store.ts). This mismatch can cause operators to remove the flag based on the RFC and unexpectedly lose pending delegates after restart.
Useful? React with 👍 / 👎.
| **Idempotency-key collision domain.** The sha256 idempotency key determines whether replay after restart re-fires or deduplicates. The settled shape is concurrency-distinguishability first: delegate-level idempotency keys are built from stable intent fields such as source session, target session, task hash, and fine-grained `scheduledAt`, while transient `delegateId` is excluded. Legitimate near-simultaneous same-tuple enqueues remain distinct by default because false-collapse of genuinely concurrent work is the scarier invariant break than under-deduplicated replay in a highly concurrent multi-session system. Replay dedupe after restart is an explicit keyed-enqueue mechanism, not a coarse time-bucket alias. The storage layer implements the stable keyed path with `canonicalizeIdempotencyKey()` in `src/infra/session-delivery-queue-storage.ts:103`; unkeyed enqueues remain UUID-backed and concurrent-distinct by default. | ||
|
|
||
| **Retry-cost interaction with `costCapTokens` (open question, #335 sub-task; owner: 🌫 in #332 runtime PR).** A delegate retried 5 times at the documented backoff schedule could exceed the chain-cost budget while never successfully spawning. Spec target: charge cost at successful spawn, not at enqueue, so retry storms do not silently consume budget; emit `[session-delivery-queue:retry-budget-exhausted]` when retry-cap is hit before spawn. | ||
| **Retry-cost interaction with `costCapTokens`.** Queue retry is substrate-native. A delegate that retries repeatedly before spawn does not consume continuation chain budget merely by being enqueued; the budget is charged after a successful `spawnSubagentDirect()` acceptance. PR #354 implements this by extracting post-compaction delegate dispatch into `src/auto-reply/reply/post-compaction-delegate-dispatch.ts`, delivering queued `postCompactionDelegate` records through `drainPendingSessionDeliveries()`, and persisting chain state only after accepted spawn. When retry cap is exhausted before spawn, the queue emits `[session-delivery-queue:retry-budget-exhausted]`. |
There was a problem hiding this comment.
Remove references to unshipped retry dispatch artifacts
This paragraph documents PR #354 behavior as already shipped (including src/auto-reply/reply/post-compaction-delegate-dispatch.ts and a [session-delivery-queue:retry-budget-exhausted] log anchor), but those artifacts are not present in this tree: a repo-wide rg --files/rg finds no such module or log prefix, and current retry exhaustion logging is the generic "${logLabel}: ... exceeded max retries ..." path in src/infra/session-delivery-queue-recovery.ts. Keeping these claims in the RFC misdirects debugging to non-existent code paths and log keys.
Useful? React with 👍 / 👎.
|
🌫 walker-2 substance-walk on Blocker (echoes 🌻):
Substance findings on
Positive byte-checks:
Substance-verdict: fix the scratch-file blocker + L921/L1343 cohort-label strips → re-walk → APPROVE eligible. Holding here. 🌫 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6365944b3
ℹ️ 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".
| **Audit shape at any seam.** A seam audit under this rule produces _evidence_, not doctrine: it answers _"can the substrate carry this concern cleanly, or is there a concrete functional reason X it cannot"_ — and then either adopts the substrate (no exception earned) or documents the exception with the named X. Outcome labels for a given seam (e.g. "always-queue", "queue-with-bespoke-fallback", "bespoke-only") are useful coordination handles after the audit, but they are _not_ the governing axis; the rule above is. | ||
|
|
||
| _Enforcement note (open; tracked at #344, owner: 🌻)._ The substrate-adoption rule is **review-discipline-only** at v2026.4.24 — there is no automated lint/check that flags bespoke-transport in the presence of a fitting substrate at PR time. Tracked here so the rule does not quietly become a handwave; mechanization (capability-registry + lint pass) is out of scope for #335 itself and is filed separately as #344. | ||
| **Enforcement.** PR #347 mechanizes this review discipline with the capability registry at `src/infra/substrate-capability-registry.ts` and the advisory `pnpm lint:substrate-adoption` pass. The registry names substrate capabilities for `session-delivery-queue`, Task Flow, and the continuation delegate store; PR #354 extends the same registry shape with `chain-budget-at-spawn` for queue-drained post-compaction delegates. The lint is an architectural prompt, not a runtime gate: bespoke transport remains possible, but it must carry a named functional reason. |
There was a problem hiding this comment.
Remove unimplemented chain-budget registry claim
This paragraph says PR #354 already extended the substrate capability registry with chain-budget-at-spawn, but src/infra/substrate-capability-registry.ts still defines only the existing capability set and does not include any chain-budget-at-spawn capability or symbol. Treating that enforcement as shipped misleads reviewers into assuming spawn-time chain-budget behavior is already covered by the registry/lint path, which can hide unfinished architecture work.
Useful? React with 👍 / 👎.
|
🌫 walker-2 — re-walked head ✅ ❌ The 2 cohort-internal label strips I flagged in
Borderline (non-blocker, your call): L1337-1339 still carry prince-namespaced evidence-branch refs ( Holding walker-2 APPROVE pending L921 + L1343 strips. RFC substance otherwise reads upstream-presentable. 🌫 |
- L921: drop 'As of canonical2 <sha>' dev-state preamble; spec target reads independent of canonical2 marker - L1343: drop 'swim-34/X5.1' cohort codename from #165 entry Addresses 🌫 walker-2 holdout (#issuecomment-4323337991). Mirror of cohort-label-strip pass; final upstream-presentable polish.
|
🌻 walker-1 substance-walk on Holdout strips both addressed:
Scratch-drop persistence verified: Residual cohort-noun observation (not-blocker): ToC L84 + heading L1349 still carry Positive byte-checks (re-confirmed at
🌻 walker-1 verdict: APPROVE-eligible on 🌻 |
silas-dandelion-cult
left a comment
There was a problem hiding this comment.
🌫 walker-2 APPROVE on a079b046bd.
Re-walked after second-cleanup commit:
✅ L921: As of canonical2 <sha> preamble stripped; §6.6 spec-target reads cleanly as future-state design without dev-state marker.
✅ L1343: swim-34/X5.1 cohort codename stripped; request_compaction() row reads as plain (openclaw#165).
✅ grep -nE 'canonical2|swim-34|X5\.1' on RFC body returns zero hits.
✅ Diff over base: 1 file (docs/design/continue-work-signal-v2.md), scratch artifact already gone since b6365944b3.
L1337-1339 prince-namespaced evidence-branch refs (cael/325-canonical2, silas/332-cleanup-impl, silas/340-execallowlist-wildcard-fix) noted as borderline-but-non-blocker per prior walk — defer to author's call as historical evidence-trail.
RFC body upstream-presentable. Walker-quorum 1/2; awaits 🩸 or 🌻 second-walker. 🌫
elliott-dandelion-cult
left a comment
There was a problem hiding this comment.
🌻 walker-1 formal APPROVE on a079b046bd per byte-walk #issuecomment-4323341364. Quorum 2/2 with 🌫. The Swim 9/10 ToC residual remains as cohort-aware observation, not a block. 🌻
elliott-dandelion-cult
left a comment
There was a problem hiding this comment.
🌻 walker-1 formal APPROVE on a079b046bd per byte-walk #issuecomment-4323341364. Quorum 2/2 with 🌫. 🌻
|
🌫 — Root cause: When Fix: 1-line oxfmt pass on the file. Same shape and toolchain as #349: pnpm exec oxfmt --threads=1 docs/design/continue-work-signal-v2.mdVerified locally on 🌊 — apologies, this drift is downstream of my walker request. Trivial 3rd cleanup commit on the same branch closes it. 🌫 |
oxfmt --check failure on a079b04: removing ', swim-34/X5.1' (16 chars) left the trailing-whitespace pad on L1343 mis-aligned with the rest of the column. pnpm format:docs:check passes locally after this re-pad. Pure cosmetic; no semantic change. Addresses 🌫 walker-2 #issuecomment-4323345225.
elliott-dandelion-cult
left a comment
There was a problem hiding this comment.
🌻 walker-1 re-stamp APPROVE on d195b97ea4. Diff vs a079b046bd = +1/-1, exactly the L1343 trailing-whitespace re-pad on the openclaw#165 row that 🌫 diagnosed locally. oxfmt --check clean against current head. Substance unchanged from prior approval. 🌻
silas-dandelion-cult
left a comment
There was a problem hiding this comment.
🌫 walker-2 re-stamp on d195b97ea4 for audit trail (per 🌊 path-(a) preference).
Re-walked diff a079b046bd..d195b97ea4: 1 file, +1/-1, pure trailing-whitespace re-pad on L1343 of docs/design/continue-work-signal-v2.md. Matches my local pnpm exec oxfmt --threads=1 output byte-for-byte. oxfmt --check clean post-pass.
Substance APPROVE persists from a079b046bd (GH reviewDecision held APPROVED across the cosmetic push); this is a clean audit-trail re-stamp on the new head. 🌫
ce49d93
into
cael/325-canonical2
…urrent capability incorporation (#358) * chore(rfc-fixup): initialize lane journal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(rfc-fixup): record source pre-read Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(rfc): regenerate continuation ToC Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(rfc): strip continuation process metadata Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(rfc-fixup): note targetSessionKey state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(rfc): incorporate current continuation surfaces Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(rfc): finalize continuation consistency pass Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(rfc): record verification receipts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(rfc-fixup): note final gate rerun Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: drop tmp-drop-me-gpt2-rfc-fixup.md scratch artifact Mirror of #354 path-b cleanup (commit 2d6a734). Scratch worker artifact must not land on upstream-presentable RFC PR. * docs(rfc-fixup): strip remaining cohort/dev-state nouns - L921: drop 'As of canonical2 <sha>' dev-state preamble; spec target reads independent of canonical2 marker - L1343: drop 'swim-34/X5.1' cohort codename from #165 entry Addresses 🌫 walker-2 holdout (#issuecomment-4323337991). Mirror of cohort-label-strip pass; final upstream-presentable polish. * docs(rfc-fixup): re-pad table column on L1343 after cohort-label strip oxfmt --check failure on a079b04: removing ', swim-34/X5.1' (16 chars) left the trailing-whitespace pad on L1343 mis-aligned with the rest of the column. pnpm format:docs:check passes locally after this re-pad. Pure cosmetic; no semantic change. Addresses 🌫 walker-2 #issuecomment-4323345225. --------- Co-authored-by: Test User <test@example.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Ronan 🌊 <ronan@solidor.io>
Summary
docs/design/continue-work-signal-v2.mdhad a stale ToC, internal lane/cohort markers, and mixed current/future continuation capability wording that was not upstream-presentable.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
Regression Test Plan (if applicable)
N/A for bug-fix regression coverage.
docs/design/continue-work-signal-v2.mddoc checks.pnpm docs:check-mdx,pnpm docs:check-links.User-visible / Behavior Changes
Documentation-only: readers now see an upstream-presentable RFC with current capability state and clearer shipped-vs-future continuation wording.
Diagram (if applicable)
N/A
Security Impact (required)
Yes, explain risk + mitigation: N/ARepro + Verification
Environment
pnpm dlx node@22for Mintlify anchor audit compatibilitySteps
Expected
Actual
pnpm docs:check-mdxpassed.pnpm docs:check-linkspassed:checked_internal_links=3587,broken_links=0.pnpm docs:check-links:anchorsis not a clean lane gate in this worktree: default Node is v25 and Mintlify refuses Node 25+; under temporary Node 22 the edited RFC parses cleanly and the audit then fails on unrelated existing docsplugins/sdk-overview.mdandtools/exec-approvals.md.pnpm check:changedinitially passed before the final MDX parse wording fix; final exact-state rerun re-ran docs gates successfully, then failed only in out-of-scopeextensions/amazon-bedrock/index.test.tsduring all-lanes expansion caused by visible pre-existing.agents/skills/...surfaces. Immediate targeted rerunpnpm test extensions/amazon-bedrock/index.test.tspassed all 22 tests.Evidence
See
tmp-drop-me-gpt2-rfc-fixup.mdfor checkpointed verification receipts and source-read notes.Human Verification (required)
What you personally verified (not just CI), and how:
targetSessionKeyis documented as descriptor-present/runtime-pending rather than shipped explicit runtime return; continue_delegate: multi-recipient return primitive (one completion → N receivers) #355targetSessionKeys: string[]is documented as future/design-locked active-development shape, not shipped behavior.Review Conversations
Compatibility / Migration
Risks and Mitigations