feat(settings): onboard hub provider credentials for dictation and voice - #1392
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Alias env overrides can be shadowed by a saved canonical alias — the startup loop locks individual names, so a saved GEMINI_API_KEY is copied into process.env even when GOOGLE_API_KEY came from the startup environment; runtime consumers prefer GEMINI_API_KEY, contradicting the documented env precedence. The same applies to DASHSCOPE_API_KEY/QWEN_API_KEY. Evidence hub/src/config/providerCredentials.ts:189.
Suggested fix:function isLogicallyEnvLocked(key: ProviderCredentialEnvKey): boolean { if (key === 'GEMINI_API_KEY' || key === 'GOOGLE_API_KEY') { return envLockedKeys.has('GEMINI_API_KEY') || envLockedKeys.has('GOOGLE_API_KEY') } if (key === 'DASHSCOPE_API_KEY' || key === 'QWEN_API_KEY') { return envLockedKeys.has('DASHSCOPE_API_KEY') || envLockedKeys.has('QWEN_API_KEY') } return envLockedKeys.has(key) }
- [Major] Blank Save values are encoded as deletion — Save sends null for an untouched password, and OpenAI-compatible fields are never initialized from the returned status, so replacing one field can clear the endpoint, model, or existing API key. A separate Clear action already exists. Evidence web/src/components/settings/TranscriptionProviderOnboard.tsx:116.
Suggested fix:const value = clear ? null : (apiKey.trim() || undefined) // Initialize baseUrl/model from status, and send undefined for untouched fields. baseUrl: baseUrl.trim() || undefined, model: model.trim() || undefined, apiKey: value,
- [Major] Provider secrets are persisted with default filesystem permissions — this new write stores long-lived third-party API keys through writeSettings(), whose temp file has no explicit mode; HAPI_HOME is also created without a restrictive mode. With a common umask 022, settings.json becomes 0644 and is readable by other local users. Evidence hub/src/config/providerCredentials.ts:338.
Suggested fix:await mkdir(dir, { recursive: true, mode: 0o700 }) await chmod(dir, 0o700) await writeFile(tmpFile, JSON.stringify(settings, null, 2), { mode: 0o600 }) await chmod(tmpFile, 0o600) await rename(tmpFile, settingsFile)
Summary
Review mode: initial
Three Major issues found: alias precedence violation, destructive blank-save behavior, and insecure on-disk permissions for newly persisted provider credentials.
Testing
Not run (review automation; PR content was not executed). Missing coverage: saved primary alias plus secondary env override, editing credentials while preserving untouched fields, and POSIX settings file/directory modes.
HAPI Bot
Alias env locks, non-destructive Save (omit empty fields), and owner-only settings.json permissions for hub-stored provider secrets. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed the three Major findings in 9091224:
Added regression tests for alias shadowing, omit-vs-clear, and POSIX mode |
There was a problem hiding this comment.
Findings
- [Major] Restrict hub-global credential management to the owner namespace — these routes are mounted behind authentication, but unlike other hub-global endpoints they do not check
c.get('namespace'). Any member of a shared-team hub can therefore inspect credential metadata and replace or clear credentials used by every namespace. Evidencehub/src/web/routes/voice.ts:490.
Suggested fix:if (c.get('namespace') !== 'default') { return c.json({ error: 'Provider credentials are only available to the hub owner' }, 403) }
- [Major] Stage updates before changing live
process.env— the patch helpers mutate runtime credentials before all validation and beforewriteSettings()succeeds. A multi-field request that later hits an env lock returns 409 after earlier keys changed; a disk error returns 500 while the new credential remains active until restart. Evidencehub/src/config/providerCredentials.ts:271.
Suggested fix:const nextStored = { ...stored } applyPatchToStored(nextStored, key, value) // stored map only; validate every field await writeSettings(settingsFile, { ...settings, providerCredentials: nextStored }) syncSettingsCredentialsToEnv(nextStored)
- [Major] Preserve per-field editing for mixed OpenAI-compatible configuration —
baseUrlEditable,modelEditable, andapiKey.editableare collapsed with&&, then that aggregate disables every input and both actions. If only one field comes from env (for example an env API token with a Settings-managed local URL/model), the UI cannot edit or clear the remaining Settings fields, defeating this onboarding path. Evidenceweb/src/components/settings/TranscriptionProviderOnboard.tsx:54.
Suggested fix:disabled={busy || !status?.openaiCompatible.baseUrlEditable} // model: use modelEditable; password: use apiKey.editable // Clear should send null only for fields whose editable flag is true.
Summary
Review mode: follow-up after new commits
Three Major issues remain at the current head: cross-namespace mutation of hub-global credentials, non-atomic live credential updates, and unusable mixed env/Settings OpenAI-compatible configurations.
Testing
Not run (review automation; PR content was not executed). Missing coverage: non-default namespace GET/PUT rejection, rollback/no runtime change on validation or persistence failure, and mixed per-field env locks in the settings UI.
HAPI Bot
|
Second-pass Majors addressed:
Tests cover non-owner 403, locked multi-field no env mutation, write failure no env mutation, and mixed-lock editability flags. |
There was a problem hiding this comment.
Findings
- [Major] Preserve incomplete OpenAI-compatible entries as clearable state - the Clear action is rendered only when
selected.configuredis true, but that flag requires both base URL and model. The API and Save flow allow partial values, including an API key alone; after reopening, that stored secret is not considered configured, blank Save means "unchanged," and no Clear button is available. Evidenceweb/src/components/settings/TranscriptionProviderOnboard.tsx:280.
Suggested fix:const canClearCompatible = provider === 'openai-compatible' && Boolean( (baseUrlEditable && compatible?.baseUrl) || (modelEditable && compatible?.model) || (apiKeyEditable && compatible?.apiKey.configured) ) const canClear = canClearCompatible || Boolean( provider !== 'openai-compatible' && selected?.configured && selected.editable ) // Render Clear when canClear is true.
- [Major] Serialize the credential read-modify-write transaction - each PUT reads the whole settings file, writes through the shared
settings.json.tmp, then syncs its private snapshot intoprocess.env. Two concurrent saves (or a save racing relay-key persistence) can lose unrelated fields, fail one rename with ENOENT, or leave disk containing one snapshot while runtime uses another. Evidencehub/src/config/providerCredentials.ts:354.
Suggested fix:return updateSettings(settingsFile, async (settings) => { const nextStored = { ...readProviderCredentials(settings) } applyCredentialPatch(nextStored, update) return { settings: { ...settings, providerCredentials: nextStored }, afterCommit: () => syncSettingsCredentialsToEnv(nextStored), } }) // updateSettings locks the full read/modify/write per file
Summary
Review mode: follow-up after new commits
The previously reported namespace, atomic-failure, and mixed-lock issues are addressed at this head. Two Major issues remain: partial OpenAI-compatible credentials can become impossible to remove through the UI, and concurrent settings writes can lose or desynchronize credentials.
Testing
Not run (review automation; PR content was not executed). Missing coverage: clearing base-URL-only/model-only/API-key-only OpenAI-compatible state, and concurrent credential/settings writes.
HAPI Bot
|
Third-pass Majors addressed:
Tests cover concurrent openai+groq updates and API-key-only partial status. |
There was a problem hiding this comment.
Findings
- [Major] Serialize credential updates against every settings writer — the new lock only orders callers that explicitly use
withSettingsLock. Runtime relay-key paths still perform unlocked read-modify-write operations through the same fixedsettings.json.tmp, so a credential PUT racing relay issuance/refresh can still loseproviderCredentials, lose the relay key, or fail a rename. Evidencehub/src/config/providerCredentials.ts:309; related contexthub/src/tunnel/relayAuth.ts:46.
Suggested fix:export async function updateSettings<T>( settingsFile: string, mutate: (settings: Settings) => Promise<{ settings: Settings; result: T }> ): Promise<T> { return withSettingsLock(settingsFile, async () => { const current = await readSettingsOrThrow(settingsFile) const { settings, result } = await mutate(current) await writeSettings(settingsFile, settings) return result }) } // Use this helper for credentials and every runtime settings read-modify-write.
- [Minor] Reset OpenAI-compatible form state after Clear — for URL-only or model-only partial configuration, the provider remains incomplete, so automatic onboarding stays mounted. The server value is cleared, but
baseUrl/modelstate still shows the old value; pressing Save restores the credential that was just cleared. Evidenceweb/src/components/settings/TranscriptionProviderOnboard.tsx:182.
Suggested fix:setStatus(next) setApiKey('') setBaseUrl(next.openaiCompatible.baseUrlEditable ? next.openaiCompatible.baseUrl ?? '' : '') setModel(next.openaiCompatible.modelEditable ? next.openaiCompatible.model ?? '' : '')
Summary
Review mode: follow-up after new commits
Two findings remain at the latest head: settings serialization does not cover cross-writer races, and clearing partial OpenAI-compatible fields can leave stale form state that re-saves the cleared value. No other high-confidence issues found in the full current diff.
Testing
Not run (review automation; PR content was not executed). Missing coverage: a credential update racing relay-key persistence, plus URL-only/model-only component state after Clear.
HAPI Bot
|
Fourth-pass findings addressed:
|
There was a problem hiding this comment.
Findings
- [Major] Join the CLI's cross-process settings lock - the new promise chain only serializes writers inside the hub process. The CLI writes the same
~/.hapi/settings.jsonundersettings.json.lockand uses the same fixed.tmppath, but the hub ignores that lock. A credential PUT racinghapi auth login/logoutcan therefore lose credentials or CLI fields, rename the other process's temp file, or fail withENOENT. Evidencehub/src/config/settings.ts:71; related contextcli/src/persistence.ts:106.
Suggested fix:// Shared by hub and CLI; retain the CLI retry/stale-lock handling. return withSettingsFileLock(settingsFile, async () => { const current = await readSettingsOrThrow(settingsFile) const outcome = await mutate(current) if (outcome.write !== false) await writeSettingsUnlocked(settingsFile, outcome.settings) return outcome.result })
- [Major] Keep secret-bearing settings owner-only after CLI rewrites - the hub now writes mode
0600, but the CLI's atomic updater recreatessettings.json.tmpwith Node's default0666 & umaskand renames it over this file. With a common022umask, any later CLI settings update turns the file containing provider API keys into0644. Evidencehub/src/config/settings.ts:89; related contextcli/src/persistence.ts:148.
Suggested fix:await writeFile(tmpFile, JSON.stringify(updated, null, 2), { mode: 0o600 }) await chmod(tmpFile, 0o600) await rename(tmpFile, configuration.settingsFile) await chmod(configuration.settingsFile, 0o600)
Summary
Review mode: follow-up after new commits
Two Major shared-file risks remain at the latest head: hub/CLI updates are not mutually serialized across processes, and CLI rewrites can widen permissions on the newly secret-bearing settings file. The previous stale OpenAI-compatible Clear state is fixed in the current component.
Testing
Not run (review automation; PR code was not executed). GitHub test check passed. Missing coverage: a real hub-writer/CLI-writer race against one settings file, and a POSIX mode assertion after a CLI update rewrites settings containing provider credentials.
HAPI Bot
|
Fifth-pass Majors addressed:
Integration test races hub credential PUT against a CLI-style lock writer and asserts both fields survive with mode |
There was a problem hiding this comment.
Findings
- [Major] Keep UI-managed provider secrets out of the global process environment — both startup loading and live saves copy settings-backed API keys into
process.env. The hub subsequently inherits that environment into unrelated subprocesses, including the tunnel binary and Cursor/Codex helpers, so adding a key in Settings exposes it to processes that do not need voice credentials. Evidencehub/src/config/providerCredentials.ts:202; related contexthub/src/tunnel/tunnelManager.ts:112,hub/src/cursor/acpVerifyProbe.ts:282,hub/src/web/routes/codexDesktop.ts:1621.
Suggested fix:Use this effective provider environment only in voice discovery/request paths; do not mutatelet settingsBackedCredentials: ProviderCredentialsMap = {} function setSettingsBackedCredentials(stored: ProviderCredentialsMap): void { settingsBackedCredentials = Object.fromEntries( Object.entries(stored).filter(([key]) => !isLogicallyEnvLocked(key as ProviderCredentialEnvKey) ) ) as ProviderCredentialsMap } export function getProviderEnvironment(): NodeJS.ProcessEnv { return { ...process.env, ...settingsBackedCredentials } }
process.env. - [Major] Do not expire a settings lock solely by file age — after laptop suspend, debugger pause, or any holder delay beyond 10 seconds, another process removes the still-live lock. The original holder then unconditionally removes the successor's lock in
finally, allowing a third writer to enter and reintroducing lost fields/temp-file races for secret-bearingsettings.json. Evidenceshared/src/settingsFileLock.ts:35andshared/src/settingsFileLock.ts:57.
Suggested fix:const owner = JSON.stringify({ pid: process.pid, token: randomUUID() }) fileHandle = await open(lockFile, 'wx', 0o600) await fileHandle.writeFile(owner) // Break only locks whose recorded process is no longer alive. if (!isProcessAlive(existing.pid)) await unlink(lockFile) // Never delete a lock acquired by a successor. await fileHandle.close() if (await readFile(lockFile, 'utf8').catch(() => null) === owner) { await unlink(lockFile) }
Summary
Review mode: follow-up after new commits
Two Major risks remain at the latest head: settings-managed provider secrets cross the hub subprocess boundary, and the new shared lock can steal a live lock after a long pause, permitting settings clobbering.
Testing
Not run (automation; PR code was not executed). GitHub test check passed. Missing coverage: asserting provider keys are absent from spawned child environments, and a lock-ownership test where a holder survives beyond the stale threshold.
HAPI Bot
|
Sixth-pass Majors addressed in ee9e5b5:
Tests cover overlay vs |
There was a problem hiding this comment.
Findings
- [Blocker] Do not reclaim an unreadable lock during acquisition —
open(..., 'wx')creates the sidecar before the owner JSON is written, so a concurrent writer can observenull, unlink the live lock, and enter the critical section too. Both writers then usesettings.json.tmp; the current GitHubtestcheck reproduces this asENOENTathub/src/config/settings.ts:102. Evidenceshared/src/settingsFileLock.ts:68.
Suggested fix:const existing = await readLockOwner(lockFile) if (existing && !isPidAlive(existing.pid)) { await unlink(lockFile).catch(() => {}) continue } // null may be a just-created lock whose payload is still being written. await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_INTERVAL_MS))
Summary
Review mode: follow-up after new commits
One Blocker remains at the latest head: the revised settings lock still permits concurrent writers during the owner-payload publication window, and CI currently fails on that race.
Testing
Not run locally (automation; PR code was not executed). Current GitHub test check failed in providerCredentials > preserves hub credentials when raced with CLI-style settings lock writer; missing coverage: pause the first writer between exclusive create and owner-payload write, then assert a second writer waits.
HAPI Bot
|
Blocker addressed in the tip commit: Do not reclaim ownerless lock sidecars — acquisition now uses sync Coverage: empty-sidecar fail-closed, dead-PID reclaim, successor-safe release, 8-way concurrent writers. |
There was a problem hiding this comment.
Findings
-
[Major] Serialize stale-lock reclamation before unlinking — two contenders can both read the same dead owner. After the first removes the stale sidecar and acquires a new lock, the second unconditional unlink can delete that live successor, allowing both callbacks to use
settings.json.tmpconcurrently and lose settings. Evidenceshared/src/settingsFileLock.ts:87.
Suggested fix:if (existing && !isPidAlive(existing.pid)) { // Fail closed until stale reclamation is protected by a separate // exclusive reaper lock and re-validates the same owner under it. throw new Error(`Stale settings lock requires cleanup: ${lockFile}`) }
-
[Major] Clean up a sidecar when owner publication fails —
openSync(..., 'wx')creates the visible lock beforewriteSync. A thrown or short write leaves an empty/partial sidecar, and the new fail-closed reader will never reclaim it; every later settings mutation then times out even after the underlying I/O problem clears. Evidenceshared/src/settingsFileLock.ts:69.
Suggested fix:const fd = openSync(lockFile, 'wx', 0o600) try { const payload = Buffer.from(ownerPayload) let offset = 0 while (offset < payload.length) { const written = writeSync(fd, payload, offset, payload.length - offset) if (written === 0) throw new Error('Failed to publish settings lock owner') offset += written } acquired = true } catch (error) { unlinkSync(lockFile) throw error } finally { closeSync(fd) }
Summary
Review mode: follow-up after new commits
The prior ownerless-publication-window finding is addressed, but two Major failure paths remain in the shared settings lock: concurrent stale-lock reapers can still break exclusivity, and owner publication failure can leave the hub/CLI unable to mutate settings.
Testing
Not run (automation; PR code was not executed). GitHub test was pending at review time. Add deterministic tests that synchronize two contenders after both read the same dead owner, and that inject a thrown/short owner write then verify the sidecar is removed and a subsequent acquisition succeeds.
HAPI Bot
|
Seventh-pass Majors addressed:
Tests cover write-failure cleanup, multi-contender dead-owner reclaim (max concurrency 1), and the existing empty-sidecar / successor-release cases. |
There was a problem hiding this comment.
Findings
- [Major] Serialize stale-lock reclamation before moving the live path — unique break names do not stop a delayed contender from renaming a newly acquired successor lock, so concurrent settings writers can still enter together and lose fields. Evidence
shared/src/settingsFileLock.ts:93.
Suggested fix:// Fail closed until stale reclamation is serialized by one fixed reaper lock. if (existing && !isPidAlive(existing.pid)) { await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_INTERVAL_MS)) continue }
- [Minor] Add
providerCredentialsto the published strict settings schema — HAPI now writes this top-level field, butdocs/public/schemas/settings.schema.jsonhasadditionalProperties: false, so editor/schema validation rejects a settings file generated by the feature. Evidencehub/src/config/settings.ts:30.
Suggested fix:"providerCredentials": { "type": "object", "additionalProperties": { "type": "string" } }
Questions
- None.
Summary
Review mode: follow-up after new commits
The owner-publication cleanup from the prior review is addressed. The stale-reclamation race remains through a different interleaving, and the new persisted field is missing from the repository's strict JSON schema.
Testing
Not run (automation; PR code was not executed). GitHub test and pr-review checks were pending. Add a deterministic race test that pauses one contender after reading the dead owner while another reclaims and acquires the successor lock; also validate a generated settings file containing providerCredentials against the published schema.
HAPI Bot
|
Eighth-pass findings addressed:
|
There was a problem hiding this comment.
Findings
- [Major] Fail closed when the CLI cannot read the shared settings document — the new locked CLI update still treats any read/JSON failure as an empty object and then replaces
settings.json, which can erase hub-only fields includingproviderCredentials,relayAuthKey, andvapidKeys. Evidencecli/src/persistence.ts:111.
Suggested fix:async function readSettingsForUpdate(): Promise<Settings> { if (!existsSync(configuration.settingsFile)) return { ...defaultSettings } const content = await readFile(configuration.settingsFile, 'utf8') const parsed: unknown = JSON.parse(content) if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { throw new Error(`Invalid settings file: ${configuration.settingsFile}`) } return parsed as Settings } const current = await readSettingsForUpdate()
- [Major] Back off when another process owns the stale-lock reaper —
tryReclaimDeadOwner()returns false while${lock}.reapexists, but this branch immediately retries without sleeping. A process scheduled while another contender holds the reaper can consume all 50 attempts synchronously and fail hub startup or a credential save instead of waiting the advertised five seconds. Evidenceshared/src/settingsFileLock.ts:192.
Suggested fix:const reclaimed = tryReclaimDeadOwner(lockFile, existing) if (!reclaimed) { await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_INTERVAL_MS)) } continue
Questions
- None.
Summary
Review mode: follow-up after new commits
The previous strict-schema issue is resolved, and the original delayed-renamer race was replaced by a fixed reaper. The current head still has one settings data-loss path and one cross-process stale-recovery failure.
Testing
Not run (automation; PR code was not executed). GitHub test passed; git diff --check passed. Add a CLI test asserting invalid settings bytes remain unchanged after an attempted update, plus a cross-process or barrier test that pauses one process after acquiring .reap while another retries.
HAPI Bot
|
Ninth-pass Majors addressed:
|
There was a problem hiding this comment.
Findings
- [Major] Make settings-lock acquisition crash-recoverable —
openSync(..., 'wx')exposes the fixed lock path before the owner JSON is published. Exception cleanup handles a thrown write, but a SIGKILL between those operations leaves an empty/partial lock;readLockOwnerSync()then returnsnullon every retry, so hub startup and all later settings writes fail until the sidecar is manually deleted. Evidenceshared/src/settingsFileLock.ts:165.
Suggested fix:function publishLockAtomically(lockFile: string, ownerPayload: string): void { const candidate = `${lockFile}.${randomUUID()}.candidate` writeFileSync(candidate, ownerPayload, { flag: 'wx', mode: 0o600 }) try { linkSync(candidate, lockFile) // EEXIST, or a fully initialized lock } finally { unlinkSync(candidate) } }
- [Major] Fix the hoisted configuration mock so the required test job can run — Vitest hoists the
vi.mockfactory above the top-leveldirinitialization, producingReferenceError: Cannot access 'dir' before initialization. The current GitHubtestcheck fails at this line before the new regression test executes, and hub/web/shared tests never run. Evidencecli/src/persistence.test.ts:10.
Suggested fix:const dir = mkdtempSync(join(tmpdir(), 'hapi-cli-settings-')) vi.doMock('@/configuration', () => ({ configuration: { happyHomeDir: dir, settingsFile: join(dir, 'settings.json'), // ... }, })) const { updateSettings } = await import('./persistence')
Questions
- None.
Summary
Review mode: follow-up after new commits
The two findings from the previous HAPI Bot pass are addressed. The current head still has one crash-recovery path that can wedge the shared settings file and one deterministic failure in the required test workflow.
Testing
Not run locally (automation; PR code was not executed). Current GitHub test check failed in cli/src/persistence.test.ts with the hoisted-mock TDZ error; 211 CLI test files passed before the job stopped. Add a child-process test that terminates after exclusive creation but before owner publication, then verifies a later acquisition can recover.
HAPI Bot
|
Tenth-pass Majors addressed:
|
There was a problem hiding this comment.
Findings
- [Major] Make the reaper sidecar crash-recoverable — the main lock is now atomically published, but
tryReclaimDeadOwner()creates${lock}.reapwithopenSync(..., 'wx')and only removes it in normal exception/finally paths. If the process is killed after line 111, the fixed reaper remains forever; every later contender getsEEXIST, returnsfalse, and eventually times out, so hub startup/settings writes remain wedged until manual deletion. Evidenceshared/src/settingsFileLock.ts:111.
Suggested fix:Replace both bespoke sidecars with an equivalent crash-recoverable lease/advisory lock, and add a child-process test that dies while holding the reclaim guard.import lockfile from 'proper-lockfile' const release = await lockfile.lock(settingsFile, { realpath: false, stale: 30_000, update: 10_000, retries: { retries: 50, minTimeout: 100, maxTimeout: 100 }, }) try { return await work() } finally { await release() }
- [Minor] Hide the owner-only credential editor from tenant namespaces — the API deliberately returns 403 outside
default, but this page renders and even auto-opens the editor for every authenticated client because it gates only onapi. Tenant users can paste a secret and receive a failure after submission. Evidenceweb/src/routes/settings/voice.tsx:96.
Suggested fix:Reuse/factor the JWT namespace helper already used byconst { api, token } = useAppContext() const canManageCredentials = getNamespace(token) === 'default' const credentialsOpen = canManageCredentials && (showCredentials || needsDictationOnboard || needsAssistantOnboard) {canManageCredentials ? <TranscriptionProviderOnboard /* ... */ /> : null}
SettingsNav, and cover the tenant rendering case.
Questions
- None.
Summary
Review mode: follow-up after new commits
The current head still has a permanent settings-lock wedge after a crash in the reclaim guard, plus an owner-only control exposed to tenant UIs.
Testing
Not run locally (automation; PR code was not executed). GitHub test check passed for 4c8e06e93026d8209028262c124242efff9cceba. Add the reaper-crash child-process test and a tenant voice-settings rendering test.
HAPI Bot
|
Eleventh-pass findings addressed by taking Codex's escape hatch:
|
|
Eleventh-pass findings addressed by taking Codex's escape hatch (fe71c2f):
|
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No high-confidence issues found in the full latest diff. The prior HAPI Bot findings about crash-recoverable settings locking and tenant visibility are addressed at this head. Residual risk is concentrated in unexercised browser rendering and real process-crash recovery paths.
Testing
Not run (automation; PR code was not executed). The GitHub test check was still in progress at review time. Suggested coverage: kill a child process while it holds the proper-lockfile lease and verify a later writer recovers; render Voice settings with a non-default namespace JWT and verify the credential editor is absent.
HAPI Bot
Alias env locks, non-destructive Save (omit empty fields), and owner-only settings.json permissions for hub-stored provider secrets. Co-authored-by: Cursor <cursoragent@cursor.com>
fe71c2f to
37c8980
Compare
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No high-confidence issues found in the full latest diff. The rebase-specific settings API adaptation preserves the prior credential-locking and owner-namespace protections. Residual risk remains in browser rendering/state transitions for the new credential editor and real process-crash recovery of the shared settings lock.
Testing
Not run (automation; PR code was not executed). The GitHub test check passed. Suggested coverage: render the credential editor with environment-locked and mixed OpenAI-compatible fields, exercise save/clear refresh behavior, and kill a child process while it holds the settings lease to verify recovery.
HAPI Bot
Env-only keys made dictation invisible; Settings can now add/edit/clear hub-side credentials (masked), with env still winning as override. Refs tiann#1384. Co-authored-by: Cursor <cursoragent@cursor.com>
Same Settings credential surface now covers ElevenLabs, Gemini Live, and Qwen Realtime (alias env pairs), not only transcription providers. Co-authored-by: Cursor <cursoragent@cursor.com>
Alias env locks, non-destructive Save (omit empty fields), and owner-only settings.json permissions for hub-stored provider secrets. Co-authored-by: Cursor <cursoragent@cursor.com>
Owner-namespace gate, stage-then-sync env after persist, and per-field OpenAI-compatible editability under mixed env locks. Co-authored-by: Cursor <cursoragent@cursor.com>
Per-file settings lock for concurrent credential PUTs, and Clear shown for partial OpenAI-compatible entries (key/url/model alone). Co-authored-by: Cursor <cursoragent@cursor.com>
Route credentials, relay auth, generators, server settings, and CLI token persistence through a locked RMW helper; reset Clear form state. Co-authored-by: Cursor <cursoragent@cursor.com>
Extract withSettingsFileLock for hub+CLI, keep owner-only 0o600 rewrites, and race hub credential updates against CLI-style writers. Co-authored-by: Cursor <cursoragent@cursor.com>
…ocks Settings-backed provider credentials now live in an in-memory overlay (getProviderEnvironment) so tunnel/ACP/Codex children do not inherit them. Settings file locks record pid+token and only reclaim dead or legacy locks. Co-authored-by: Cursor <cursoragent@cursor.com>
wx creates the lock path before the owner JSON is visible; unlinking null owners let a waiter steal a live acquisition and collide on settings.json.tmp (CI ENOENT). Only reclaim parsed owners with dead PIDs. Co-authored-by: Cursor <cursoragent@cursor.com>
Stale reclaim renames the sidecar to a unique break path and re-verifies the expected dead owner before deleting it, so a loser cannot unlink a successor's live lock. Failed owner writes unlink the wx sidecar. Reclaim uses a sync owner read so contenders do not all observe one dead owner across an await and race the exclusive create. Co-authored-by: Cursor <cursoragent@cursor.com>
Stale reclaim now takes a fixed settings.json.lock.reap lock, re-validates pid+token, then unlinks — so a delayed contender cannot move a successor's live lock aside. Also document providerCredentials in settings.schema.json. Co-authored-by: Cursor <cursoragent@cursor.com>
CLI updateSettings now uses a strict read that rejects invalid JSON
instead of treating errors as {}, which could wipe providerCredentials.
Settings lock reclaim sleeps when another process holds .reap so retries
are not burned synchronously.
Co-authored-by: Cursor <cursoragent@cursor.com>
Acquire settings locks by writing a complete candidate then linkSync to the fixed path so a crash cannot leave an empty live sidecar. Fix the CLI persistence regression test to create its temp dir inside vi.hoisted. Co-authored-by: Cursor <cursoragent@cursor.com>
… creds UI Codex kept finding crash windows in hand-rolled lock sidecars. Switch the shared settings lock to proper-lockfile's mkdir + mtime lease. Hide the owner-only credentials editor from non-default namespaces on the voice page. Co-authored-by: Cursor <cursoragent@cursor.com>
Rebase onto main brought tiann#1376 unique tmp + outcome-shaped writers; wire sessionSummaryContract and the write-failure credential test to match. Co-authored-by: Cursor <cursoragent@cursor.com>
37c8980 to
da0c6c2
Compare
Empty commit — Meta reported no checks on da0c6c2 after tip-forward rebase. Co-authored-by: Cursor <cursoragent@cursor.com>
|
CI kick: no check-suites on tip after rebase (GitHub Actions lag/outage). Reopening to retrigger workflows. |
Co-authored-by: Cursor <cursoragent@cursor.com>
Tip-forward can land new deps (tiann#1392 proper-lockfile) while live driver/node_modules stays stale; typecheck ran against DRIVER and rolled back a good tip. Co-authored-by: Cursor <cursoragent@cursor.com>
tiann#1392 upstream take of hub server.ts dropped createFeaturesRoutes; PR chips depend on GET /api/features. Heal re-applies the mount on remat. Co-authored-by: Cursor <cursoragent@cursor.com>
Same tip-forward tiann#1392 class as features (heal 99). Restores Session Log and overseer HTTP mounts on remat. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
hub.env.providerCredentialsinsettings.json(masked reads; authenticated writes; process env still wins when set).Motivation
Env-only configuration for these keys was a shipping shortcut, not a design win. HAPI is local-first / single-operator; empty "No transcription provider is configured on the hub" with no UI path made the feature look broken. Env remains a valid ops override.
Test plan
bun typecheckproviderCredentials, voice routes, client, voice settings)Made with Cursor