Skip to content

fix(openclaw): mission semantics + retainQueue config whitelist - #1473

Merged
nicoloboschi merged 1 commit into
mainfrom
fix/openclaw-mission-and-retain-queue
May 6, 2026
Merged

fix(openclaw): mission semantics + retainQueue config whitelist#1473
nicoloboschi merged 1 commit into
mainfrom
fix/openclaw-mission-and-retain-queue

Conversation

@nicoloboschi

Copy link
Copy Markdown
Collaborator

Summary

Fixes three triaged openclaw plugin issues. Each one is small, self-contained, and unblocked.

#1270bankMission default no longer clobbers per-bank missions

Before: every gateway restart called createBank({ reflectMission: defaultMission }) for the active bank, overwriting any per-bank mission written out-of-band via PATCH /banks/{id}. Setting bankMission: "" didn't help because getPluginConfig() substituted the default text on empty.

After: getPluginConfig() returns bankMission only when the user explicitly set a non-empty string. No mission, no stamping. Per-bank API patches survive across restarts.

#1353retainMission and observationsMission plugin config fields

Previously the plugin only exposed bankMission, which writes to reflect_mission and only affects /reflect — but the README claimed it helped fact extraction. That was wrong: retain ignores reflect_mission.

This PR:

  • Exposes retainMission (steers retain extraction) and observationsMission (steers observation consolidation) as plugin config fields.
  • Each maps to its respective bank-config column on first use of the bank.
  • README updated to clarify what each mission actually affects.

#1443retainQueue.* config keys reach the queue

retainQueuePath, retainQueueMaxAgeMs, and retainQueueFlushIntervalMs were declared in openclaw.plugin.json and read at queue init, but the strict whitelist in getPluginConfig() silently dropped them — so the queue always used ~/.openclaw/data/hindsight-retain-queue.jsonl regardless of config. Added all three to the whitelist with the same string/number validation as adjacent fields.

Implementation notes

  • Mission stamping is centralised in applyConfiguredMissions() + hasConfiguredMissions(). Six ad-hoc setMission(mission) call sites collapsed into a single helper that no-ops when nothing is configured. The single createBank call carries whichever subset of {reflectMission, retainMission, observationsMission} the user set; the server's get_config_updates() already skips None fields, so unset missions don't touch existing values.
  • BankScopedClient.setMission(string)setMissions(BankMissionsUpdate). Internal facade — no published-API impact, all in-repo call sites migrated.
  • getPluginConfig() exported for direct unit-testability (matches the export pattern of other helpers in the same file).

Test plan

  • npm test — 207 tests pass (14 new covering whitelist + mission semantics)
  • npm run build — tsc clean
  • ./scripts/hooks/lint.sh — clean
  • Manually verify bankMission: "" no longer overwrites a PATCHed per-bank mission across a gateway restart
  • Manually verify retainQueuePath: "/custom/path" produces the queue file at the configured location

Closes #1270
Closes #1353
Closes #1443

Out of scope (also from the same triage):

Addresses three triaged issues against the openclaw plugin:

- #1270: Stop substituting a default `bankMission` when none is configured.
  Previously every gateway restart re-stamped the default text via
  `createBank({reflectMission})`, clobbering per-bank missions written
  out-of-band via `PATCH /banks/{id}`. Empty/unset is now a true opt-out.

- #1353: Expose `retainMission` and `observationsMission` plugin config
  fields. They each map to the matching bank-config column on first use,
  so users can steer retain extraction and observation consolidation
  declaratively in `openclaw.json` instead of patching the bank API
  out-of-band. README clarified that `bankMission` only affects reflect.

- #1443: Add `retainQueuePath`, `retainQueueMaxAgeMs`, and
  `retainQueueFlushIntervalMs` to the `getPluginConfig()` whitelist.
  These keys were declared in the plugin schema and read by queue init,
  but the strict whitelist silently dropped them — so the queue always
  used the hardcoded default path regardless of user config.

Mission stamping is now centralised in `applyConfiguredMissions()` and
gated by `hasConfiguredMissions()`, replacing six ad-hoc `setMission`
call sites with a single helper that no-ops when nothing is configured.
@nicoloboschi
nicoloboschi merged commit aca0383 into main May 6, 2026
54 of 55 checks passed
nicoloboschi added a commit that referenced this pull request May 6, 2026
OpenClaw's plugin loader runs configSchema validation with
`additionalProperties: false`, so any PluginConfig field not declared
in openclaw.plugin.json is silently rejected at config-set time. The
manifest had drifted from the type:

