fix: close Human Gates review races
This commit is contained in:
parent
dbb8ff4d4e
commit
9d84d5c5c4
|
|
@ -7886,6 +7886,8 @@
|
||||||
confirmedOwnerLogin = String(saved.user?.login || '').trim();
|
confirmedOwnerLogin = String(saved.user?.login || '').trim();
|
||||||
restoreReleaseReceipt();
|
restoreReleaseReceipt();
|
||||||
planningOwnerLogin = confirmedOwnerLogin;
|
planningOwnerLogin = confirmedOwnerLogin;
|
||||||
|
planningOwnerAccountKey = confirmedOwnerLogin && saved.user?.id ?
|
||||||
|
String(saved.user.id) + ':' + confirmedOwnerLogin : '';
|
||||||
interruptionPrompt.restore();
|
interruptionPrompt.restore();
|
||||||
updatePlanningAvailability();
|
updatePlanningAvailability();
|
||||||
syncPendingTomorrow();
|
syncPendingTomorrow();
|
||||||
|
|
|
||||||
|
|
@ -9,7 +9,9 @@ function createHumanGates(options = {}) {
|
||||||
let queue = { pending_count: 0, items: [] };
|
let queue = { pending_count: 0, items: [] };
|
||||||
let reviewSnapshot = [];
|
let reviewSnapshot = [];
|
||||||
let reviewIndex = -1;
|
let reviewIndex = -1;
|
||||||
|
let loadedAccountKey = '';
|
||||||
const decisionKeys = new Map();
|
const decisionKeys = new Map();
|
||||||
|
let decisionFlight = null;
|
||||||
|
|
||||||
const escape = value => String(value ?? '').replace(/[&<>"']/g, character => ({
|
const escape = value => String(value ?? '').replace(/[&<>"']/g, character => ({
|
||||||
'&': '&', '<': '<', '>': '>', '"': '"', "'": ''',
|
'&': '&', '<': '<', '>': '>', '"': '"', "'": ''',
|
||||||
|
|
@ -78,6 +80,14 @@ function createHumanGates(options = {}) {
|
||||||
}
|
}
|
||||||
|
|
||||||
async function load() {
|
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();
|
const cached = restore();
|
||||||
if (cached) { queue = cached; render(); }
|
if (cached) { queue = cached; render(); }
|
||||||
try {
|
try {
|
||||||
|
|
@ -149,21 +159,30 @@ function createHumanGates(options = {}) {
|
||||||
reason: String(values.reason || '').trim(),
|
reason: String(values.reason || '').trim(),
|
||||||
override_reason: String(values.override_reason || '').trim(), checklist,
|
override_reason: String(values.override_reason || '').trim(), checklist,
|
||||||
};
|
};
|
||||||
const decisionKey = idempotencyKey(item, decision, payload);
|
if (decisionFlight) return decisionFlight;
|
||||||
const receipt = await fetchJson('api/v1/human-gates/' + encodeURIComponent(item.id) + '/decision', {
|
const operation = (async () => {
|
||||||
method: 'POST',
|
const decisionKey = idempotencyKey(item, decision, payload);
|
||||||
headers: { 'Content-Type': 'application/json', 'Idempotency-Key': decisionKey.key },
|
const receipt = await fetchJson('api/v1/human-gates/' + encodeURIComponent(item.id) + '/decision', {
|
||||||
body: JSON.stringify(payload),
|
method: 'POST',
|
||||||
});
|
headers: { 'Content-Type': 'application/json', 'Idempotency-Key': decisionKey.key },
|
||||||
decisionKeys.delete(decisionKey.operation);
|
body: JSON.stringify(payload),
|
||||||
queue.items = queue.items.filter(candidate => candidate.id !== item.id);
|
});
|
||||||
queue.pending_count = Math.max(0, queue.pending_count - 1);
|
decisionKeys.delete(decisionKey.operation);
|
||||||
save(queue); render();
|
queue.items = queue.items.filter(candidate => candidate.id !== item.id);
|
||||||
reviewIndex += 1;
|
queue.pending_count = Math.max(0, queue.pending_count - 1);
|
||||||
const next = current();
|
save(queue); render();
|
||||||
if (next) reviewNext(); else renderDetail(null);
|
reviewIndex += 1;
|
||||||
setText(nodes.status, next ? (decision === 'release' ? 'Released. Reviewing next gate.' : 'Held. Reviewing next gate.') : 'Decision saved. Human Gates review snapshot complete.');
|
const next = current();
|
||||||
return { receipt, next };
|
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() {
|
function open() {
|
||||||
|
|
|
||||||
|
|
@ -218,15 +218,20 @@ class HumanGateStore:
|
||||||
if immutable_old != immutable_new:
|
if immutable_old != immutable_new:
|
||||||
raise GateConflict("Candidate hash is already bound to different facts")
|
raise GateConflict("Candidate hash is already bound to different facts")
|
||||||
if old_payload != payload:
|
if old_payload != payload:
|
||||||
|
if existing["state"] == "superseded":
|
||||||
|
raise GateConflict("Candidate hash was superseded by a newer candidate")
|
||||||
now = float(self.clock())
|
now = float(self.clock())
|
||||||
revision = existing["revision"] + 1
|
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(
|
connection.execute(
|
||||||
"UPDATE human_gates SET payload_json=?, revision=?, updated_at=? WHERE id=?",
|
"UPDATE human_gates SET payload_json=?, state=?, revision=?, updated_at=?, decision_reason='', override_reason='', checklist_json='{}' WHERE id=?",
|
||||||
(_canonical(payload), revision, now, existing["id"]),
|
(_canonical(payload), state, revision, now, existing["id"]),
|
||||||
)
|
)
|
||||||
connection.execute(
|
connection.execute(
|
||||||
"INSERT INTO human_gate_history(gate_id,action,at,details_json) VALUES (?,?,?,?)",
|
"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(
|
existing = connection.execute(
|
||||||
"SELECT * FROM human_gates WHERE id=? AND login=?",
|
"SELECT * FROM human_gates WHERE id=? AND login=?",
|
||||||
|
|
|
||||||
|
|
@ -115,6 +115,25 @@ def test_same_hash_update_coalesces_one_card_and_preserves_history(tmp_path):
|
||||||
assert store.list("timmy")["pending_count"] == 1
|
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):
|
def test_same_hash_identity_facts_cannot_be_redefined(tmp_path):
|
||||||
store = HumanGateStore(tmp_path / "gates.sqlite3", clock=lambda: 100)
|
store = HumanGateStore(tmp_path / "gates.sqlite3", clock=lambda: 100)
|
||||||
store.intake("timmy", candidate(), idempotency_key="run-1")
|
store.intake("timmy", candidate(), idempotency_key="run-1")
|
||||||
|
|
|
||||||
|
|
@ -82,6 +82,18 @@ const gates=createHumanGates({storage,getLogin:()=> 'timmy',getAccountKey:()=>ac
|
||||||
assert output["restored"] is None
|
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():
|
def test_decision_retry_reuses_the_same_idempotency_key():
|
||||||
output = run_node(r"""
|
output = run_node(r"""
|
||||||
let attempts=0; const keys=[];
|
let attempts=0; const keys=[];
|
||||||
|
|
@ -93,6 +105,19 @@ const gates=createHumanGates({storage:{getItem:()=>null,setItem(){}},getLogin:()
|
||||||
assert output["keys"][0] == output["keys"][1]
|
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():
|
def test_selecting_a_queue_card_opens_that_exact_gate():
|
||||||
output = run_node(r"""
|
output = run_node(r"""
|
||||||
const details={
|
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 'static/human-gates.js' in index
|
||||||
assert "#/my-work/human-gates" in dashboard
|
assert "#/my-work/human-gates" in dashboard
|
||||||
assert "createHumanGates" in dashboard
|
assert "createHumanGates" in dashboard
|
||||||
|
assert "planningOwnerAccountKey = confirmedOwnerLogin && saved.user?.id" in dashboard
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user