Skip to content

feat(openclaw): merge inline retain tags with defaults#948

Open
Aldoustheorchestrator wants to merge 7 commits intovectorize-io:mainfrom
Aldoustheorchestrator:aldous/openclaw-retain-parity-pr6290
Open

feat(openclaw): merge inline retain tags with defaults#948
Aldoustheorchestrator wants to merge 7 commits intovectorize-io:mainfrom
Aldoustheorchestrator:aldous/openclaw-retain-parity-pr6290

Conversation

@Aldoustheorchestrator
Copy link
Copy Markdown
Contributor

@Aldoustheorchestrator Aldoustheorchestrator commented Apr 9, 2026

Summary

  • merge configured retainTags with inline per-message retain tags from user content
  • keep retain-tag normalization strict so non-string config values are dropped
  • preserve the existing structured transcript format while stripping tag directives from stored content

Why

There are two useful retain-tag layers here:

  • stable defaults from config, such as source_system:openclaw
  • context-specific tags attached at retain time, such as client:acme or type:decision

OpenClaw should support both. Default tags provide consistent source labeling, while per-message tags let retained memories capture immediate context at the moment they are stored.

For OpenClaw, the clean runtime caller is a per-message override: user messages can include an inline retain-tag directive, and auto-retain merges those tags with configured defaults when building the retain request.

What changed

  • keep normalizeRetainTags(...) strict: trim, deduplicate, preserve order, and reject non-string values
  • add inline retain-tag parsing for user messages via:
    • <retain_tags>...</retain_tags>
    • <hindsight_retain_tags>...</hindsight_retain_tags>
  • merge configured retainTags first, then inline per-message tags
  • strip inline retain-tag directives back out before transcript storage so they do not pollute retained content
  • leave the existing [role: ...] ... [role:end] transcript structure intact
  • add tests for inline tag extraction, merged tag behavior, and directive stripping

Why this shape

This addresses the review concern about the old options.tags path having no runtime caller.

The caller is now real and production-facing: user messages can provide per-message retain tags, and the auto-retain hook threads those through to buildRetainRequest(...).

Testing

  • npm test -- src/index.test.ts in hindsight-integrations/openclaw

Follow-up

@Aldoustheorchestrator
Copy link
Copy Markdown
Contributor Author

Heads up on the current red CI: the failing build-openclaw-integration job appears unrelated to this retain-tags patch.

The failure is in hindsight-integrations/openclaw/src/backfill.test.ts (treats a symlinked bin path as direct execution), which exercises isDirectExecution(...) in the backfill CLI entrypoint rather than the retain path touched here.

Locally, the retain-focused tests for this change pass, and the only failures I have seen are the same pre-existing backfill/symlink-path issue. I would prefer to keep this PR narrowly scoped and fix that CLI portability problem separately unless maintainers want it bundled.

@nicoloboschi
Copy link
Copy Markdown
Collaborator

Small, mostly sensible change, but a couple of issues worth addressing before merge.

Must-fix

  1. The new options.tags parameter has no caller. The only production call site (src/index.ts:1505) does not pass tags — it's only exercised in the unit test. So this PR ships a public API surface with zero runtime effect. Either wire it up to a real caller (plugin hook / per-message override) or drop it. "Gives downstream callers a clean path" isn't enough justification on its own.

  2. normalizeRetainTags silently accepts non-string config. Old code filtered with typeof tag === 'string'. New code does String(item ?? '').trim(), so e.g. {retainTags: [{a:1}]} now produces the literal tag \"[object Object]\" instead of being dropped. That's a regression vs. the prior behavior. Either keep the type filter or reject non-strings explicitly.

Nits / scope

  1. The ternary → if rewrite in prepareRetentionTranscript is pure churn and contradicts the "intentionally narrow patch" framing. Please drop it.

  2. The PR body cites "Abner's Hermes PR (feat(hindsight): add richer session-scoped retain metadata NousResearch/hermes-agent#6290)" as motivation — I can't find that reference. If there's a real upstream PR driving this, please link it; otherwise it's fine to just motivate the change on its own merits.

  3. Pre-existing backfill.test.ts failures are acknowledged but not tracked — please file an issue so they don't rot.

Looks fine: dedup logic, order preservation (config tags first, per-call tags appended), test for the merged case, type comment update.

@Aldoustheorchestrator Aldoustheorchestrator changed the title feat(openclaw): merge retain tags for parity with Hermes fix(openclaw): normalize configured retain tags Apr 13, 2026
@Aldoustheorchestrator
Copy link
Copy Markdown
Contributor Author

Aldoustheorchestrator commented Apr 13, 2026

Addressed.

The merged-tag behavior has a real runtime caller instead of a test-only option.

  • keeps configured retainTags normalization strict, so non-string values are dropped rather than stringified
  • allows per-message retain tags in user content via <retain_tags>...</retain_tags> or <hindsight_retain_tags>...</hindsight_retain_tags>
  • merges configured retainTags first, then the inline per-message tags, with deduplication and stable ordering
  • strips those inline tag directives back out before storing the retained transcript
  • leaves prepareRetentionTranscript(...) in the same structured transcript format

That gives the merged-tag behavior a real OpenClaw-side caller: the user message being auto-retained.

I also filed #1033 for the separate backfill symlink-path failure so that issue remains tracked independently.

Tests run locally:

  • npm test -- src/index.test.ts in hindsight-integrations/openclaw

@Aldoustheorchestrator Aldoustheorchestrator changed the title fix(openclaw): normalize configured retain tags feat(openclaw): merge inline retain tags with defaults Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants