From fa7eeca7ce08314d5781058c0fbfdf7a9ab38b40 Mon Sep 17 00:00:00 2001 From: rshirali Date: Wed, 28 Jan 2026 14:53:39 +0100 Subject: [PATCH] feat(gateway): add register and awaitDecision methods to ExecApprovalManager Separates registration (synchronous) from waiting (async) to allow callers to confirm registration before the decision is made. Adds grace period for resolved entries to prevent race conditions. --- src/gateway/exec-approval-manager.ts | 63 ++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/src/gateway/exec-approval-manager.ts b/src/gateway/exec-approval-manager.ts index 5e51f2abc..ed5dfec54 100644 --- a/src/gateway/exec-approval-manager.ts +++ b/src/gateway/exec-approval-manager.ts @@ -28,6 +28,7 @@ type PendingEntry = { resolve: (decision: ExecApprovalDecision | null) => void; reject: (err: Error) => void; timer: ReturnType; + promise: Promise; }; export class ExecApprovalManager { @@ -49,17 +50,47 @@ export class ExecApprovalManager { return record; } + /** + * Register an approval record and return a promise that resolves when the decision is made. + * This separates registration (synchronous) from waiting (async), allowing callers to + * confirm registration before the decision is made. + */ + register(record: ExecApprovalRecord, timeoutMs: number): Promise { + let resolvePromise: (decision: ExecApprovalDecision | null) => void; + let rejectPromise: (err: Error) => void; + const promise = new Promise((resolve, reject) => { + resolvePromise = resolve; + rejectPromise = reject; + }); + const timer = setTimeout(() => { + const entry = this.pending.get(record.id); + resolvePromise(null); + // Keep entry briefly for in-flight awaitDecision calls + setTimeout(() => { + if (this.pending.get(record.id) === entry) { + this.pending.delete(record.id); + } + }, 5000); + }, timeoutMs); + // Registration happens synchronously here + this.pending.set(record.id, { + record, + resolve: resolvePromise!, + reject: rejectPromise!, + timer, + promise, + }); + return promise; + } + + /** + * @deprecated Use register() instead for explicit separation of registration and waiting. + */ async waitForDecision( record: ExecApprovalRecord, timeoutMs: number, ): Promise { - return await new Promise((resolve, reject) => { - const timer = setTimeout(() => { - this.pending.delete(record.id); - resolve(null); - }, timeoutMs); - this.pending.set(record.id, { record, resolve, reject, timer }); - }); + return this.register(record, timeoutMs); } resolve(recordId: string, decision: ExecApprovalDecision, resolvedBy?: string | null): boolean { @@ -69,8 +100,15 @@ export class ExecApprovalManager { pending.record.resolvedAtMs = Date.now(); pending.record.decision = decision; pending.record.resolvedBy = resolvedBy ?? null; - this.pending.delete(recordId); + // Resolve the promise first, then delete after a grace period. + // This allows in-flight awaitDecision calls to find the resolved entry. pending.resolve(decision); + setTimeout(() => { + // Only delete if the entry hasn't been replaced + if (this.pending.get(recordId) === pending) { + this.pending.delete(recordId); + } + }, 5000); return true; } @@ -78,4 +116,13 @@ export class ExecApprovalManager { const entry = this.pending.get(recordId); return entry?.record ?? null; } + + /** + * Wait for decision on an already-registered approval. + * Returns the decision promise if the ID is pending, null otherwise. + */ + awaitDecision(recordId: string): Promise | null { + const entry = this.pending.get(recordId); + return entry?.promise ?? null; + } }