Greptile Summary

This PR fixes a real bug where getCustomProviderApiKey would silently return undefined when apiKey in the provider config was a SecretRef object (or an ${ENV_VAR} template string) instead of a plain string, causing agents to fail with "No API key found for provider". The fix correctly uses resolveSecretInputRef to detect and unwrap env-based SecretRefs before falling back to the existing plain-string path.

Key changes:

• src/agents/model-auth.ts: getCustomProviderApiKey now detects SecretRef inputs via resolveSecretInputRef, resolves env sources from process.env, and returns undefined for unresolvable file/exec sources.

Issues found:

• coerceSecretRef is imported but never referenced in the modified file — the unused import should be removed.
• The ref.provider field is ignored when reading process.env[ref.id]; users who configure a non-default env provider with an allowlist will have that allowlist silently bypassed. Scoping the direct process.env read to DEFAULT_SECRET_PROVIDER_ALIAS only would make the behaviour more predictable.

Confidence Score: 4/5

• Safe to merge with a trivial fix for the unused import; the core logic is correct for the common case.
• The change is small and well-scoped, and the bug it addresses is clearly real. The logic for detecting and resolving env SecretRefs is sound. The two concerns (unused import and provider allowlist bypass) are low-severity — the unused import is a lint issue and the allowlist concern is an edge case that doesn't affect standard configurations. No regressions are introduced for the existing plain-string path.
• src/agents/model-auth.ts — remove the unused coerceSecretRef import.

This is a comment left during a code review.
Path: src/agents/model-auth.ts
Line: 6

Comment:
**Unused import: `coerceSecretRef`**

`coerceSecretRef` is imported here but never referenced in the file. The resolution logic already goes through `resolveSecretInputRef` (which calls `coerceSecretRef` internally), so this import is redundant.

```suggestion
import { resolveSecretInputRef } from "../config/types.secrets.js";
```

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

---

This is a comment left during a code review.
Path: src/agents/model-auth.ts
Line: 64-70

Comment:
**SecretRef provider field and allowlist not respected**

The resolution short-circuits to `process.env[ref.id]` for any `env` source, but `SecretsConfig` supports multiple named env providers (via `cfg?.secrets?.providers`), each of which can carry an `allowlist` of permitted env var names. By ignoring `ref.provider`, this code will happily read an env var that a non-default provider would restrict.

For the common `provider: "default"` case this is fine, but if a user has configured a custom env provider with an allowlist, the allowlist is silently bypassed. At a minimum, consider logging a warning or gating the direct `process.env` read to `ref.provider === DEFAULT_SECRET_PROVIDER_ALIAS` only:

```typescript
if (ref.source === "env" && ref.provider === DEFAULT_SECRET_PROVIDER_ALIAS) {
  const envValue = process.env[ref.id];
  if (envValue) {
    return normalizeOptionalSecretInput(envValue);
  }
}
```

This keeps the fix correct for the reported bug while making it clear that non-default providers fall through to the `return undefined` path (with a follow-up comment explaining the gap).

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

_{Last reviewed commit: "fix: Handle SecretRe..."}

Thanks for the contribution here, @xi7ang.

Outcome: this PR is superseded by #66818, which landed the fix as part of the broader SecretRef lifecycle alignment.

Why #66818 covers this: it implements custom provider SecretRef handling and extends it with provider-policy checks (env source + allowlist) so read-only resolution stays safe and consistent.