Greptile Summary

This PR adds a resolveMonitorTelegramToken helper that passes bot tokens through resolveConfiguredSecretInputString before the Telegram provider starts, fixing startup failures when TELEGRAM_BOT_TOKEN is a SecretRef-style value (e.g. op://...). The env-token path and accompanying regression test work correctly for the primary use case.

Issues found:

• Explicit SecretRef fallback logic is broken (extensions/telegram/src/monitor.ts lines 96–101): when opts.token is a SecretRef that cannot be resolved, the guard explicitToken === params.resolvedAccountToken is almost always false (the explicit token and the account-resolved token come from different sources), so the throw is skipped and the unresolved op://... string is returned to the caller. This passes the raw SecretRef to Telegram API calls — the exact failure mode the PR is intended to prevent — in the explicit-token code path.

• Env stub leaks across tests (extensions/telegram/src/monitor.test.ts): vi.stubEnv(\"TELEGRAM_BOT_TOKEN\", ...) is called in the new test but vi.unstubAllEnvs() is never called in afterEach. This violates the repo's testing guideline requiring cleanup of env/globals/state so --isolate=false stays green.

Confidence Score: 3/5

Not safe to merge without addressing the explicit SecretRef fallback — the primary new code path silently passes unresolved SecretRef strings to Telegram in a common scenario.

A P1 logic defect exists in the explicit-token branch of resolveMonitorTelegramToken: the equality check used to decide whether to throw (explicitToken === params.resolvedAccountToken) is almost never true when an explicit SecretRef is passed via opts.token, so the function silently returns the unresolved op://... string. This directly undermines the PR's stated goal. Additionally there is a P2 env-cleanup gap in the test.

extensions/telegram/src/monitor.ts (explicit-token SecretRef fallback logic, lines 96–101) and extensions/telegram/src/monitor.test.ts (missing vi.unstubAllEnvs() in afterEach).

This is a comment left during a code review.
Path: extensions/telegram/src/monitor.ts
Line: 96-101

Comment:
**Unresolved explicit SecretRef silently passed to Telegram**

When `opts.token` is a SecretRef string (e.g. `op://vault/telegram`) and `resolveConfiguredSecretInputString` cannot resolve it, the code reaches line 101 only if `explicitToken !== params.resolvedAccountToken`. In practice this condition is almost always false — the explicit token passed via `opts.token` and `account.token` typically come from different sources — so the `throw` on line 97 is never reached for a failed explicit SecretRef. The result is that `return explicitToken` silently returns the unresolved `op://...` string as the bot token, causing the same `404 Not Found` / `deleteWebhook failed` failures this PR claims to prevent.

Consider throwing unconditionally when `resolved.value` is falsy for the explicit-token path, instead of relying on the `explicitToken === resolvedAccountToken` equality check:

```suggestion
    if (resolved.value) {
      return resolved.value;
    }
    throw new Error(
      `Telegram bot token unavailable: ${resolved.unresolvedRefReason ?? "explicit token SecretRef is unresolved."}`,
    );
  }
```

If you intentionally want to allow plaintext fallback for non-SecretRef values, `resolveConfiguredSecretInputString` should return the original plaintext as `resolved.value` for non-SecretRef inputs, making the fallback unreachable for valid tokens.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/telegram/src/monitor.test.ts
Line: 424

Comment:
**`vi.stubEnv` not cleaned up — env leaks to subsequent tests**

`vi.stubEnv("TELEGRAM_BOT_TOKEN", ...)` stubs the env variable for the remainder of the test suite unless `vi.unstubAllEnvs()` is called. The current `afterEach` block only calls `consoleErrorSpy?.mockRestore()`, so `TELEGRAM_BOT_TOKEN` stays set to `"op://secrets/telegram/credential"` for every test that runs after this one.

The repo testing guideline explicitly requires: _"Write tests to clean up timers, env, globals, mocks, sockets, temp dirs, and module state so `--isolate=false` stays green._"

Add `vi.unstubAllEnvs()` to the existing `afterEach`:

```ts
afterEach(() => {
  consoleErrorSpy?.mockRestore();
  vi.unstubAllEnvs();
});
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(telegram): resolve env SecretRef bot..." | Re-trigger Greptile}

This PR was green on March 29, but main has moved since then and the branch now needs an update/rebase before merge.

Thanks for this Telegram SecretRef fix proposal, @davetanaka.

Outcome: this PR is superseded by #66818, which landed a broader version of the same runtime fix.

Why #66818 covers this: Telegram token resolution now supports env-backed SecretRefs at runtime, preserves strict behavior for unresolved non-env refs, and enforces env provider policy checks.