Skip to content

feat(config): add allowFrom fallback transition foundation#81259

Closed
mcaxtr wants to merge 5 commits into
mainfrom
refactor/group-allowfrom-fallback-transition
Closed

feat(config): add allowFrom fallback transition foundation#81259
mcaxtr wants to merge 5 commits into
mainfrom
refactor/group-allowfrom-fallback-transition

Conversation

@mcaxtr

@mcaxtr mcaxtr commented May 13, 2026

Copy link
Copy Markdown
Member

Purpose

This PR adds the generic, dormant machinery needed to disable legacy allowFrom fallback per channel later. It does not disable fallback for any channel in this PR.

User Config Impact

This PR does not add user-facing fallback flags.

Users should never set *FallbackToAllowFrom in their OpenClaw config. Those booleans are channel runtime/package metadata for future channel-owned PRs.

The only user config change openclaw doctor --fix may make is copying existing allowlist values from allowFrom into the explicit allowlist that already represents the same access:

  • channels.<id>.groupAllowFrom
  • channels.<id>.commandGroupAllowFrom
  • channels.<id>.groupOwnerAllowFrom
  • commands.allowFrom.<channel-id>
  • tools.elevated.allowFrom.<channel-id>

The goal is preservation: when a later channel PR disables a fallback in code, users keep the same effective access after doctor copies the old allowFrom values to the correct explicit target.

Future Channel PR Checklist

For each fallback a channel later disables, that channel PR must do both:

  1. Change runtime behavior so the channel reads the explicit replacement allowlist.
  2. Set the matching package doctor metadata boolean to false.

Doctor infers the copy target from the boolean:

Fallback being removed later Set metadata to false Doctor copies allowFrom to
Group senders use DM allowFrom groupAllowFromFallbackToAllowFrom channels.<id>.groupAllowFrom
Group command senders use DM/group fallback commandGroupAllowFromFallbackToAllowFrom channels.<id>.commandGroupAllowFrom
Group command owners use DM allowFrom groupOwnerAllowFromFallbackToAllowFrom channels.<id>.groupOwnerAllowFrom
Text commands use channel allowFrom commandAllowFromFallbackToAllowFrom commands.allowFrom.<channel-id>
Elevated access uses channel allowFrom elevatedAllowFromFallbackToAllowFrom tools.elevated.allowFrom.<channel-id>

Only set a fallback boolean to false after the target config key is accepted by that channel schema and actually read by that channel runtime. Multi-account channels must preserve account scope; otherwise doctor warns instead of copying into a broader provider target.

Example: future Telegram opt-out

A later Telegram PR that disables group-sender fallback would set the same capability in two places:

// Telegram runtime
createChannelIngressResolver({
  channelId: "telegram",
  accountId,
  identity: telegramIngressIdentity,
  cfg,
  groupAllowFromFallbackToAllowFrom: false,
});
// Package metadata
{
  "openclaw": {
    "channel": {
      "id": "telegram",
      "doctorCapabilities": {
        "groupAllowFromFallbackToAllowFrom": false
      }
    }
  }
}

The runtime flag changes live authorization behavior. The package metadata lets openclaw doctor --fix preserve existing configs without loading Telegram runtime.

When that future PR lands, doctor can copy:

{
  "channels": {
    "telegram": {
      "allowFrom": ["123456789"]
    }
  }
}

to:

{
  "channels": {
    "telegram": {
      "allowFrom": ["123456789"],
      "groupAllowFrom": ["123456789"]
    }
  }
}

This PR only adds that generic machinery; it does not set the Telegram runtime flag or package metadata.

Summary

  • Adds fallback capability metadata to plugin manifests/read-only discovery.
  • Wires shared runtime paths to honor fallback-disable metadata when channels opt out later.
  • Adds openclaw doctor --fix preservation-copy repair.
  • Adds capability-aware doctor warnings for configs still relying on fallback.
  • Removes separate migration-target metadata; copy targets are inferred from the disabled fallback.

Verification

  • pnpm test src/config/allowfrom-fallback-transition.test.ts src/config/materialize.test.ts src/commands/doctor/channel-capabilities.test.ts src/commands/doctor/shared/empty-allowlist-policy.test.ts src/commands/doctor/repair-sequencing.test.ts src/channels/plugins/read-only.test.ts -- --reporter=verbose
  • pnpm test src/commands/doctor/repair-sequencing.test.ts src/commands/doctor/shared/empty-allowlist-policy.test.ts src/commands/doctor/shared/preview-warnings.test.ts src/config/allowfrom-fallback-transition.test.ts src/config/materialize.test.ts -- --reporter=verbose
  • pnpm test src/config/allowfrom-fallback-transition.test.ts src/config/materialize.test.ts src/commands/doctor/repair-sequencing.test.ts src/commands/doctor/shared/empty-allowlist-policy.test.ts src/commands/doctor/shared/empty-allowlist-scan.test.ts src/commands/doctor/shared/open-policy-allowfrom.test.ts src/commands/doctor/shared/preview-warnings.test.ts src/commands/doctor/channel-capabilities.test.ts src/commands/doctor-config-flow.test.ts src/auto-reply/command-control.test.ts src/auto-reply/reply/reply-elevated.test.ts src/plugin-sdk/channel-ingress-runtime.test.ts src/channels/plugins/read-only.test.ts src/security/dm-policy-shared.test.ts -- --reporter=verbose
  • pnpm docs:list
  • pnpm check:changed before rebase
  • git diff --check origin/main...HEAD

Known after rebase: pnpm check:changed currently fails in untouched src/agents/pi-embedded-runner/run/helpers.test.ts with an upstream-main UsageAccumulator type error. The file is identical to origin/main.