- retainMission, observationsMission (added in #1473) — never declared
- debugPerfTiming (added earlier in this PR) — never declared
- retainDocumentScope — pre-existing gap, declared now
- enableKnowledgeTools — was in configSchema but missing from uiHints

All five are now in both configSchema.properties and uiHints. Also
fixed the bankMission description to match the corrected README from
#1353 (only affects /reflect, not retain).

Added a manifest.test.ts parity test that compares the type's keys to
the manifest's declared keys and fails on either side of drift. This
is the same class of bug as #1443 (whitelist drift) — having a test
prevents the next round.
nicoloboschi added a commit that referenced this pull request May 6, 2026
…ng (#1477)

* feat(openclaw): drop redundant before_agent_start hook + add debugPerfTiming

Two unrelated-but-tiny openclaw improvements:

- #1354: Stop registering `before_agent_start`. Its body only called
  `resolveAndCacheIdentity()` + emitted a debug log. The same identity
  resolution already happens in `before_dispatch` (earlier in the
  inbound path), `before_prompt_build` (re-resolves before recall, can
  infer senderId from prompt content), and `agent_end` (re-resolves
  before retain). Subscribing here was duplicate work on the hot path.

- #1406: Add `debugPerfTiming?: boolean` plugin config flag (default
  false). When enabled, the plugin emits one info-level perf line per
  recall path and per retain path:

    perf: before_prompt_build hook_total=4200ms recall_main=3800ms source=fresh results=3
    perf: agent_end hook_total=1200ms retain=1100ms outcome=ok bank=main messages=4

  Lets users diagnose latency without patching the dist. The
  `source=fresh|reused` field reflects in-flight recall dedup; the
  `outcome=ok|queued|error` field reflects whether retain succeeded
  inline, was queued for retry, or failed outright.

Also fixes a stale comment that referenced before_agent_start where the
actual lifecycle stage is before_prompt_build.

* fix(openclaw): sync manifest with PluginConfig type + add parity test

OpenClaw's plugin loader runs configSchema validation with
`additionalProperties: false`, so any PluginConfig field not declared
in openclaw.plugin.json is silently rejected at config-set time. The
manifest had drifted from the type:

- retainMission, observationsMission (added in #1473) — never declared
- debugPerfTiming (added earlier in this PR) — never declared
- retainDocumentScope — pre-existing gap, declared now
- enableKnowledgeTools — was in configSchema but missing from uiHints

All five are now in both configSchema.properties and uiHints. Also
fixed the bankMission description to match the corrected README from
#1353 (only affects /reflect, not retain).

Added a manifest.test.ts parity test that compares the type's keys to
the manifest's declared keys and fails on either side of drift. This
is the same class of bug as #1443 (whitelist drift) — having a test
prevents the next round.
nepenth pushed a commit to nepenth/hindsight that referenced this pull request Aug 7, 2026
OpenClaw's plugin loader runs configSchema validation with
`additionalProperties: false`, so any PluginConfig field not declared
in openclaw.plugin.json is silently rejected at config-set time. The
manifest had drifted from the type:

- retainMission, observationsMission (added in vectorize-io#1473) — never declared
- debugPerfTiming (added earlier in this PR) — never declared
- retainDocumentScope — pre-existing gap, declared now
- enableKnowledgeTools — was in configSchema but missing from uiHints

All five are now in both configSchema.properties and uiHints. Also
fixed the bankMission description to match the corrected README from
vectorize-io#1353 (only affects /reflect, not retain).

Added a manifest.test.ts parity test that compares the type's keys to
the manifest's declared keys and fails on either side of drift. This
is the same class of bug as vectorize-io#1443 (whitelist drift) — having a test
prevents the next round.
nepenth pushed a commit to nepenth/hindsight that referenced this pull request Aug 7, 2026
OpenClaw's plugin loader runs configSchema validation with
`additionalProperties: false`, so any PluginConfig field not declared
in openclaw.plugin.json is silently rejected at config-set time. The
manifest had drifted from the type:

- retainMission, observationsMission (added in vectorize-io#1473) — never declared
- debugPerfTiming (added earlier in this PR) — never declared
- retainDocumentScope — pre-existing gap, declared now
- enableKnowledgeTools — was in configSchema but missing from uiHints

All five are now in both configSchema.properties and uiHints. Also
fixed the bankMission description to match the corrected README from
vectorize-io#1353 (only affects /reflect, not retain).

Added a manifest.test.ts parity test that compares the type's keys to
the manifest's declared keys and fails on either side of drift. This
is the same class of bug as vectorize-io#1443 (whitelist drift) — having a test
prevents the next round.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment