fix: harden Human Gates review flow
This commit is contained in:
parent
364fc60116
commit
dbb8ff4d4e
|
|
@ -392,6 +392,7 @@
|
|||
let editingOutboxId = null;
|
||||
let confirmedOwnerLogin = '';
|
||||
let planningOwnerLogin = '';
|
||||
let planningOwnerAccountKey = '';
|
||||
let activeFlushLogin = '';
|
||||
let rR = null;
|
||||
function rRC() {
|
||||
|
|
@ -637,6 +638,7 @@
|
|||
const humanGates = createHumanGates({
|
||||
storage:localStorage,
|
||||
getLogin:()=>planningOwnerLogin,
|
||||
getAccountKey:()=>planningOwnerAccountKey,
|
||||
isOnline:()=>navigator.onLine,
|
||||
location:window.location,
|
||||
fetchJson:fetchReviewJson,
|
||||
|
|
@ -655,8 +657,9 @@
|
|||
if (window.location.hash === '#/my-work/human-gates') window.history.replaceState({}, '', '#/my-work');
|
||||
});
|
||||
qs('#human-gates-list').addEventListener('click', event => {
|
||||
if (!event.target.closest('[data-human-gate-id]')) return;
|
||||
humanGates.reviewNext();
|
||||
const card = event.target.closest('[data-human-gate-id]');
|
||||
if (!card) return;
|
||||
humanGates.select(card.dataset.humanGateId);
|
||||
});
|
||||
qs('#human-gate-detail').addEventListener('click', event => {
|
||||
const decision = event.target.closest('[data-gate-decision]')?.dataset.gateDecision;
|
||||
|
|
@ -5564,6 +5567,8 @@
|
|||
const retainedPlanningLogin = !snapshot.context.error ?
|
||||
String(snapshot.context.user?.login || '').trim() : '';
|
||||
planningOwnerLogin = retainedPlanningLogin;
|
||||
planningOwnerAccountKey = retainedPlanningLogin && snapshot.context.user?.id ?
|
||||
String(snapshot.context.user.id) + ':' + retainedPlanningLogin : '';
|
||||
updatePlanningAvailability();
|
||||
if (planningOwnerLogin) {
|
||||
syncPendingTomorrow();
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
function createHumanGates(options = {}) {
|
||||
const storage = options.storage || window.localStorage;
|
||||
const getLogin = options.getLogin || (() => '');
|
||||
const getAccountKey = options.getAccountKey || getLogin;
|
||||
const isOnline = options.isOnline || (() => navigator.onLine);
|
||||
const location = options.location || window.location;
|
||||
const fetchJson = options.fetchJson;
|
||||
|
|
@ -8,11 +9,12 @@ function createHumanGates(options = {}) {
|
|||
let queue = { pending_count: 0, items: [] };
|
||||
let reviewSnapshot = [];
|
||||
let reviewIndex = -1;
|
||||
const decisionKeys = new Map();
|
||||
|
||||
const escape = value => String(value ?? '').replace(/[&<>"']/g, character => ({
|
||||
'&': '&', '<': '<', '>': '>', '"': '"', "'": ''',
|
||||
})[character]);
|
||||
const cacheKey = () => 'stackchain.human-gates.v1:' + String(getLogin() || '').trim().toLowerCase();
|
||||
const cacheKey = () => 'stackchain.human-gates.v1:' + String(getAccountKey() || '').trim().toLowerCase();
|
||||
const setText = (node, value) => { if (node) node.textContent = value; };
|
||||
const setHtml = (node, value) => { if (node) node.innerHTML = value; };
|
||||
|
||||
|
|
@ -111,10 +113,22 @@ function createHumanGates(options = {}) {
|
|||
return item;
|
||||
}
|
||||
|
||||
function select(gateId) {
|
||||
if (reviewIndex < 0) reviewSnapshot = queue.items.slice();
|
||||
const index = reviewSnapshot.findIndex(item => item.id === gateId);
|
||||
if (index < 0) throw new Error('Gate is not in the current review snapshot.');
|
||||
reviewIndex = index;
|
||||
return reviewNext();
|
||||
}
|
||||
|
||||
function current() { return reviewIndex < 0 ? null : (reviewSnapshot[reviewIndex] || null); }
|
||||
function idempotencyKey(item, decision) {
|
||||
function idempotencyKey(item, decision, payload) {
|
||||
const operation = item.id + ':' + item.revision + ':' + decision + ':' + JSON.stringify(payload);
|
||||
if (decisionKeys.has(operation)) return { operation, key: decisionKeys.get(operation) };
|
||||
const nonce = globalThis.crypto?.randomUUID?.() || (Date.now().toString(36) + '-' + Math.random().toString(36).slice(2));
|
||||
return 'human-gate:' + item.id + ':' + item.revision + ':' + decision + ':' + nonce;
|
||||
const key = 'human-gate:' + item.id + ':' + item.revision + ':' + decision + ':' + nonce;
|
||||
decisionKeys.set(operation, key);
|
||||
return { operation, key };
|
||||
}
|
||||
|
||||
async function decideAndNext(decision, values = {}) {
|
||||
|
|
@ -135,11 +149,13 @@ 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': idempotencyKey(item, decision) },
|
||||
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();
|
||||
|
|
@ -157,7 +173,8 @@ function createHumanGates(options = {}) {
|
|||
}
|
||||
|
||||
return {
|
||||
load, open, reviewNext, decideAndNext, current,
|
||||
load, open, reviewNext, select, decideAndNext, current,
|
||||
restoreCached: restore,
|
||||
snapshot: () => JSON.parse(JSON.stringify(queue)),
|
||||
route: () => location.hash,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -187,7 +187,7 @@
|
|||
<button class="end-today-session" id="end-today-session" type="button" hidden>End session</button>
|
||||
<button class="find-work-action" id="find-work" type="button">Find work</button>
|
||||
<button class="new-issue" id="new-issue" type="button">New issue</button>
|
||||
<button class="human-gates-launcher" id="open-human-gates" type="button">Human Gates <span id="human-gates-count">0</span></button>
|
||||
<button class="human-gates-launcher" id="open-human-gates" type="button">Review next <span id="human-gates-count">0</span></button>
|
||||
</div>
|
||||
<section id="human-gates" class="human-gates" aria-labelledby="human-gates-heading" hidden>
|
||||
<div class="human-gates-header">
|
||||
|
|
|
|||
|
|
@ -212,8 +212,26 @@ class HumanGateStore:
|
|||
(login, payload["source"], payload["project"], payload["candidate_hash"]),
|
||||
).fetchone()
|
||||
if existing:
|
||||
if _fingerprint(json.loads(existing["payload_json"])) != fingerprint:
|
||||
old_payload = json.loads(existing["payload_json"])
|
||||
immutable_old = {key: value for key, value in old_payload.items() if key != "checks"}
|
||||
immutable_new = {key: value for key, value in payload.items() if key != "checks"}
|
||||
if immutable_old != immutable_new:
|
||||
raise GateConflict("Candidate hash is already bound to different facts")
|
||||
if old_payload != payload:
|
||||
now = float(self.clock())
|
||||
revision = existing["revision"] + 1
|
||||
connection.execute(
|
||||
"UPDATE human_gates SET payload_json=?, revision=?, updated_at=? WHERE id=?",
|
||||
(_canonical(payload), 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 = connection.execute(
|
||||
"SELECT * FROM human_gates WHERE id=? AND login=?",
|
||||
(existing["id"], login),
|
||||
).fetchone()
|
||||
connection.execute("INSERT INTO human_gate_intake_keys VALUES (?,?,?,?)", (login, key, fingerprint, existing["id"]))
|
||||
return self._present(connection, existing, history=True)
|
||||
now, gate_id = float(self.clock()), str(uuid.uuid4())
|
||||
|
|
|
|||
|
|
@ -1818,10 +1818,10 @@ def health() -> dict[str, str]:
|
|||
|
||||
async def _human_gate_login(request: Request) -> str:
|
||||
session = getattr(request.state, "dashboard_session", None)
|
||||
if session is not None and session.principal_login:
|
||||
return session.principal_login
|
||||
_principal_id, login = await _upstream_identity()
|
||||
return login
|
||||
if session is not None and session.principal_login and session.principal_id:
|
||||
return f"{session.principal_id}:{session.principal_login}"
|
||||
principal_id, login = await _upstream_identity()
|
||||
return f"{principal_id}:{login}"
|
||||
|
||||
|
||||
def _gate_error(error: Exception) -> HTTPException:
|
||||
|
|
|
|||
|
|
@ -52,7 +52,7 @@ async def test_intake_list_and_detail_are_account_bound_and_no_store(gate_api):
|
|||
|
||||
@pytest.mark.anyio
|
||||
async def test_decision_requires_revision_and_returns_durable_receipt(gate_api):
|
||||
gate = gate_api.intake("timmy", CANDIDATE, idempotency_key="run-9")
|
||||
gate = gate_api.intake("1:timmy", CANDIDATE, idempotency_key="run-9")
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
payload = {"expected_revision": gate["revision"], "decision": "release", "reason": "", "override_reason": "", "checklist": CHECKLIST}
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
|
|
@ -68,6 +68,29 @@ async def test_decision_requires_revision_and_returns_durable_receipt(gate_api):
|
|||
assert all(response.headers["cache-control"] == "no-store" for response in (decided, repeated, receipt, stale))
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_recycled_login_cannot_read_another_principal_gates(monkeypatch, tmp_path):
|
||||
store = HumanGateStore(tmp_path / "gates.sqlite3", clock=lambda: 100)
|
||||
monkeypatch.setattr(main, "_human_gate_store", lambda: store, raising=False)
|
||||
principal = {"id": 1, "login": "timmy"}
|
||||
|
||||
async def identity():
|
||||
return principal.copy()
|
||||
|
||||
monkeypatch.setattr(main, "current_user", identity)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
created = await client.post(
|
||||
"/api/v1/human-gates/intake", json=CANDIDATE,
|
||||
headers={"Idempotency-Key": "principal-1"},
|
||||
)
|
||||
principal["id"] = 2
|
||||
listing = await client.get("/api/v1/human-gates")
|
||||
|
||||
assert created.status_code == 201
|
||||
assert listing.json()["pending_count"] == 0
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_gate_store_failure_is_sanitized_no_store(monkeypatch):
|
||||
def unavailable():
|
||||
|
|
@ -91,7 +114,7 @@ async def test_gate_store_failure_is_sanitized_no_store(monkeypatch):
|
|||
@pytest.mark.anyio
|
||||
async def test_gate_mutations_require_idempotency_key_and_validate_override(gate_api):
|
||||
failing = {**CANDIDATE, "candidate_hash": "fail123", "checks": [{"name": "browser", "state": "failure", "required": True}]}
|
||||
gate = gate_api.intake("timmy", failing, idempotency_key="run-fail")
|
||||
gate = gate_api.intake("1:timmy", failing, idempotency_key="run-fail")
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
missing_key = await client.post("/api/v1/human-gates/intake", json=CANDIDATE)
|
||||
|
|
|
|||
|
|
@ -102,7 +102,20 @@ def test_hold_requires_reason_and_release_requires_checklist_and_override_for_un
|
|||
assert receipt["checklist"] == {}
|
||||
|
||||
|
||||
def test_same_hash_cannot_be_redefined_by_a_new_idempotency_key(tmp_path):
|
||||
def test_same_hash_update_coalesces_one_card_and_preserves_history(tmp_path):
|
||||
store = HumanGateStore(tmp_path / "gates.sqlite3", clock=iter([100, 101]).__next__)
|
||||
original = store.intake("timmy", candidate(check_state="pending"), idempotency_key="run-1")
|
||||
|
||||
updated = store.intake("timmy", candidate(check_state="success"), idempotency_key="run-2")
|
||||
|
||||
assert updated["id"] == original["id"]
|
||||
assert updated["revision"] == 2
|
||||
assert updated["checks"][0]["state"] == "success"
|
||||
assert updated["history"][-1]["action"] == "updated"
|
||||
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")
|
||||
|
||||
|
|
|
|||
|
|
@ -71,11 +71,49 @@ const gates=createHumanGates({storage:{getItem:()=>null,setItem(){}},getLogin:()
|
|||
assert output["posts"] == 0
|
||||
|
||||
|
||||
def test_offline_cache_is_scoped_to_immutable_account_identity():
|
||||
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'; const nodes={count:{},list:{},status:{},panel:{}};
|
||||
const gates=createHumanGates({storage,getLogin:()=> 'timmy',getAccountKey:()=>account,isOnline:()=>true,nodes,location:{hash:''},fetchJson:async()=>({pending_count:1,items:[{id:'g1',title:'Private',candidate_hash:'a1'}]})});
|
||||
(async()=>{await gates.load();account='2:timmy';process.stdout.write(JSON.stringify({keys:[...values.keys()],restored:gates.restoreCached()}));})();
|
||||
""")
|
||||
assert output["keys"] == ["stackchain.human-gates.v1:1:timmy"]
|
||||
assert output["restored"] is None
|
||||
|
||||
|
||||
def test_decision_retry_reuses_the_same_idempotency_key():
|
||||
output = run_node(r"""
|
||||
let attempts=0; const keys=[];
|
||||
const item={id:'g1',title:'Candidate',candidate_hash:'a1',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'){keys.push(options.headers['Idempotency-Key']);attempts++;if(attempts===1)throw new Error('network');return {receipt_id:'r1'}};return {pending_count:1,items:[item]};}});
|
||||
(async()=>{await gates.load();gates.reviewNext();const values={reason:'wait',checklist:{}};try{await gates.decideAndNext('hold',values)}catch(_){}await gates.decideAndNext('hold',values);process.stdout.write(JSON.stringify({keys}));})();
|
||||
""")
|
||||
assert len(output["keys"]) == 2
|
||||
assert output["keys"][0] == output["keys"][1]
|
||||
|
||||
|
||||
def test_selecting_a_queue_card_opens_that_exact_gate():
|
||||
output = run_node(r"""
|
||||
const details={
|
||||
g1:{id:'g1',title:'First',project:'p/one',candidate_hash:'a1',revision:1,checks:[]},
|
||||
g2:{id:'g2',title:'Second',project:'p/two',candidate_hash:'b2',revision:1,checks:[]},
|
||||
};
|
||||
const nodes={count:{},list:{},status:{},panel:{},detail:{innerHTML:''}};
|
||||
const gates=createHumanGates({storage:{getItem:()=>null,setItem(){}},getLogin:()=> 'timmy',isOnline:()=>true,nodes,location:{hash:''},fetchJson:async path=>path.includes('/g')?details[path.split('/').pop()]:{pending_count:2,items:Object.values(details)}});
|
||||
(async()=>{await gates.load();gates.reviewNext();gates.select('g2');await new Promise(resolve=>setTimeout(resolve,0));process.stdout.write(JSON.stringify({current:gates.current().id,html:nodes.detail.innerHTML}));})();
|
||||
""")
|
||||
assert output["current"] == "g2"
|
||||
assert "Second" in output["html"]
|
||||
assert "p/two" in output["html"]
|
||||
|
||||
|
||||
def test_human_gate_mobile_shell_and_deep_route_are_wired():
|
||||
index = INDEX.read_text()
|
||||
dashboard = DASHBOARD.read_text()
|
||||
assert 'id="human-gates"' in index
|
||||
assert 'id="human-gates-count"' in index
|
||||
assert 'Review next <span id="human-gates-count"' in index
|
||||
assert 'static/human-gates.js' in index
|
||||
assert "#/my-work/human-gates" in dashboard
|
||||
assert "createHumanGates" in dashboard
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user