Constant declared between import statements

REDACTED_API_KEY_SENTINEL is placed between two import blocks (after the coerceSecretRef import on line 3, before the createSubsystemLogger import on line 12). While ES modules hoist imports and this doesn't cause a runtime error, it is non-standard and will likely trigger import-order lint rules. The constant should be placed after all top-level imports.

Then declare the sentinel after all imports:

/**
 * Sentinel value written to models.json instead of a real API key when the
 * auth source is an exec/keychain SecretRef that cannot be resolved
 * synchronously.  The runtime async path (resolveApiKeyForProvider) handles
 * actual credential resolution at request time.  See #34335.
 */
export const REDACTED_API_KEY_SENTINEL = "__redacted__";

This is a comment left during a code review.
Path: src/agents/models-config.providers.ts
Line: 3-12

Comment:
**Constant declared between import statements**

`REDACTED_API_KEY_SENTINEL` is placed between two `import` blocks (after the `coerceSecretRef` import on line 3, before the `createSubsystemLogger` import on line 12). While ES modules hoist imports and this doesn't cause a runtime error, it is non-standard and will likely trigger import-order lint rules. The constant should be placed after all top-level imports.

```suggestion
import { coerceSecretRef } from "../config/types.secrets.js";
import { createSubsystemLogger } from "../logging/subsystem.js";
```
Then declare the sentinel after all imports:
```ts
/**
 * Sentinel value written to models.json instead of a real API key when the
 * auth source is an exec/keychain SecretRef that cannot be resolved
 * synchronously.  The runtime async path (resolveApiKeyForProvider) handles
 * actual credential resolution at request time.  See #34335.
 */
export const REDACTED_API_KEY_SENTINEL = "__redacted__";
```

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

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Sentinel can be propagated forward when user switches away from SecretRef

The guard added here prevents preserving a stale plain-text key when the new entry uses the sentinel. However, it does not prevent an old sentinel value in existing.apiKey (from a previous models.json write) from overwriting a real key in newEntry.apiKey.

Failing scenario:

1. User configures an exec SecretRef → models.json is written with apiKey set to the sentinel.
2. User switches to a plain API key.
3. Next refresh: newEntry.apiKey is the real key, existing.apiKey is still the old sentinel.
4. The current guard passes (new entry is not the sentinel), so preserved.apiKey is set to the old sentinel.
5. { ...newEntry, ...preserved } results in the sentinel silently overwriting the real key.

The fix should also check existing.apiKey !== REDACTED_API_KEY_SENTINEL so that a stale sentinel is never carried forward.

This is a comment left during a code review.
Path: src/agents/models-config.ts
Line: 166-172

Comment:
**Sentinel can be propagated forward when user switches away from SecretRef**

The guard added here prevents preserving a stale plain-text key when the *new* entry uses the sentinel. However, it does not prevent an *old* sentinel value in `existing.apiKey` (from a previous `models.json` write) from overwriting a real key in `newEntry.apiKey`.

Failing scenario:
1. User configures an exec SecretRef → `models.json` is written with `apiKey` set to the sentinel.
2. User switches to a plain API key.
3. Next refresh: `newEntry.apiKey` is the real key, `existing.apiKey` is still the old sentinel.
4. The current guard passes (new entry is not the sentinel), so `preserved.apiKey` is set to the old sentinel.
5. `{ ...newEntry, ...preserved }` results in the sentinel silently overwriting the real key.

The fix should also check `existing.apiKey !== REDACTED_API_KEY_SENTINEL` so that a stale sentinel is never carried forward.

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

Skip preserving old sentinel when key source changes

When a provider previously wrote "__redacted__" (exec/file SecretRef) and later switches to a resolvable key source, this merge path still preserves existing.apiKey because it only checks newEntry.apiKey !== REDACTED_API_KEY_SENTINEL. Since mergedProviders[key] = { ...newEntry, ...preserved }, the stale sentinel overwrites the new key and models.json cannot recover from the placeholder in merge mode, leaving downstream models.json consumers with an unusable credential.

Useful? React with 👍 / 👎.

Derive sentinel before preserving existing apiKey

In merge mode this guard still preserves leaked plaintext keys for explicit providers that rely on exec/file SecretRefs, because resolveProvidersForMode() runs before normalizeProviders() in ensureOpenClawModelsJson, so newEntry.apiKey is still undefined here (not the sentinel) when the merge happens. In that common path, an old plaintext existing.apiKey from models.json is copied back and never replaced, so migrating a provider from inline key to SecretRef can continue to leave the real key on disk.

Useful? React with 👍 / 👎.

Thanks for the analysis. I looked at this carefully — the concern is a false positive.

The flow is:

1. resolveProvidersForModelsJson() calls resolveApiKeyFromProfiles() which returns REDACTED_API_KEY_SENTINEL for exec/file SecretRefs → newEntry.apiKey IS the sentinel at merge time
2. resolveProvidersForMode() → calls mergeWithExistingProviderSecrets() with sentinel already set
3. normalizeProviders() runs after merge

The Codex analysis seems to confuse normalizeProviders (which normalizes model definitions) with resolveProvidersForModelsJson (which resolves API keys). The sentinel is set in step 1 before merge, not in step 3 after.

For explicit providers with apiKey as a SecretRef object in config: hasConfiguredApiKey is true so it skips the profiles path entirely — the object flows through and is handled by normalizeOptionalSecretInput in step 3.