Greptile Summary

This PR fixes a crash in getReplyFromConfig() where channel monitors (Telegram, Slack, Discord) capture a config snapshot at startup via loadConfig() before the secrets runtime snapshot is activated. The captured config contains raw SecretRef objects that then propagate through the dispatch chain as configOverride, bypassing the snapshot-aware loadConfig() and ultimately crashing runEmbeddedPiAgent() with an "unresolved SecretRef" error.

The fix introduces a hasUnresolvedChannelSecrets() guard that inspects the configOverride for unresolved SecretRef objects in channel token fields. When detected, the stale override is discarded and loadConfig() is used instead, which returns the correctly resolved runtime snapshot.

Key changes:

• New hasUnresolvedChannelSecrets() function that checks Telegram botToken (top-level and per-account), and Slack/Discord botToken, appToken, token, and userToken fields.
• getReplyFromConfig() now conditionally discards a stale configOverride and logs a diagnostic error when fallback is triggered.
• userToken (previously flagged in review) is now included in the Slack/Discord token-key scan.

Confidence Score: 5/5

• This PR is safe to merge — it eliminates a real crash with no possible regression, since discarding a config that would crash later is strictly better than keeping it.
• The fix is narrowly scoped and provably safe: any configOverride that contains an unresolved SecretRef in a channel token field would crash runEmbeddedPiAgent() without this guard, so falling back to loadConfig() is always an improvement. The coerceSecretRef utility is battle-tested and correctly detects all three unresolved-secret forms (plain SecretRef object, legacy form without provider, and ${ENV_VAR} template strings). The userToken concern from the prior review thread has been addressed. pnpm lint and pnpm check are clean. No regressions are possible for callers who supply a valid, fully-resolved configOverride.
• No files require special attention.

_{Reviews (4): Last reviewed commit: "fix(secrets): prevent unresolved SecretR..." | Re-trigger Greptile}

@codex review
@greptile review

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

@greptile review

@greptile review

Reworked this to fix the contract, not patch stale configs in getReplyFromConfig().

The real issue was that dispatchReplyFromConfig() always forwarded cfg as if it were a real configOverride, so long-lived channel monitors could leak their startup snapshot into the reply path. This rewrite stops passing cfg implicitly, makes getReplyFromConfig() start from fresh loadConfig(), and only merges an explicit configOverride when a caller actually has a per-message patch to apply.

For this PR, MSTeams now passes only its sender-timezone patch as configOverride, and the normal path no longer needs channel-specific stale-secret detection.

Landed on main.

• Landed source commit: 753531b
• Merge commit: 0a04ef4

Thanks @Lukavyi.

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

​// dispatch-from-config.ts
const replyResult = await replyResolver(
  ctx,
  { /* opts */ },
  params.configOverride, // now undefined when no override
);

​// get-reply.ts
const cfg =
  configOverride == null
    ? loadConfig() // reloads instead of using the already-resolved cfg
    : applyMergePatch(loadConfig(), configOverride);

await replyResolver(ctx, opts, params.configOverride ?? cfg);

Analyzed PR: #53577 at commit 753531b

_{Last updated on: 2026-03-24T16:14:54Z}