refactor(config): config-surface reduction tranche 1 — retire dead keys, dedupe channel schemas, add growth ratchet#111142
Conversation
openclaw-barnacle
Bot
added
docs
Security-sensitive changes notedThis PR includes security-sensitive file changes. The guard is informational because the PR author is a repository admin or a member of
Changed files:
Security review is still recommended before merge when the change is intentional. |
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 3:39 AM ET / 07:39 UTC. Summary Reproducibility: yes. from source: compile an external plugin that imports Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Keep runtime configuration canonical, restore Do we have a high-confidence way to reproduce the issue? Yes, from source: compile an external plugin that imports Is this the best way to solve the issue? No: removing the retired runtime key is compatible with preserving a deprecated type-only SDK alias, which is the narrower way to avoid breaking external plugin compilation. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 53d600ab8337. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (6 earlier review cycles)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
openclaw/src/config/channel-alias-migration.ts
Lines 172 to 174 in 9af9a1d
Remove conflicting nested DM aliases during migration
When a Discord, Slack, or Google Chat entry contains both a canonical DM field and a different legacy nested value—for example dmPolicy: "pairing" with dm.policy: "open"—this path invokes normalizeLegacyDmAliases, but that helper only deletes the alias when both values are equal (src/channels/plugins/dm-access.ts:261-298). The conflicting alias therefore remains even though the new strict schemas reject it, so openclaw doctor --fix cannot produce a valid config and affected upgrades remain blocked; canonical values should win while the nested fields are always removed.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const GoogleChatDmSchema = z | ||
| .object({ | ||
| enabled: z.boolean().optional(), | ||
| policy: DmPolicySchema.optional().default("pairing"), | ||
| allowFrom: z.array(z.union([z.string(), z.number()])).optional(), | ||
| }) |
There was a problem hiding this comment.
When the root Google Chat DM policy is open and a named account has dm: { enabled: true } without an explicit policy, the previous account schema materialized dm.policy: "pairing", so that account stayed pairing-only despite the open root. Removing this default while retaining the nested dm object makes doctor migrate only the root policy to dmPolicy; the named account then inherits open through resolveGoogleChatAccount, allowing arbitrary DMs on an account that previously required pairing. The migration or account schema needs to preserve the former per-account pairing default.
Useful? React with 👍 / 👎.
9af9a1d to
719213e
Compare
clawsweeper
Bot
removed
the
rating: 🦐 gold shrimp
377e50c to
54b34e7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54b34e725f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** @deprecated Use `direct` instead. Kept for backward compatibility. */ | ||
| z.literal("dm"), | ||
| ]), | ||
| kind: z.union([z.literal("direct"), z.literal("group"), z.literal("channel")]), |
There was a problem hiding this comment.
When an existing config contains the previously supported bindings[].match.peer.kind: "dm", this enum now rejects the entire config, and no doctor migration rewrites the value to direct. The compatibility contract is still documented in src/config/schema.help.runtime.ts:349-350, while normalizeChatType and the compatibility tests in src/routing/resolve-route.test.ts:840-850 still treat dm as direct; therefore affected upgrades cannot reach the runtime normalization or converge through doctor --fix. Add a raw-config migration before removing this enum member.
Useful? React with 👍 / 👎.
| const AllowDenyChatTypeSchema = z | ||
| .union([ | ||
| z.literal("direct"), | ||
| z.literal("group"), | ||
| z.literal("channel"), | ||
| /** @deprecated Use `direct` instead. Kept for backward compatibility. */ | ||
| z.literal("dm"), | ||
| ]) | ||
| .union([z.literal("direct"), z.literal("group"), z.literal("channel")]) | ||
| .optional(); |
There was a problem hiding this comment.
When a shipped config uses the documented chatType: "dm" alias in session.sendPolicy.rules—or in the memory/media scope surfaces backed by this shared schema—validation now fails before the existing runtime normalization can map it to direct. docs/gateway/config-agents.md:1327 still advertises this alias, and both src/sessions/send-policy.ts:127 and src/media-understanding/scope.ts:50 normalize it, but none of the doctor migrations rewrites these rule arrays, so affected users cannot repair the upgrade with doctor --fix. Migrate these matches to direct before narrowing the schema.
Useful? React with 👍 / 👎.
54b34e7 to
053d5fb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 053d5fb028
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const requestTimeoutSeconds = value.timeout; | ||
| if ( | ||
| next.requestTimeoutMs === undefined && | ||
| typeof requestTimeoutSeconds === "number" && | ||
| requestTimeoutSeconds > 0 && |
There was a problem hiding this comment.
When a Hermes MCP server specifies timeout: "${MCP_TIMEOUT}", this number-only branch skips the value. Since this change also removes timeout from MCP_CONNECTION_FIELDS, importsMcpSensitiveValues and mcpManualItems no longer resolve the reference or emit the previous manual-review warning, so the migration silently drops the configured request timeout. Resolve the alias before converting it to milliseconds, or retain it in the interpolation/warning inputs.
Useful? React with 👍 / 👎.
053d5fb to
9a68886
Compare
clawsweeper
Bot
added
rating: 🦪 silver shellfish
9a68886 to
a5434cf
Compare
clawsweeper
Bot
added
rating: 🧂 unranked krab
a5434cf to
b6b7b60
Compare
clawsweeper
Bot
added
rating: 🦪 silver shellfish
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6b7b60545
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| account: accountConfig as Record<string, unknown> | undefined, | ||
| parent: rootConfig as Record<string, unknown> | undefined, | ||
| }); | ||
| const allowFrom = accountConfig?.allowFrom ?? rootConfig?.allowFrom; |
There was a problem hiding this comment.
When operators follow the current documentation, they are still told that Discord and Slack nested DM aliases are runtime-compatible (docs/channels/discord.md:437, docs/channels/slack.md:1235, and docs/channels/slack.md:1349), while docs/tools/exec-approvals-advanced.md:316 still instructs Google Chat users to set channels.googlechat.dm.allowFrom. These canonical-only lookups and the new strict schemas now reject or ignore those paths, as locked by src/config/dead-config-keys.test.ts:18-59, so copying the documented configuration can prevent startup. Update these pages to use the top-level canonical keys and describe the old paths as doctor-only migration inputs.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
…ys, dedupe channel schemas, add growth ratchet (openclaw#111142) * refactor(config): retire dead and aliased config keys via doctor migrations * refactor(config): dedupe bundled channel config schemas into shared builders * feat(config): add config-surface count ratchet to doc-baseline check * test(config): drop stale fixtures for retired config keys * fix(doctor): migrate only positive finite MCP timeout aliases * fix(migrate-hermes): emit canonical MCP timeouts only * fix(config): satisfy lint and contract gates
1 participant
What Problem This Solves
The
openclaw.jsonsurface has grown past 10,000 documented config paths (plus ~650OPENCLAW_*env vars) and is growing by roughly 40 paths/day. Dead keys linger after retirement, ~20 identical messaging options are hand-copied into 7 of 9 bundled channel schemas (drifting independently — GoogleChat silently lacked knobs every other channel has), and nothing pushes back on new growth.This PR is the first tranche of the config-surface reduction program (maintainer-directed): retire dead/aliased keys, deduplicate the channel schema layer, and add a ratchet so the surface can only grow deliberately.
Why This Change Was Made
Per AGENTS.md config policy: runtime reads canonical config only; the config/env surface bar is high; prefer removing/consolidating config options when touching these surfaces.
Phase 1 — retirements (all doctor-migrated). Removed keys whose runtime readers are gone or that alias a canonical key: root
tui,canvasHost,defaultModel;cron.webhook;commands.modelsWrite;messages.messagePrefix;tools.media.asyncCompletion;tools.message.allowCrossContextSend;talk/Discordrealtime.voicealiases; media Deepgram option aliases; MCP timeout aliases (connectTimeout/connect_timeout/timeout→ canonicalconnectionTimeoutMs/requestTimeoutMs, seconds→ms preserved);session.maintenance.pruneDays;session.resetByType.dm; Telegramdm.threadRepliesfamily; Slackdm.replyToMode; WhatsAppexposeErrorText; GoogleChatactions.reactions; thread-bindingspawnAcpSessions/spawnSubagentSessionsaliases. Every removal: schema+types+labels+help deleted,openclaw doctor --fixmigration (move value to canonical home or strip no-ops; canonical value wins on conflict), dead-config-keys test, dated deprecation-compat record (window to 2026-09-18), docs updated. Candidates that turned out to have live readers were kept (messages.removeAckAfterReply,cron.webhookToken). Env: deprecatedOPENCLAW_DEBUG_PROXY_DB_PATH/_BLOB_DIRremoved; auto-update gates collapsed to documentedOPENCLAW_NO_AUTO_UPDATE.Phase 2 — channel schema dedup (no user-visible changes except two intentional ones). New internal
buildCommonChannelAccountShape()(generalizing the WhatsApp pattern) now emits the ~20 shared messaging leaves for all eight bundled channel schema families; shared builders for exec approvals, group entries, reactions, allowBots, botLoopProtection, read receipts, name-matching; channel TS types extendCommonChannelMessagingConfig. Intentional surface changes: (1) legacy nesteddm.{policy,allowFrom}retired on Discord/Slack/GoogleChat (doctor promotes to canonical top-level keys); (2) GoogleChat gains the shareddmPolicy/allowFrom/markdown/heartbeat/contextVisibilityknobs it was missing (generic runtime readers verified). All other channel baseline paths byte-equivalent before/after.Phase 0 — growth ratchet.
pnpm config:docs:checknow also enforces a tracked per-kind count budget (docs/.generated/config-baseline.counts.json). Surface growth fails the check until the budget is consciously raised in the same PR; shrinkage must be ratcheted down viaconfig:docs:gen. Runs in the existingworkflow-sanitygenerated-doc-baselines lane and the changed-lanes release-metadata plan.User Impact
openclaw doctor --fixmigrates every retired key (values move to their canonical home; no-ops are stripped with a named change message). Configs that skip doctor and still set retired keys fail validation with a message naming the replacement — consistent with the documented ~2-month migration window.TelegramDmThreadRepliesremoved with its retired key (no callable surface change;plugin-sdk:surface:checkgreen). One non-callable schema re-export added for the shared mention-patterns schema.Evidence
Focused suites (all green): dead-config-keys, schema-regressions, channel-alias-migration, doctor legacy-config migrations + deprecation-compat, per-channel config/doctor-contract suites (Discord/Telegram/Slack/WhatsApp/Signal/iMessage/GoogleChat/IRC/Matrix/LINE), doc-baseline integration (ratchet growth/stale/write modes), MCP transport/CLI, Talk normalization, WhatsApp accounts/inbound.
pnpm tsgo:coreandpnpm tsgo:extensions:testgreen;pnpm config:docs:check,pnpm config:channels:check,pnpm plugin-sdk:surface:checkgreen;git diff --checkclean.Channel baseline path diff verified byte-equivalent outside the 38 intentional path changes (18 removed legacy dm paths, 20 added GoogleChat paths).
Fresh autoreview: clean (Phase 2 fixed two type-fidelity findings it raised; Phase 0 zero findings).
Hosted CI on this PR is the land gate (OPENCLAW_TESTBOX=1 scripts/pr flow).
Named tradeoff: legacy debug-proxy sidecars stored at custom
OPENCLAW_DEBUG_PROXY_DB_PATH/_BLOB_DIRlocations (both @deprecated, undocumented) are not auto-imported by the one-time sidecar migration; the default-path migration is unchanged. Diagnostic capture cache, per AGENTS.md cache-migration policy.Autoreview: 5 rounds; fixed dangling help-test fixtures, non-positive MCP timeout migration guard, Hermes importer canonicalization; final round clean (0 findings, patch-correct 0.9).