feat(memory): inject ADR-012 cycle-reflection trailer into all heartbeat templates - #295
Closed
samxu01 wants to merge 2 commits into
Closed
feat(memory): inject ADR-012 cycle-reflection trailer into all heartbeat templates#295samxu01 wants to merge 2 commits into
samxu01 wants to merge 2 commits into
Conversation
…eat templates
Empirical finding from the post-deploy observation window: the §10.3
inline cue prepended to scheduler heartbeat payload.content was NOT
sufficient to produce cycle writes. Five heartbeats acked across the
cluster after Phase 2 deploy with 0 new agent-authored cycles[]
entries — agents follow HEARTBEAT.md's structured steps verbatim and
deprioritize the inline narrative directive.
Fix: add the cycle-write directive to HEARTBEAT.md itself — the file
the model treats as authoritative. Implemented as a single chokepoint
helper (`withCyclesDirective`) applied at provision/reprovision time:
- New `withCyclesDirective(template)` in presets.ts: appends a uniform
"## Memory cycle reflection" trailer to any heartbeat template.
Idempotent (no-op if the trailer is already present); preserves the
customizations.heartbeat flag's "user edited" semantics by leaving
the 25 individual template strings untouched.
- Wrapped all four call-sites in provision.ts and reprovision.ts that
read matchedPreset.heartbeatTemplate (provisioner + AgentProfile
cache).
Trailer instructs: append a ≤500-char takeaway via
commonly_save_my_memory({sections:{cycles:{append:{content}}}}) just
before HEARTBEAT_OK; skip if nothing memorable; specific over generic;
past entries surface in cyclesDigest on every future event so writing
isn't wasted.
Single point of injection means new presets automatically get the
directive without per-preset edits. reprovision-all on dev pushes the
new HEARTBEAT.md to all 25 agent PVCs in one pass.
…lifecycle Folds Phase 3 docs into the same PR as the §10.3 trailer-inject helper. - Event Queue: document pending → delivered → acked lifecycle, mutate-on-claim in list(), status-gated ack with monotone $max bump on lastSeenRevision. - New 'Memory contract (ADR-012)' section: sections envelope (with system_exchanges + cycles writer/reader/cap matrix), cycles append shape, four-field event payload digest, revision contract, invariant 8a carve-out. - Document the HEARTBEAT.md trailer auto-inject as the single chokepoint for the cycle-write directive.
samxu01
added a commit
that referenced
this pull request
May 4, 2026
…eat templates (#295) Empirical finding from the post-Phase-2 observation window: the §10.3 inline cue prepended to scheduler heartbeat payload.content was NOT sufficient to produce cycle writes. Five heartbeats acked across the cluster after Phase 2 deploy with 0 new agent-authored cycles[] entries. Agents follow HEARTBEAT.md's structured steps verbatim and deprioritize the inline narrative directive — confirms the hierarchy: authoritative workspace file > inline payload.content > structured metadata. Fix: add the cycle-write directive to HEARTBEAT.md itself, where the model treats it as authoritative. Implemented as a single chokepoint helper applied at provision/reprovision time rather than editing 25 individual heartbeatTemplate strings. - New `withCyclesDirective(template)` in presets.ts: appends a uniform '## Memory cycle reflection' trailer to any heartbeat template. Idempotent (no-op if the trailer is already present); preserves the customizations.heartbeat flag's 'user edited' semantics by leaving the 25 individual template strings untouched. - Wrapped all four call-sites in provision.ts + reprovision.ts that read matchedPreset.heartbeatTemplate (provisioner + AgentProfile cache). Trailer instructs: append ≤500-char takeaway via commonly_save_my_memory({sections:{cycles:{append:{content}}}}) just before HEARTBEAT_OK; skip if nothing memorable; specific over generic; past entries surface in cyclesDigest on every future event so writing isn't wasted. Single point of injection means new presets automatically get the directive without per-preset edits. reprovision-all on dev pushes the new HEARTBEAT.md to all 25 agent PVCs in one pass. Folds in ADR-012 Phase 3 docs: - AGENT_RUNTIME.md: new 'Memory contract (ADR-012)' section + Event Queue lifecycle subsection (mutate-on-claim, monotone \$max bump, dup-ack semantics).
Contributor
Author
|
Squash-merged to main as 4097a8d. |
2 tasks
samxu01
added a commit
that referenced
this pull request
May 4, 2026
Phase 2.J's withCyclesDirective trailer (PR #295) instructed agents to call commonly_save_my_memory({sections:{cycles:{append:...}}}), but no openclaw extension tool with that name exists today. Agents waste 3+ turn-budget tool calls per heartbeat hunting for it before falling back to the real commonly_write_agent_memory, which exhausts their budget before they can post a chat reply. Verified on Nova: silent on Sam's "hey Nova" (06:01 UTC) and "hey" (06:21 UTC) DMs immediately after the trailer rolled out via reprovision-all. Pre-rollout, Nova was responding cleanly to four prior pings within minutes. Session log /state/agents/nova/sessions/*.jsonl confirms 3× attempted 'commonly_save_my_memory' tool calls (missing tool name) before 1× successful 'commonly_write_agent_memory' fallback. This commit makes withCyclesDirective a no-op so HEARTBEAT.md reverts to its pre-Phase-2.J shape on the next reprovision-all. The helper + its four call-sites stay in place so re-enabling is a one-line flip once the openclaw extension exposes a real cycles-write tool. Forward fix path (separate branch): add commonly_log_cycle(content, podId?) to Team-Commonly/openclaw fork, build + push gateway image, update trailer to call the new tool name, flip this back to the real implementation, reprovision-all + observe.
4 tasks
samxu01
pushed a commit
that referenced
this pull request
Aug 4, 2026
…e ADR surface @sprint-review found that this PR's central claim was false. The header said "the test beside this file pins the tool name so the two cannot silently diverge again." It pinned the module's CONSTANT. Nothing pinned that the scheduler still calls it — and PR #295 was a *delivery* failure, a stale inline string in the scheduler, not a wrong constant. Reproduced: revert `content: buildHeartbeatContent(...)` to an inline rolled-back literal and the suite stays 9/9 green with `tsc --noEmit` clean. Extracting the cue into a module made the string testable and, in the same move, made bypassing it a one-line diff no test could see. Adds a `cycle-cue delivery surfaces` block pinning WIRING: - schedulerService requires ./heartbeatCue, calls buildHeartbeatContent, and contains no inline cue literal - the HEARTBEAT.md trailer names the same writer tool The trailer assertion runs against the DELIVERED string via withCyclesDirective(''), not the source text: presets.ts documents the #295 incident in a comment that necessarily quotes the rolled-back shape, so a source grep fails on a deliberate mention. Same distinction the NO_REPLY sentinel draws between a bare token and a quoted one. Also closes the third surface, which the header's "change it in BOTH places" enumeration missed. ADR-012 §10.3 — the section this module cites as its authority — still displayed the rolled-back call as the *canonical* cue, with the correction ~40 lines downstream under "What actually shipped." Visible reading linearly, invisible jumping to the cited section, which is plausibly how the original miss happened and was still armed. Adds a supersede marker there and corrects the three remaining spec-level lines (§10.1 append contract, route-change and event-payload phasing bullets) that named the wrong writer. An enumeration finds gaps only for the members it names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lilyshen0722
added a commit
that referenced
this pull request
Aug 4, 2026
…ack in May (#818) * fix(heartbeat): the inline cue still names the tool that was rolled back in May Every scheduled heartbeat tells the agent to append its cycle takeaway via commonly_save_my_memory({ sections: { cycles: { append: { content } } } }) That call cannot be made. `commonly_save_my_memory` accepts neither the `cycles` section (not in its section list) nor the nested shape (`additionalProperties: false`, no `append`). The writer is `commonly_log_cycle({ content, podId? })`. This is not a new bug. It shipped in PR #295 on 2026-05-04, agents burned 3+ tool-call turns per heartbeat hunting for the missing surface and ran out of turn budget mid-conversation — Nova missed DM responses that day — and it was rolled back the same week. routes/registry/presets.ts carries the whole incident in a comment above its own, correct, HEARTBEAT.md trailer: "use commonly_log_cycle for every write." The forward fix landed on the template surface and never landed here. That inversion is the actual finding. By ADR-012 §10.3's own reasoning — quoted in the code this replaces — the inline cue in payload.content beats structured metadata for behavior steering, which makes it the STRONGEST heartbeat surface. So the corrected instruction sat in HEARTBEAT.md, a moltbot PVC artifact that ADR-005 wrapper seats do not even have, while the surface that wins by design kept the rolled-back one. Reproduced today, 2026-08-04: a seat followed the cue, got `400 unknown section: undefined`, and burned the call. Three months. Moved to services/heartbeatCue.ts with a test, rather than fixing the string in place. It is a contract with every agent, it has now drifted from its sibling surface once at measured cost, and it had no test at any tier — inline in an IIFE inside schedulerService, whose import graph makes it untestable in practice. Now it has a name, a home, and a guard. Cue also gained two clauses that cost nothing and close the same class: it names commonly_save_my_memory as the tool that does NOT own cycles (the cheapest way to stop the next agent hunting — an error that names the payload but not the owner reads as "you called this tool wrong" when the truth is "you called the wrong tool"), and it states that the 500-char cap truncates silently while still returning ok, so a takeaway's conclusion is not quietly deleted. 9 tests. Mutation-verified: restore the exact PR #295 cue 4 fail swap the writer to commonly_write_agent_memory 1 fail The second is the one that matters — it is the fallback agents actually reached for in the 2026-05-04 incident, and any looser assertion ("the cue mentions cycles") passes it. The test pins the owner, because naming the wrong owner was the defect. Typecheck 57 before and after — all pre-existing, 0 in these files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(heartbeat): the cue asserted a fact #804 inverts, and a test pinned it The truncation clause read "the cap truncates silently and still returns ok, so confirm by reading your memory back rather than by the response." That is true against main and false the moment #804 lands: #804 adds `truncated` / `evicted` / `entryCap` / `retainedEntries` to the write response. After it, this cue instructs every agent, on every heartbeat, to distrust the exact field #804 built to be trusted — the same false-model defect this cue exists to fix, one clause over. Worse, the test asserted /truncates silently/, so it PINNED the claim. A textual merge that keeps this file's structure (which is the better structure) keeps the assertion green while the sentence it defends turns into a lie. A green test guarding a statement another branch is making false is worse than no test there at all. Fix is to say what holds in both worlds — state the cap, stop — and to pin the ABSENCE of any claim about how truncation is reported, so re-adding one has to argue with a test. This also drops the semantic half of the #804/#818 conflict: what remains is textual, and either merge order now yields a true cue. Found by @ux-lead, who spotted that the two cue texts assert opposite facts about truncation rather than merely colliding on the same lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(heartbeat): pin the delivery, not just the constant, and close the ADR surface @sprint-review found that this PR's central claim was false. The header said "the test beside this file pins the tool name so the two cannot silently diverge again." It pinned the module's CONSTANT. Nothing pinned that the scheduler still calls it — and PR #295 was a *delivery* failure, a stale inline string in the scheduler, not a wrong constant. Reproduced: revert `content: buildHeartbeatContent(...)` to an inline rolled-back literal and the suite stays 9/9 green with `tsc --noEmit` clean. Extracting the cue into a module made the string testable and, in the same move, made bypassing it a one-line diff no test could see. Adds a `cycle-cue delivery surfaces` block pinning WIRING: - schedulerService requires ./heartbeatCue, calls buildHeartbeatContent, and contains no inline cue literal - the HEARTBEAT.md trailer names the same writer tool The trailer assertion runs against the DELIVERED string via withCyclesDirective(''), not the source text: presets.ts documents the #295 incident in a comment that necessarily quotes the rolled-back shape, so a source grep fails on a deliberate mention. Same distinction the NO_REPLY sentinel draws between a bare token and a quoted one. Also closes the third surface, which the header's "change it in BOTH places" enumeration missed. ADR-012 §10.3 — the section this module cites as its authority — still displayed the rolled-back call as the *canonical* cue, with the correction ~40 lines downstream under "What actually shipped." Visible reading linearly, invisible jumping to the cited section, which is plausibly how the original miss happened and was still armed. Adds a supersede marker there and corrects the three remaining spec-level lines (§10.1 append contract, route-change and event-payload phasing bullets) that named the wrong writer. An enumeration finds gaps only for the members it names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cues): the mention frame named two tools MCP agents do not have @ux-lead noticed their own per-turn pod context instructing them to call `commonly_open_dm` and `commonly_read_attachment` — neither resolves on their seat. Verified: buildContentForTarget ships these cues to every agent with NO driver-class branch, so one namespace's names reach the whole fleet. commonly_read_attachment exists nowhere — not in @commonlyai/mcp, not in this repo outside the sentence naming it. MCP has commonly_read_file. commonly_open_dm openclaw-extension only (live 11878b43c). MCP exposes the same capability as commonly_dm_agent — and every ADR-005 wrapper and cloud-codex seat is an MCP consumer. This is PR #818's defect one layer up and on a far wider surface: the heartbeat cue misfires per tick, this misfires on every mention to every agent. An instruction must name a tool that can serve it. Fix names the verified tool for the file read, and names BOTH DM tools since the call site has no notion of driver class — the same "one knob, two driver classes" shape @ux-lead found in the agentEventService requeue, where `pending` means redelivery to pull drivers and a deletion countdown to push drivers. Guard asserts against the DELIVERED payload, not the source text: every `commonly_*` token in an enqueued mention must be in a provenance-tagged allow-list (which surface provides it), plus a non-vacuity control so a cue that stops naming tools can't pass by emptiness. That control is the #818 lesson — pinning the constant left the delivery unpinned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(cues): pin that commonly_open_dm is never named unqualified The allow-list in this describe block treats `commonly_open_dm` as a known tool, which it is — for openclaw seats. So a cue reading "open a DM with commonly_open_dm", with no runtime qualifier, passes that assertion identically to the corrected text. The allow-list cannot guard the defect this PR fixes, because the defect is not an unknown token. The property is sentence-level: the openclaw-only name may appear, but never unqualified. Asserted on the delivered payload, with a control on the match count so zero occurrences can't pass vacuously. Mutation-verified: stripping "on openclaw runtimes" from both cues turns this red while the existing allow-list and dm_agent assertions stay green — which is the gap it exists to close. Suggested by @ux-lead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * style(cues): drop the aligned trailing comments that tripped no-multi-spaces Comment-only. The provenance they carried moves to the line above, so the annotation survives without the alignment lint objects to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(mentions): derive the tool inventory from the doc it cites CI caught this branch's own new guard failing — and the failure was a false positive, which is the more useful outcome. 'every commonly_* tool in a delivered mention payload is a real tool' hand- listed twelve MCP tools under a comment naming docs/MCP_INTEGRATION.md as their source. That doc lists twenty-six. So the guard called commonly_get_messages — shipped, documented, and the tool #798 fixed pagination for — a tool that does not exist, the moment a cue naming it reached the payload via #815. A guard against drift that keeps its own copy of the thing it guards IS the defect it exists to catch, one level up. Same shape as this PR's own lesson (an extracted cue gone stale against the delivered one) and as ADR-016's rule that a creation gate must consult the DM predicate rather than a hand-kept allowlist that happens to agree with it. Now reads the doc at test time. Added a companion test asserting the inventory actually loaded — an allowlist that silently reads empty would pass every cue and prove nothing. 41/41 in this suite. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
lilyshen0722
added a commit
that referenced
this pull request
Aug 5, 2026
) * fix(ax): the tool-contract guard skipped the strongest cue surface The guard reads the presets.ts cycles trailer and the inline mention cues. It did not read backend/services/heartbeatCue.ts — which by ADR-012 s10.3's own reasoning is the strongest agent-facing surface we ship, reaching every agent on every tick. That gap had already been paid for. The heartbeat cue named commonly_save_my_memory for a `cycles` write from 2026-05-03 (#293) to 2026-08-04 (#804/#818) — a tool that refuses the section by design — and neither repo's suite could see the other, so both stayed green for three months. A peer re-derived the same contradiction from source on 2026-08-05, hours after it was fixed, which is what surfaced the omission. Adds the cue as a required-tool source. The constant interpolates ${CYCLES_WRITER_TOOL}, so it is resolved from the same file rather than restated here — a restated name is the drift this script exists to catch. No namedForOtherDrivers exemption: #840 reconciled the lineages and the pin declares commonly_log_cycle, so this is a hard requirement and a future bump that drops it goes red instead of the fleet going quiet. Two assertions the name-matching check cannot make, because existence is not capability — the #295 cue would PASS a declares-the-tool check against today's pin, since commonly_save_my_memory is declared at :526 right above a description saying `cycles` is intentionally unavailable there: - the cue names exactly one writer tool, so a cue that also names a second (the #295 shape, and the shape a well-meaning "mention both" edit reproduces) fails - the escape-hatch clause survives. It looks redundant now that the pin has the tool and would be the first thing an editor cuts; deleting it re-arms the turn-burn that forced the #296 rollback the next time a bump drops it. Mutation-verified: each of the three fails exactly its own assertion, with measured anchors and byte-identical restores. CLAUDE.md's pin-skew entry described the divergence in the present tense and prescribed the fix #840 had already executed. It cost a wrong claim in the sprint pod nine hours later. Dated, struck, and led with the resolution; verified against the running gateway container rather than the source tree. AX entry 21: remediation text quotes the defect verbatim, so a repo accumulates copies of what is no longer true in the surfaces agents grep first. Six verbatim copies of the dead cue, zero live ones — and the only two queued events matching it were the peer's own reports about it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: the cycles silence is moltbot-scoped, and dated by measurement The pin-skew entry asserted the skew caused "the fleet's" cycles silence with an undocumented ~87-day figure. Measured against the memory collection 2026-08-05: moltbots (agentName=openclaw) 22 docs with entries, newest 2026-05-24T08:49:56Z non-moltbot seats 13 docs with entries, newest same evening The moltbot cutoff falls on the exact day of the d6e63b2 bundled-skills bump that dropped commonly_log_cycle — so the root cause is corroborated by a source independent of the submodule log, rather than inferred from it. Two corrections this forces. The silence is moltbot-scoped, not fleet-wide: MCP and wrapper seats wrote cycles throughout, including during the outage being debugged. And every moltbot array sits at the 40-entry cap — they are FULL, not empty — so a recovery check looking for "cycles appear" reads healthy off both the cap and the unaffected seats. It has to filter to agentName=openclaw and require an entry newer than 05-24. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 2 (#293) closed the read loop at the protocol level but the §10.3 inline cue on `payload.content` was empirically insufficient — 0 new `cycles[]` writes across 25 agents in the post-deploy window. Agents follow `HEARTBEAT.md`'s structured steps verbatim and deprioritize the inline narrative directive.
This PR moves the cycle-write directive into `HEARTBEAT.md` itself, where the model treats it as authoritative.
Approach: single chokepoint
Instead of editing 25 individual `heartbeatTemplate` strings in `presets.ts`:
New presets automatically inherit the directive. The 25 raw template strings stay untouched, so the `customizations.heartbeat` flag's "user edited the template" semantics are preserved.
Test plan
🤖 Generated with Claude Code