From 9d84d5c5c4573bd2b517d0707f33fab4f8608a67 Mon Sep 17 00:00:00 2001 From: timmy Date: Wed, 26 Aug 2026 00:56:52 +0000 Subject: [PATCH] fix: close Human Gates review races --- frontend/dashboard.js | 2 ++ frontend/human-gates.js | 49 +++++++++++++++++++++--------- src/human_gate_store.py | 11 +++++-- tests/test_human_gate_store.py | 19 ++++++++++++ tests/test_human_gates_frontend.py | 26 ++++++++++++++++ 5 files changed, 89 insertions(+), 18 deletions(-) diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 9ab6edb..fe86c5b 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -7886,6 +7886,8 @@ confirmedOwnerLogin = String(saved.user?.login || '').trim(); restoreReleaseReceipt(); planningOwnerLogin = confirmedOwnerLogin; + planningOwnerAccountKey = confirmedOwnerLogin && saved.user?.id ? + String(saved.user.id) + ':' + confirmedOwnerLogin : ''; interruptionPrompt.restore(); updatePlanningAvailability(); syncPendingTomorrow(); diff --git a/frontend/human-gates.js b/frontend/human-gates.js index 052b668..40ce754 100644 --- a/frontend/human-gates.js +++ b/frontend/human-gates.js @@ -9,7 +9,9 @@ function createHumanGates(options = {}) { let queue = { pending_count: 0, items: [] }; let reviewSnapshot = []; let reviewIndex = -1; + let loadedAccountKey = ''; const decisionKeys = new Map(); + let decisionFlight = null; const escape = value => String(value ?? '').replace(/[&<>"']/g, character => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''', @@ -78,6 +80,14 @@ function createHumanGates(options = {}) { } async function load() { + const accountKey = String(getAccountKey() || '').trim().toLowerCase(); + if (accountKey !== loadedAccountKey) { + loadedAccountKey = accountKey; + queue = { pending_count: 0, items: [] }; + reviewSnapshot = []; + reviewIndex = -1; + render(); + } const cached = restore(); if (cached) { queue = cached; render(); } try { @@ -149,21 +159,30 @@ function createHumanGates(options = {}) { reason: String(values.reason || '').trim(), override_reason: String(values.override_reason || '').trim(), checklist, }; - const decisionKey = idempotencyKey(item, decision, payload); - const receipt = await fetchJson('api/v1/human-gates/' + encodeURIComponent(item.id) + '/decision', { - method: 'POST', - headers: { 'Content-Type': 'application/json', 'Idempotency-Key': decisionKey.key }, - body: JSON.stringify(payload), - }); - decisionKeys.delete(decisionKey.operation); - queue.items = queue.items.filter(candidate => candidate.id !== item.id); - queue.pending_count = Math.max(0, queue.pending_count - 1); - save(queue); render(); - reviewIndex += 1; - const next = current(); - if (next) reviewNext(); else renderDetail(null); - setText(nodes.status, next ? (decision === 'release' ? 'Released. Reviewing next gate.' : 'Held. Reviewing next gate.') : 'Decision saved. Human Gates review snapshot complete.'); - return { receipt, next }; + if (decisionFlight) return decisionFlight; + const operation = (async () => { + const decisionKey = idempotencyKey(item, decision, payload); + const receipt = await fetchJson('api/v1/human-gates/' + encodeURIComponent(item.id) + '/decision', { + method: 'POST', + headers: { 'Content-Type': 'application/json', 'Idempotency-Key': decisionKey.key }, + body: JSON.stringify(payload), + }); + decisionKeys.delete(decisionKey.operation); + queue.items = queue.items.filter(candidate => candidate.id !== item.id); + queue.pending_count = Math.max(0, queue.pending_count - 1); + save(queue); render(); + reviewIndex += 1; + const next = current(); + if (next) reviewNext(); else renderDetail(null); + setText(nodes.status, next ? (decision === 'release' ? 'Released. Reviewing next gate.' : 'Held. Reviewing next gate.') : 'Decision saved. Human Gates review snapshot complete.'); + return { receipt, next }; + })(); + decisionFlight = operation; + try { + return await operation; + } finally { + if (decisionFlight === operation) decisionFlight = null; + } } function open() { diff --git a/src/human_gate_store.py b/src/human_gate_store.py index e07bd2d..6932284 100644 --- a/src/human_gate_store.py +++ b/src/human_gate_store.py @@ -218,15 +218,20 @@ class HumanGateStore: if immutable_old != immutable_new: raise GateConflict("Candidate hash is already bound to different facts") if old_payload != payload: + if existing["state"] == "superseded": + raise GateConflict("Candidate hash was superseded by a newer candidate") now = float(self.clock()) revision = existing["revision"] + 1 + reopened = existing["state"] in {"released", "held"} + state = "pending" if reopened else existing["state"] + action = "reopened" if reopened else "updated" connection.execute( - "UPDATE human_gates SET payload_json=?, revision=?, updated_at=? WHERE id=?", - (_canonical(payload), revision, now, existing["id"]), + "UPDATE human_gates SET payload_json=?, state=?, revision=?, updated_at=?, decision_reason='', override_reason='', checklist_json='{}' WHERE id=?", + (_canonical(payload), state, revision, now, existing["id"]), ) connection.execute( "INSERT INTO human_gate_history(gate_id,action,at,details_json) VALUES (?,?,?,?)", - (existing["id"], "updated", now, _canonical({"candidate_hash": payload["candidate_hash"]})), + (existing["id"], action, now, _canonical({"candidate_hash": payload["candidate_hash"]})), ) existing = connection.execute( "SELECT * FROM human_gates WHERE id=? AND login=?", diff --git a/tests/test_human_gate_store.py b/tests/test_human_gate_store.py index d71f4e9..1df8c55 100644 --- a/tests/test_human_gate_store.py +++ b/tests/test_human_gate_store.py @@ -115,6 +115,25 @@ def test_same_hash_update_coalesces_one_card_and_preserves_history(tmp_path): assert store.list("timmy")["pending_count"] == 1 +def test_updated_checks_reopen_a_released_hash_for_review(tmp_path): + store = HumanGateStore(tmp_path / "gates.sqlite3", clock=iter([100, 101, 102]).__next__) + gate = store.intake("timmy", candidate(), idempotency_key="run-1") + store.decide( + "timmy", gate["id"], expected_revision=1, decision="release", + reason="", override_reason="", checklist=checklist(), idempotency_key="decision-1", + ) + + reopened = store.intake( + "timmy", candidate(check_state="failure"), idempotency_key="run-2", + ) + + assert reopened["state"] == "pending" + assert reopened["revision"] == 3 + assert reopened["checks"][0]["state"] == "failure" + assert reopened["history"][-1]["action"] == "reopened" + assert store.list("timmy")["pending_count"] == 1 + + def test_same_hash_identity_facts_cannot_be_redefined(tmp_path): store = HumanGateStore(tmp_path / "gates.sqlite3", clock=lambda: 100) store.intake("timmy", candidate(), idempotency_key="run-1") diff --git a/tests/test_human_gates_frontend.py b/tests/test_human_gates_frontend.py index f075836..df5a91e 100644 --- a/tests/test_human_gates_frontend.py +++ b/tests/test_human_gates_frontend.py @@ -82,6 +82,18 @@ const gates=createHumanGates({storage,getLogin:()=> 'timmy',getAccountKey:()=>ac assert output["restored"] is None +def test_account_switch_clears_in_memory_gate_data_before_failed_load(): + output = run_node(r""" +const values=new Map(); const storage={getItem:k=>values.get(k)||null,setItem:(k,v)=>values.set(k,v)}; +let account='1:timmy', fail=false; const nodes={count:{},list:{innerHTML:''},status:{},panel:{}}; +const gates=createHumanGates({storage,getLogin:()=> 'timmy',getAccountKey:()=>account,isOnline:()=>true,nodes,location:{hash:''},fetchJson:async()=>{if(fail)throw new Error('offline');return {pending_count:1,items:[{id:'private-1',title:'Principal 1 private',candidate_hash:'secret'}]}}}); +(async()=>{await gates.load();account='2:timmy';fail=true;try{await gates.load()}catch(_){}process.stdout.write(JSON.stringify({snapshot:gates.snapshot(),html:nodes.list.innerHTML}));})(); +""") + assert output["snapshot"] == {"pending_count": 0, "items": []} + assert "Principal 1 private" not in output["html"] + assert "secret" not in output["html"] + + def test_decision_retry_reuses_the_same_idempotency_key(): output = run_node(r""" let attempts=0; const keys=[]; @@ -93,6 +105,19 @@ const gates=createHumanGates({storage:{getItem:()=>null,setItem(){}},getLogin:() assert output["keys"][0] == output["keys"][1] +def test_concurrent_decision_taps_submit_once_and_advance_once(): + output = run_node(r""" +let posts=0, releasePost; const posted=new Promise(resolve=>releasePost=resolve); +const items=[{id:'g1',title:'One',candidate_hash:'a1',revision:1,checks:[]},{id:'g2',title:'Two',candidate_hash:'b2',revision:1,checks:[]}]; +const gates=createHumanGates({storage:{getItem:()=>null,setItem(){}},getLogin:()=> 'timmy',isOnline:()=>true,nodes:{count:{},list:{},status:{},panel:{},detail:{}},location:{hash:''},fetchJson:async(path,options={})=>{if(options.method==='POST'){posts++;await posted;return {receipt_id:'r1'}};return {pending_count:2,items};}}); +(async()=>{await gates.load();gates.reviewNext();const values={reason:'wait',checklist:{}};const first=gates.decideAndNext('hold',values);const second=gates.decideAndNext('hold',values);releasePost();await Promise.all([first,second]);process.stdout.write(JSON.stringify({posts,current:gates.current()?.id,snapshot:gates.snapshot()}));})(); +""") + assert output["posts"] == 1 + assert output["current"] == "g2" + assert output["snapshot"]["pending_count"] == 1 + assert [item["id"] for item in output["snapshot"]["items"]] == ["g2"] + + def test_selecting_a_queue_card_opens_that_exact_gate(): output = run_node(r""" const details={ @@ -117,3 +142,4 @@ def test_human_gate_mobile_shell_and_deep_route_are_wired(): assert 'static/human-gates.js' in index assert "#/my-work/human-gates" in dashboard assert "createHumanGates" in dashboard + assert "planningOwnerAccountKey = confirmedOwnerLogin && saved.user?.id" in dashboard