@mcaxtr mcaxtr requested a review from a team as a code owner May 13, 2026 02:58
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation commands Command implementations size: XL maintainer Maintainer-authored PR labels May 13, 2026
@clawsweeper

clawsweeper Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
The PR adds dormant allowFrom fallback capability metadata, runtime opt-out plumbing, doctor preservation-copy repair/warnings, documentation, and targeted tests for future channel-owned fallback removals.

Reproducibility: not applicable. this is transition-foundation feature work rather than a bug report. The intended behavior is source-checkable in the PR head and covered by targeted tests listed in the PR body, but I did not run tests in this read-only review.

Real behavior proof
Not applicable: Contributor real-behavior proof is not required for this MEMBER-authored PR with the protected maintainer label.

Next step before merge
Protected maintainer-labeled MEMBER PR; the remaining action is maintainer review and CI, not an automated repair job.

Security
Cleared: The diff changes authorization/config and doctor repair logic but introduces no dependency, workflow, lockfile, secret-handling, install, download, or package-execution change.

Review details

Best possible solution:

Review and land the dormant metadata-driven transition seam once CI is green, leaving actual per-channel fallback removals to later channel-owned PRs.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is transition-foundation feature work rather than a bug report. The intended behavior is source-checkable in the PR head and covered by targeted tests listed in the PR body, but I did not run tests in this read-only review.

Is this the best way to solve the issue?

Yes as a direction: a dormant metadata-driven seam plus doctor preservation copying is the narrow maintainable path for later channel-owned fallback removals. Because it changes authorization/config plumbing, maintainer review and CI remain the correct merge gate.

What I checked:

  • Protected PR context: Provided live PR context shows the PR is open, authored by mcaxtr with MEMBER association, labeled maintainer, and at head c314391; repository policy keeps protected maintainer items open for explicit maintainer handling. (c31439128bb4)
  • Current main is not already implementing the PR: Current main has existing command-group fallback plumbing, but no applyAllowFromFallbackTransition, classifyAllowFromFallbackTransitions, commandAllowFromFallbackToAllowFrom, or elevatedAllowFromFallbackToAllowFrom hits, so the PR is not obsolete on main. (7715b29aa231)
  • Doctor repair sequencing: At the PR head, doctor repair runs applyAllowFromFallbackTransition after allowlist-policy repair and before open-policy allowFrom repair, using channel doctor capability metadata from the current config/env. (src/commands/doctor/repair-sequencing.ts:111, c31439128bb4)
  • Preservation-copy implementation: The new transition helper classifies disabled fallback families and writes inferred explicit targets such as commands.allowFrom, tools.elevated.allowFrom, or channel-local allowlists only when migration is eligible. (src/config/allowfrom-fallback-transition.ts:757, c31439128bb4)
  • Runtime fallback gates: The PR head makes command authorization treat missing commands.allowFrom as an empty list when command fallback metadata is disabled, while retaining the old fallback when metadata is absent or true. (src/auto-reply/command-auth.ts:660, c31439128bb4)
  • Security-sensitive scope review: The provided file list and raw source reads show authorization/config/doctor/docs/test changes, with no dependency source, lockfile, workflow, package execution, install script, or downloaded artifact changes. (c31439128bb4)

Likely related people:

  • steipete: Recent current-main history moved doctor capability metadata to channel manifests and includes adjacent runtime conversion work touched by this PR’s doctor/runtime boundary. (role: recent area contributor; confidence: high; commits: 86667d670e69, 9e0d35869521; files: src/commands/doctor/channel-capabilities.ts, src/channels/message-access/runtime.ts, src/auto-reply/reply/reply-elevated.ts)
  • vincentkoc: Prior current-main commits centralized doctor channel capability metadata, split allowFrom mode types, and worked on type surfaces directly adjacent to the manifest/doctor metadata contract extended here. (role: doctor capability and plugin-boundary contributor; confidence: medium; commits: b9e71240ed0b, 714adcb1246b, 74e7b8d47b18; files: src/commands/doctor/channel-capabilities.ts, src/plugins/manifest.ts, src/plugins/manifest-registry.ts)

Remaining risk / open question:

  • Tests and CI were not independently rerun in this read-only review.
  • The patch touches authorization and doctor repair behavior, so maintainer review should confirm the fallback semantics before merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 7715b29aa231.

Re-review progress:

@mcaxtr mcaxtr changed the title Build safe allowFrom fallback transition foundation feat(config): add allowFrom fallback transition foundation May 13, 2026
@mcaxtr mcaxtr force-pushed the refactor/group-allowfrom-fallback-transition branch from ed54eee to 4413aa1 Compare May 15, 2026 16:17
@mcaxtr mcaxtr force-pushed the refactor/group-allowfrom-fallback-transition branch from 4413aa1 to c314391 Compare May 15, 2026 16:55
@steipete

Copy link
Copy Markdown
Contributor

Thanks for putting this together. I am going to close this PR and take over the change in a smaller shape.

The direction should be a doctor migration, not a dormant runtime/manifest fallback-transition framework. We should preserve existing effective access by having openclaw doctor --fix materialize the explicit allowlists, then keep runtime behavior and plugin metadata simpler until a concrete channel needs behavior changed.

The current PR adds too much future-facing authorization plumbing for that goal, especially the *FallbackToAllowFrom metadata/runtime flags. I will rewrite this as a narrow doctor repair with focused regression coverage instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands Command implementations docs Improvements or additions to documentation maintainer Maintainer-authored PR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants