feat(ai): added credential disable tracking with auditability improvements
- Added `disabledCause` parameter to credential deletion methods to track reason credentials are disabled. - Changed credential disabling mechanism from boolean `disabled` flag to `disabled_cause` text field for better auditability. - Fixed credential purging to respect disabled credentials during email deduplication operations. - Refactored `replaceAuthCredentialsForProvider()` to update matching credentials instead of deleting all, preserving credential history.
This commit is contained in:
can1357
@@ -1,7 +1,6 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
- Changed `reasoning` parameter from `ThinkingLevel | undefined` to `Effort | undefined` in `SimpleStreamOptions`; 'off' is no longer valid (omit the field instead)
|
||||
@@ -32,6 +31,8 @@
|
||||
|
||||
### Changed
|
||||
|
||||
- Changed credential disabling mechanism from boolean `disabled` flag to `disabled_cause` text field for tracking why credentials were disabled
|
||||
- Changed `deleteAuthCredential()` and `deleteAuthCredentialsForProvider()` methods to require a `disabledCause` parameter explaining the reason for disabling
|
||||
- Changed Gemini model parsing to strip `-preview` suffix for consistent model identification
|
||||
- Changed OpenAI Codex websocket error handling to detect fatal connection errors and immediately fall back to SSE without retrying
|
||||
- Changed OpenAI Codex to always use websockets v2 protocol (removed v1 support)
|
||||
@@ -65,6 +66,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed credential purging to respect disabled credentials when deduplicating by email, preventing re-enablement of intentionally disabled credentials
|
||||
- Fixed OpenAI Codex websocket error reporting to include detailed error messages from error events
|
||||
- Fixed conversation history reconstruction to support incremental updates from multiple assistant messages while maintaining backward compatibility with full-snapshot payloads
|
||||
- Fixed OpenAI Codex to reject unsupported effort levels instead of silently clamping them, providing clear error messages about supported efforts
|
||||
|
||||
@@ -108,6 +108,7 @@ export interface StoredAuthCredential {
|
||||
id: number;
|
||||
provider: string;
|
||||
credential: AuthCredential;
|
||||
disabledCause: string | null;
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
@@ -484,7 +485,7 @@ export class AuthStorage {
|
||||
}
|
||||
if (removed.length > 0) {
|
||||
for (const entry of removed) {
|
||||
this.#store.deleteAuthCredential(entry.id);
|
||||
this.#store.deleteAuthCredential(entry.id, "deduplicated duplicate credential");
|
||||
}
|
||||
this.#resetProviderAssignments(provider);
|
||||
}
|
||||
@@ -659,10 +660,10 @@ export class AuthStorage {
|
||||
* The credential remains in the database but is excluded from active queries.
|
||||
* Cleans up provider entry if last credential disabled.
|
||||
*/
|
||||
#removeCredentialAt(provider: string, index: number): void {
|
||||
#disableCredentialAt(provider: string, index: number, disabledCause: string): void {
|
||||
const entries = this.#getStoredCredentials(provider);
|
||||
if (index < 0 || index >= entries.length) return;
|
||||
this.#store.deleteAuthCredential(entries[index].id);
|
||||
this.#store.deleteAuthCredential(entries[index].id, disabledCause);
|
||||
const updated = entries.filter((_value, idx) => idx !== index);
|
||||
this.#setStoredCredentials(provider, updated);
|
||||
this.#resetProviderAssignments(provider);
|
||||
@@ -693,7 +694,7 @@ export class AuthStorage {
|
||||
* Remove credential for a provider.
|
||||
*/
|
||||
async remove(provider: string): Promise<void> {
|
||||
this.#store.deleteAuthCredentialsForProvider(provider);
|
||||
this.#store.deleteAuthCredentialsForProvider(provider, "deleted by user");
|
||||
this.#data.delete(provider);
|
||||
this.#resetProviderAssignments(provider);
|
||||
}
|
||||
@@ -1858,8 +1859,8 @@ export class AuthStorage {
|
||||
});
|
||||
|
||||
if (isDefinitiveFailure) {
|
||||
// Permanently remove invalid credentials
|
||||
this.#removeCredentialAt(provider, selection.index);
|
||||
// Permanently disable invalid credentials with an explicit cause for inspection/debugging
|
||||
this.#disableCredentialAt(provider, selection.index, `oauth refresh failed: ${errorMsg}`);
|
||||
if (this.#getCredentialsForProvider(provider).some(credential => credential.type === "oauth")) {
|
||||
return this.getApiKey(provider, sessionId, options);
|
||||
}
|
||||
@@ -1950,6 +1951,7 @@ type AuthRow = {
|
||||
provider: string;
|
||||
credential_type: string;
|
||||
data: string;
|
||||
disabled_cause: string | null;
|
||||
};
|
||||
|
||||
function serializeCredential(
|
||||
@@ -1993,6 +1995,26 @@ function deserializeCredential(row: AuthRow): AuthCredential | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
function normalizeDisabledCause(disabledCause: string): string {
|
||||
const normalized = disabledCause.trim();
|
||||
return normalized.length > 0 ? normalized : "disabled";
|
||||
}
|
||||
|
||||
function toStoredAuthCredential(row: AuthRow, credential: AuthCredential): StoredAuthCredential {
|
||||
return { id: row.id, provider: row.provider, credential, disabledCause: row.disabled_cause };
|
||||
}
|
||||
|
||||
/** Returns a stable identity string for matching credentials across replace operations. */
|
||||
function credentialIdentity(credential: AuthCredential): string | null {
|
||||
if (credential.type === "api_key") return `api_key:${credential.key}`;
|
||||
if (credential.type === "oauth") {
|
||||
if (credential.accountId) return `account:${credential.accountId}`;
|
||||
const [email] = extractCredentialEmails(credential);
|
||||
if (email) return `email:${email}`;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Extracts normalized email identifiers from a credential, including JWT profile claims. */
|
||||
function extractCredentialEmails(credential: AuthCredential): string[] {
|
||||
if (credential.type !== "oauth") return [];
|
||||
@@ -2053,13 +2075,13 @@ export class AuthCredentialStore {
|
||||
this.#initializeSchema();
|
||||
|
||||
this.#listActiveStmt = this.#db.prepare(
|
||||
"SELECT id, provider, credential_type, data FROM auth_credentials WHERE disabled = 0 ORDER BY id ASC",
|
||||
"SELECT id, provider, credential_type, data, disabled_cause FROM auth_credentials WHERE disabled_cause IS NULL ORDER BY id ASC",
|
||||
);
|
||||
this.#listActiveByProviderStmt = this.#db.prepare(
|
||||
"SELECT id, provider, credential_type, data FROM auth_credentials WHERE provider = ? AND disabled = 0 ORDER BY id ASC",
|
||||
"SELECT id, provider, credential_type, data, disabled_cause FROM auth_credentials WHERE provider = ? AND disabled_cause IS NULL ORDER BY id ASC",
|
||||
);
|
||||
this.#listDisabledByProviderStmt = this.#db.prepare(
|
||||
"SELECT id, credential_type, data FROM auth_credentials WHERE provider = ? AND disabled = 1 ORDER BY id ASC",
|
||||
"SELECT id, provider, credential_type, data, disabled_cause FROM auth_credentials WHERE provider = ? AND disabled_cause IS NOT NULL ORDER BY id ASC",
|
||||
);
|
||||
this.#insertStmt = this.#db.prepare(
|
||||
"INSERT INTO auth_credentials (provider, credential_type, data) VALUES (?, ?, ?) RETURNING id",
|
||||
@@ -2068,10 +2090,10 @@ export class AuthCredentialStore {
|
||||
"UPDATE auth_credentials SET credential_type = ?, data = ?, updated_at = unixepoch() WHERE id = ?",
|
||||
);
|
||||
this.#deleteStmt = this.#db.prepare(
|
||||
"UPDATE auth_credentials SET disabled = 1, updated_at = unixepoch() WHERE id = ?",
|
||||
"UPDATE auth_credentials SET disabled_cause = ?, updated_at = unixepoch() WHERE id = ?",
|
||||
);
|
||||
this.#deleteByProviderStmt = this.#db.prepare(
|
||||
"UPDATE auth_credentials SET disabled = 1, updated_at = unixepoch() WHERE provider = ?",
|
||||
"UPDATE auth_credentials SET disabled_cause = ?, updated_at = unixepoch() WHERE provider = ? AND disabled_cause IS NULL",
|
||||
);
|
||||
this.#hardDeleteStmt = this.#db.prepare("DELETE FROM auth_credentials WHERE id = ?");
|
||||
this.#getCacheStmt = this.#db.prepare("SELECT value FROM cache WHERE key = ? AND expires_at > unixepoch()");
|
||||
@@ -2112,7 +2134,7 @@ CREATE TABLE IF NOT EXISTS auth_credentials (
|
||||
provider TEXT NOT NULL,
|
||||
credential_type TEXT NOT NULL,
|
||||
data TEXT NOT NULL,
|
||||
disabled INTEGER NOT NULL DEFAULT 0,
|
||||
disabled_cause TEXT DEFAULT NULL,
|
||||
created_at INTEGER NOT NULL DEFAULT (unixepoch()),
|
||||
updated_at INTEGER NOT NULL DEFAULT (unixepoch())
|
||||
);
|
||||
@@ -2126,10 +2148,18 @@ CREATE TABLE IF NOT EXISTS cache (
|
||||
CREATE INDEX IF NOT EXISTS idx_cache_expires ON cache(expires_at);
|
||||
`);
|
||||
|
||||
// Migration: add disabled column if missing (for databases created by old CliAuthStorage)
|
||||
const cols = this.#db.prepare("PRAGMA table_info(auth_credentials)").all() as Array<{ name?: string }>;
|
||||
if (!cols.some(c => c.name === "disabled")) {
|
||||
this.#db.exec("ALTER TABLE auth_credentials ADD COLUMN disabled INTEGER NOT NULL DEFAULT 0");
|
||||
const hasDisabledCause = cols.some(c => c.name === "disabled_cause");
|
||||
const hasDisabled = cols.some(c => c.name === "disabled");
|
||||
if (!hasDisabledCause) {
|
||||
this.#db.exec("ALTER TABLE auth_credentials ADD COLUMN disabled_cause TEXT DEFAULT NULL");
|
||||
}
|
||||
if (hasDisabled) {
|
||||
this.#db.exec(`
|
||||
UPDATE auth_credentials
|
||||
SET disabled_cause = COALESCE(disabled_cause, 'disabled')
|
||||
WHERE disabled = 1 AND disabled_cause IS NULL
|
||||
`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2145,26 +2175,52 @@ CREATE INDEX IF NOT EXISTS idx_cache_expires ON cache(expires_at);
|
||||
for (const row of rows) {
|
||||
const credential = deserializeCredential(row);
|
||||
if (!credential) continue;
|
||||
results.push({ id: row.id, provider: row.provider, credential });
|
||||
results.push(toStoredAuthCredential(row, credential));
|
||||
}
|
||||
return results;
|
||||
}
|
||||
|
||||
replaceAuthCredentialsForProvider(provider: string, credentials: AuthCredential[]): StoredAuthCredential[] {
|
||||
const replace = this.#db.transaction((providerName: string, items: AuthCredential[]) => {
|
||||
this.#deleteByProviderStmt.run(providerName);
|
||||
const inserted: StoredAuthCredential[] = [];
|
||||
const existingRows = this.#listActiveByProviderStmt.all(providerName) as AuthRow[];
|
||||
const existing: Array<{ id: number; credential: AuthCredential; identity: string | null }> = [];
|
||||
for (const row of existingRows) {
|
||||
const credential = deserializeCredential(row);
|
||||
if (!credential) continue;
|
||||
existing.push({ id: row.id, credential, identity: credentialIdentity(credential) });
|
||||
}
|
||||
|
||||
const result: StoredAuthCredential[] = [];
|
||||
const matchedExistingIds = new Set<number>();
|
||||
|
||||
for (const credential of items) {
|
||||
const serialized = serializeCredential(credential);
|
||||
if (!serialized) continue;
|
||||
const row = this.#insertStmt.get(providerName, serialized.credentialType, serialized.data) as
|
||||
| { id?: number }
|
||||
| undefined;
|
||||
if (row?.id) {
|
||||
inserted.push({ id: row.id, provider: providerName, credential });
|
||||
const identity = credentialIdentity(credential);
|
||||
const match = identity
|
||||
? existing.find(e => e.identity === identity && !matchedExistingIds.has(e.id))
|
||||
: null;
|
||||
if (match) {
|
||||
matchedExistingIds.add(match.id);
|
||||
this.#updateStmt.run(serialized.credentialType, serialized.data, match.id);
|
||||
result.push({ id: match.id, provider: providerName, credential, disabledCause: null });
|
||||
} else {
|
||||
const row = this.#insertStmt.get(providerName, serialized.credentialType, serialized.data) as
|
||||
| { id?: number }
|
||||
| undefined;
|
||||
if (row?.id) {
|
||||
result.push({ id: row.id, provider: providerName, credential, disabledCause: null });
|
||||
}
|
||||
}
|
||||
}
|
||||
return inserted;
|
||||
|
||||
for (const row of existing) {
|
||||
if (!matchedExistingIds.has(row.id)) {
|
||||
this.#deleteStmt.run("replaced by newer credential", row.id);
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
});
|
||||
|
||||
const result = replace(provider, credentials);
|
||||
@@ -2187,13 +2243,9 @@ CREATE INDEX IF NOT EXISTS idx_cache_expires ON cache(expires_at);
|
||||
}
|
||||
if (activeEmails.size === 0) return;
|
||||
|
||||
const disabledRows = this.#listDisabledByProviderStmt.all(provider) as Array<{
|
||||
id: number;
|
||||
credential_type: string;
|
||||
data: string;
|
||||
}>;
|
||||
const disabledRows = this.#listDisabledByProviderStmt.all(provider) as AuthRow[];
|
||||
for (const row of disabledRows) {
|
||||
const credential = deserializeCredential({ ...row, provider });
|
||||
const credential = deserializeCredential(row);
|
||||
if (!credential) {
|
||||
this.#hardDeleteStmt.run(row.id);
|
||||
continue;
|
||||
@@ -2224,17 +2276,17 @@ CREATE INDEX IF NOT EXISTS idx_cache_expires ON cache(expires_at);
|
||||
}
|
||||
}
|
||||
|
||||
deleteAuthCredential(id: number): void {
|
||||
deleteAuthCredential(id: number, disabledCause: string): void {
|
||||
try {
|
||||
this.#deleteStmt.run(id);
|
||||
this.#deleteStmt.run(normalizeDisabledCause(disabledCause), id);
|
||||
} catch {
|
||||
// Ignore delete failures
|
||||
}
|
||||
}
|
||||
|
||||
deleteAuthCredentialsForProvider(provider: string): void {
|
||||
deleteAuthCredentialsForProvider(provider: string, disabledCause: string): void {
|
||||
try {
|
||||
this.#deleteByProviderStmt.run(provider);
|
||||
this.#deleteByProviderStmt.run(normalizeDisabledCause(disabledCause), provider);
|
||||
} catch {
|
||||
// Ignore delete failures
|
||||
}
|
||||
@@ -2328,7 +2380,7 @@ CREATE INDEX IF NOT EXISTS idx_cache_expires ON cache(expires_at);
|
||||
* Delete all credentials for a provider.
|
||||
*/
|
||||
deleteProvider(provider: string): void {
|
||||
this.deleteAuthCredentialsForProvider(provider);
|
||||
this.deleteAuthCredentialsForProvider(provider, "deleted by user");
|
||||
}
|
||||
|
||||
close(): void {
|
||||
|
||||
@@ -48,6 +48,20 @@ function countCredentialRows(dbPath: string, provider: string): number {
|
||||
}
|
||||
}
|
||||
|
||||
function readDisabledCauses(dbPath: string, provider: string): string[] {
|
||||
const db = new Database(dbPath, { readonly: true });
|
||||
try {
|
||||
const rows = db
|
||||
.prepare(
|
||||
"SELECT disabled_cause FROM auth_credentials WHERE provider = ? AND disabled_cause IS NOT NULL ORDER BY id ASC",
|
||||
)
|
||||
.all(provider) as Array<{ disabled_cause?: string | null }>;
|
||||
return rows.flatMap(row => (typeof row.disabled_cause === "string" ? [row.disabled_cause] : []));
|
||||
} finally {
|
||||
db.close();
|
||||
}
|
||||
}
|
||||
|
||||
describe("AuthStorage openai-codex email dedupe", () => {
|
||||
let tempDir = "";
|
||||
let dbPath = "";
|
||||
@@ -171,4 +185,58 @@ describe("AuthStorage openai-codex email dedupe", () => {
|
||||
const credentials = store.listAuthCredentials("openai-codex");
|
||||
expect(credentials).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("stores the disable cause when a credential is soft-disabled", async () => {
|
||||
if (!store || !dbPath) throw new Error("test setup failed");
|
||||
|
||||
store.replaceAuthCredentialsForProvider("openai-codex", [
|
||||
createCredential({ suffix: "only", accountId: "account-a", email: "only@example.com" }),
|
||||
]);
|
||||
|
||||
const [credential] = store.listAuthCredentials("openai-codex");
|
||||
if (!credential) throw new Error("expected stored credential");
|
||||
|
||||
const disabledCause = "oauth refresh failed: invalid_grant";
|
||||
store.deleteAuthCredential(credential.id, disabledCause);
|
||||
|
||||
expect(store.listAuthCredentials("openai-codex")).toHaveLength(0);
|
||||
expect(readDisabledCauses(dbPath, "openai-codex")).toEqual([disabledCause]);
|
||||
});
|
||||
|
||||
it("backfills a default disabled cause when migrating legacy disabled rows", async () => {
|
||||
if (!tempDir) throw new Error("test setup failed");
|
||||
|
||||
const legacyDbPath = path.join(tempDir, "legacy-agent.db");
|
||||
const legacyDb = new Database(legacyDbPath);
|
||||
legacyDb.exec(`
|
||||
CREATE TABLE auth_credentials (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
provider TEXT NOT NULL,
|
||||
credential_type TEXT NOT NULL,
|
||||
data TEXT NOT NULL,
|
||||
disabled INTEGER NOT NULL DEFAULT 0,
|
||||
created_at INTEGER NOT NULL DEFAULT (unixepoch()),
|
||||
updated_at INTEGER NOT NULL DEFAULT (unixepoch())
|
||||
);
|
||||
`);
|
||||
legacyDb
|
||||
.prepare("INSERT INTO auth_credentials (provider, credential_type, data, disabled) VALUES (?, ?, ?, ?)")
|
||||
.run(
|
||||
"openai-codex",
|
||||
"oauth",
|
||||
JSON.stringify(
|
||||
createCredential({ suffix: "legacy", accountId: "legacy-account", email: "legacy@example.com" }),
|
||||
),
|
||||
1,
|
||||
);
|
||||
legacyDb.close();
|
||||
|
||||
const migratedStore = await AuthCredentialStore.open(legacyDbPath);
|
||||
try {
|
||||
expect(migratedStore.listAuthCredentials("openai-codex")).toHaveLength(0);
|
||||
expect(readDisabledCauses(legacyDbPath, "openai-codex")).toEqual(["disabled"]);
|
||||
} finally {
|
||||
migratedStore.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
- Changed `ThinkingLevel` type to be imported from `@oh-my-pi/pi-agent-core` instead of `@oh-my-pi/pi-ai`
|
||||
@@ -27,6 +28,8 @@
|
||||
|
||||
### Changed
|
||||
|
||||
- Changed credential deletion to disable credentials with persisted cause instead of permanent deletion
|
||||
- Added `disabledCause` parameter to credential deletion methods to track reason for disabling
|
||||
- Changed thinking level parsing to use `parseEffort()` from local thinking module instead of `parseThinkingLevel()` from pi-ai
|
||||
- Changed model list display to show supported thinking efforts (e.g., "low,medium,high") instead of yes/no reasoning indicator
|
||||
- Changed footer and status line to check `model.thinking` instead of `model.reasoning` for thinking level display
|
||||
|
||||
@@ -276,22 +276,20 @@ CREATE TABLE settings (
|
||||
* @returns Array of stored credentials with their database IDs
|
||||
*/
|
||||
listAuthCredentials(provider?: string, includeDisabled = false): StoredAuthCredential[] {
|
||||
// AuthCredentialStore doesn't expose includeDisabled yet, so we filter if needed
|
||||
const credentials = this.#authStore.listAuthCredentials(provider);
|
||||
if (!includeDisabled) return credentials;
|
||||
|
||||
// For now, includeDisabled requires direct DB access
|
||||
// This is only used internally, so it's acceptable
|
||||
const stmt = this.#db.prepare(
|
||||
provider
|
||||
? "SELECT id, provider, credential_type, data FROM auth_credentials WHERE provider = ? ORDER BY id ASC"
|
||||
: "SELECT id, provider, credential_type, data FROM auth_credentials ORDER BY id ASC",
|
||||
? "SELECT id, provider, credential_type, data, disabled_cause FROM auth_credentials WHERE provider = ? ORDER BY id ASC"
|
||||
: "SELECT id, provider, credential_type, data, disabled_cause FROM auth_credentials ORDER BY id ASC",
|
||||
);
|
||||
const rows = (provider ? stmt.all(provider) : stmt.all()) as Array<{
|
||||
id: number;
|
||||
provider: string;
|
||||
credential_type: string;
|
||||
data: string;
|
||||
disabled_cause: string | null;
|
||||
}>;
|
||||
|
||||
const results: StoredAuthCredential[] = [];
|
||||
@@ -309,7 +307,7 @@ CREATE TABLE settings (
|
||||
continue;
|
||||
}
|
||||
|
||||
results.push({ id: row.id, provider: row.provider, credential });
|
||||
results.push({ id: row.id, provider: row.provider, credential, disabledCause: row.disabled_cause });
|
||||
} catch {}
|
||||
}
|
||||
return results;
|
||||
@@ -336,19 +334,21 @@ CREATE TABLE settings (
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes an auth credential by ID.
|
||||
* @param id - Database row ID of the credential to delete
|
||||
* Disables an auth credential by ID with a persisted cause.
|
||||
* @param id - Database row ID of the credential to disable
|
||||
* @param disabledCause - Human-readable cause stored with the disabled row
|
||||
*/
|
||||
deleteAuthCredential(id: number): void {
|
||||
this.#authStore.deleteAuthCredential(id);
|
||||
deleteAuthCredential(id: number, disabledCause: string): void {
|
||||
this.#authStore.deleteAuthCredential(id, disabledCause);
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes all auth credentials for a provider.
|
||||
* @param provider - Provider name whose credentials should be deleted
|
||||
* Disables all auth credentials for a provider with a persisted cause.
|
||||
* @param provider - Provider name whose credentials should be disabled
|
||||
* @param disabledCause - Human-readable cause stored with the disabled rows
|
||||
*/
|
||||
deleteAuthCredentialsForProvider(provider: string): void {
|
||||
this.#authStore.deleteAuthCredentialsForProvider(provider);
|
||||
deleteAuthCredentialsForProvider(provider: string, disabledCause: string): void {
|
||||
this.#authStore.deleteAuthCredentialsForProvider(provider, disabledCause);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user