fix(exec-approval): treat expiresAtMs=-1 as non-expiring
This commit is contained in:
parent
a36cbac3fd
commit
6ecdbc5783
4 changed files with 33 additions and 7 deletions
|
|
@ -22,7 +22,7 @@ export interface ExecApprovalRequestPayload {
|
|||
riskLevel: "safe" | "needs-review" | "dangerous";
|
||||
/** Reasons for the risk assessment */
|
||||
riskReasons: string[];
|
||||
/** When this approval expires (ms since epoch) */
|
||||
/** When this approval expires (ms since epoch). -1 means no timeout. */
|
||||
expiresAtMs: number;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -20,12 +20,17 @@ export interface ExecApprovalItemProps {
|
|||
onDecision: (decision: "allow-once" | "allow-always" | "deny") => void
|
||||
}
|
||||
|
||||
function useCountdown(expiresAtMs: number): number {
|
||||
const [remaining, setRemaining] = useState(() =>
|
||||
Math.max(0, Math.ceil((expiresAtMs - Date.now()) / 1000)),
|
||||
function useCountdown(expiresAtMs: number): number | null {
|
||||
const [remaining, setRemaining] = useState<number | null>(() =>
|
||||
expiresAtMs < 0 ? null : Math.max(0, Math.ceil((expiresAtMs - Date.now()) / 1000)),
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
if (expiresAtMs < 0) {
|
||||
setRemaining(null)
|
||||
return
|
||||
}
|
||||
|
||||
const id = setInterval(() => {
|
||||
const next = Math.max(0, Math.ceil((expiresAtMs - Date.now()) / 1000))
|
||||
setRemaining(next)
|
||||
|
|
@ -73,7 +78,7 @@ export const ExecApprovalItem = memo(function ExecApprovalItem({
|
|||
<HugeiconsIcon icon={CommandLineIcon} strokeWidth={2} className="size-3.5 shrink-0" />
|
||||
<span className="font-medium text-foreground">{riskLabel}</span>
|
||||
</div>
|
||||
{remaining > 0 && !decided && (
|
||||
{remaining !== null && remaining > 0 && !decided && (
|
||||
<span className="text-xs text-muted-foreground/60 font-[tabular-nums]">
|
||||
{remaining}s
|
||||
</span>
|
||||
|
|
@ -100,7 +105,7 @@ export const ExecApprovalItem = memo(function ExecApprovalItem({
|
|||
)}
|
||||
|
||||
{/* Actions */}
|
||||
{!decided && remaining > 0 ? (
|
||||
{!decided && (remaining === null || remaining > 0) ? (
|
||||
<div className="flex items-center gap-2">
|
||||
<Button
|
||||
variant="outline"
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ export interface ExecApprovalRequest {
|
|||
riskLevel: "safe" | "needs-review" | "dangerous";
|
||||
/** Reasons for the risk assessment */
|
||||
riskReasons: string[];
|
||||
/** When this approval expires (ms since epoch) */
|
||||
/** When this approval expires (ms since epoch). -1 means no timeout. */
|
||||
expiresAtMs: number;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -88,6 +88,27 @@ describe("ExecApprovalManager", () => {
|
|||
expect(result.decision).toBe("deny");
|
||||
});
|
||||
|
||||
it("keeps approval pending indefinitely when timeoutMs is -1", async () => {
|
||||
const promise = manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
command: "cmd",
|
||||
riskLevel: "needs-review",
|
||||
riskReasons: [],
|
||||
timeoutMs: -1,
|
||||
});
|
||||
|
||||
const request = sendToClient.mock.calls[0]![1];
|
||||
expect(request.expiresAtMs).toBe(-1);
|
||||
|
||||
vi.advanceTimersByTime(60_000);
|
||||
expect(manager.pendingCount).toBe(1);
|
||||
|
||||
manager.resolveApproval(request.approvalId, "allow-once");
|
||||
const result = await promise;
|
||||
expect(result.approved).toBe(true);
|
||||
expect(result.decision).toBe("allow-once");
|
||||
});
|
||||
|
||||
it("honors askFallback full on timeout", async () => {
|
||||
const promise = manager.requestApproval({
|
||||
agentId: "agent-1",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue