diff --git a/packages/webui/src/decision-http.mjs b/packages/webui/src/decision-http.mjs index 1e016e2c0..e64d16e1e 100644 --- a/packages/webui/src/decision-http.mjs +++ b/packages/webui/src/decision-http.mjs @@ -48,14 +48,16 @@ const BUS_CODES = { // the decision, then #hireConsole checks it, consumes it, and applies it. apply() runs check() again // and the writers, authorityHost and #fireApproved run after the consume, so most codes can come from // either side of it; for a fire the agent's runs may already be stopped. The code alone does not say -// which, so those say how to tell: a consumed decision shows as answered in the inbox. Only codes +// which, and the inbox cannot tell either: it lists no closed decision (broker inbox verb), so a +// consumed one simply drops out of it. Those codes therefore say part of the change may already be +// applied and send the user to the Agents page (re-check finding 1). Only codes // that prepare() alone raises say "Nothing was applied.": the route checks (checkRoutes runs in // prepare(), and in adjusted() only for an adjust, which a start never carries), definition-shared, // definition-not-hireable and no-chief-of-staff. organization-uncertain, a lost reply and the // bus server's own request codes (request-timeout and invalid-envelope can fire while the request is // processing) stay outcome-unknown. const NONE = ' Nothing was applied.'; -const EITHER = ' If the inbox shows this proposal as answered, part of it may already be applied: check the Agents page before trying again. Otherwise nothing was applied.'; +const EITHER = ' Part of it may already be applied: check the Agents page before trying again.'; const CHECK = ' Check the Agents page and the inbox before trying again.'; const HIRE_START_REFUSALS = { 'model-provider-unavailable': [409, 'The model proxy does not offer this model, or its catalogue is not ready. Pick a model it offers, or ask the owner to check Settings > Model proxy.' + NONE], diff --git a/packages/webui/src/public/autonomy.js b/packages/webui/src/public/autonomy.js index 2137b8566..13a65827f 100644 --- a/packages/webui/src/public/autonomy.js +++ b/packages/webui/src/public/autonomy.js @@ -23,22 +23,24 @@ const value=await r.json();if(!r.ok)throw Error(guidance[value.error]??`The bus refused: ${value.error??'outcome unknown'}. Refresh before retrying.`);return value; } // E2E6 P1-1: a poll reloads this state; the last save's receipt and the open review list stay. - // A failed poll keeps the last good read (review P3), so a dirty form the page holds is not dropped. - async function load(org) {const prev=states.get(org),keep={status:prev?.status??'',reviewOpen:prev?.reviewOpen===true};try{states.set(org,{...await call(org,''),...keep});}catch(error){states.set(org,prev&&!prev.error?{...prev,...keep}:{error:error.message,...keep});}} + // A failed poll keeps the last good read only while the page holds the form (dirty, or a field has + // focus), so the held form is not dropped (review P3). A clean form shows the read error (re-check P3). + const held=org=>{const form=[...document.querySelectorAll('[data-autonomy-org]')].find(n=>n.dataset.autonomyOrg===org)?.querySelector('[data-autonomy-form]');return !!form&&(form.hasAttribute('data-live-dirty')||form.contains(document.activeElement));}; + async function load(org) {const prev=states.get(org),keep={status:prev?.status??'',reviewOpen:prev?.reviewOpen===true};try{states.set(org,{...await call(org,''),...keep});}catch(error){states.set(org,prev&&!prev.error&&held(org)?{...prev,...keep}:{error:error.message,...keep});}} function section(org) { const s=states.get(org); if(!s || s.error)return `

Autonomy

${esc(s?.error??'Autonomy could not be read.')}

`; - const v=s.autonomy??{}; + const v=s.autonomy??{},fk=k=>` data-focus-key="${esc(`autonomy-${k}:${org}`)}"`; // The form carries the revision it was drawn from: a form a poll kept (workspace holdLive) saves // against that revision, so it cannot overwrite a change it never showed. return `

Autonomy

Choose how closely you follow each class of decision. Hard waits, flagged grants and credentials always need a person.

${classes.map(k=>``).join('')}
- +

In the loop, the arbiter resolves and you confirm before work takes effect. Near the loop, work takes effect and you can reverse it within the window. Out of the loop, the trail records it without a notice.

- ${s.writable?'

':'

Only the owner can change autonomy.

'} + ${s.writable?`

`:'

Only the owner can change autonomy.

'}

${esc(s.status)}

- ${s.writable?`
Review reversals before moving out of the loop

Read the decision trail and its reversals before recording your review.

${classes.map(k=>``).join('')}
`:''}
`; + ${s.writable?`
Review reversals before moving out of the loop

Read the decision trail and its reversals before recording your review.

${classes.map(k=>``).join('')}
`:''}`; } function notice(d) { if(!d?.autonomy)return ''; diff --git a/packages/webui/tests/decision-http.test.mjs b/packages/webui/tests/decision-http.test.mjs index bfd10db10..64aa4c6b4 100644 --- a/packages/webui/tests/decision-http.test.mjs +++ b/packages/webui/tests/decision-http.test.mjs @@ -335,7 +335,7 @@ test('E2E6 hire start: an unknown project, an unknown agent on fire and an unkno assert.equal(r.json().error,code);assert.match(r.json().message,re); // harness-off comes only from prepare()'s route check; the other two can also come from apply(). if(code==='harness-off')assert.match(r.json().message,/ Nothing was applied\.$/); - else assert.match(r.json().message,/inbox shows this proposal as answered.*Otherwise nothing was applied\.$/); + else {assert.match(r.json().message,/ Part of it may already be applied: check the Agents page before trying again\.$/);assert.doesNotMatch(r.json().message,/nothing was applied/i);} assert.match(s.lines.at(-1),new RegExp(`step=start error=${code}$`)); } }); @@ -345,12 +345,14 @@ test('E2E6 hire start: the other proposal and apply codes keep their code; apply assert.equal(r.json().error,code,`${code}: ${r.text}`);assert.match(r.json().message,/ Nothing was applied\.$/); } // Review finding 1: apply() runs check() again and the writers run after the consume, so these - // never claim nothing changed outright; they say how to tell from the inbox. + // never claim nothing changed, in any case or clause. The inbox lists no closed decision, so they + // do not send the user there to tell which side of the consume the refusal came from (re-check). for(const code of ['organization-instance-exists','instance-kept','root-fire-shared','definition-rev-conflict','definition-missing','credential-leak','decision-stale','organization-instance-retired','organization-instance-referenced','organization-refused','definition-secret-check-required','not-configured','invalid-request']) { const {r}=await startWith(new BusReadError(code)); assert.equal(r.json().error,code,`${code}: ${r.text}`); - assert.doesNotMatch(r.json().message,/(^|\. )Nothing was applied/,`${code} does not claim nothing changed`); - assert.match(r.json().message,/If the inbox shows this proposal as answered, part of it may already be applied: check the Agents page/); + assert.doesNotMatch(r.json().message,/nothing was applied/i,`${code} does not claim nothing changed`); + assert.doesNotMatch(r.json().message,/inbox/,`${code} does not point at the inbox, which drops a consumed decision`); + assert.match(r.json().message,/Part of it may already be applied: check the Agents page/); } for(const code of ['organization-busy','fire-in-progress','credential-revoking']) { const {r}=await startWith(new BusReadError(code)); @@ -365,8 +367,8 @@ test('E2E6 hire start: an apply-phase decision-stale on a fire does not say noth const s=setup({resolve:()=>{throw new BusReadError('decision-stale');}}); const r=await s.run('/api/decisions/hire/start',request({body:{business:'demo',hire:{op:'fire',level:'project',project:'site',name:'coder',reason:'done'}}})); assert.equal(s.calls.length,1);assert.equal(r.status,409,r.text);assert.equal(r.json().error,'decision-stale'); - assert.doesNotMatch(r.json().message,/(^|\. )Nothing was applied/); - assert.doesNotMatch(r.json().message,/answer it there/,'a consumed decision cannot be answered again'); + assert.doesNotMatch(r.json().message,/nothing was applied/i); + assert.doesNotMatch(r.json().message,/inbox|answer it there/,'a consumed decision is gone from the inbox'); assert.match(r.json().message,/check the Agents page/); }); test('E2E6 hire start: invalid-request from the bus says the values, not the shape, were refused',async()=>{ diff --git a/packages/webui/tests/org-live-edit-browser.test.mjs b/packages/webui/tests/org-live-edit-browser.test.mjs index 884520c96..5287a60e7 100644 --- a/packages/webui/tests/org-live-edit-browser.test.mjs +++ b/packages/webui/tests/org-live-edit-browser.test.mjs @@ -120,7 +120,7 @@ test('E2E6 P2-2: a pack apply re-reads the other sections and keeps its receipt' assert.match(await b.evaluate(`document.querySelector('[data-packs-status]').textContent`), /^Applied\. Added board\./, 'the receipt survives the re-read'); }); -test('E2E6 review P3: a failed read keeps an open editor, Discard drops a held form, and a focused button does not hold its section', { timeout: 60000 }, async t => { +test('E2E6 review P3: a failed read keeps an open editor, Discard drops a held form, a focused button does not hold its section, and a clean form shows a failed read', { timeout: 60000 }, async t => { const { b, ctl, refresh } = await fixture(t); await waitFor(b, `document.querySelector('[data-wf="intake"] [data-wf-edit]')&&document.querySelector('[data-autonomy-form] select[name=merge]')&&document.querySelector('[data-pack-preview-button]')`, 'sections'); // A focused Preview button alone no longer holds the packs section: the next poll shows a changed @@ -150,4 +150,15 @@ test('E2E6 review P3: a failed read keeps an open editor, Discard drops a held f assert.equal(await b.evaluate(`document.querySelector('[data-autonomy-form] select[name=merge]').value`), 'out of the loop', 'the stored value is back'); assert.equal(await b.evaluate(`document.querySelector('[data-autonomy-form]').hasAttribute('data-live-dirty')`), false); assert.equal(await b.evaluate(`document.activeElement.hasAttribute('data-autonomy-discard')`), true, 'focus stays on Discard'); + // Re-check P3: a focused button in a clean form gets its focus back after the poll repaints it. + await refresh({ poll: true }); + assert.equal(await b.evaluate(`document.activeElement.dataset.focusKey??document.activeElement.tagName`), 'autonomy-discard:acme', 'focus returns to Discard after a poll'); + // Re-check P3: a clean form the page does not hold shows a failed read instead of hiding it. + await b.evaluate(`document.activeElement.blur();true`); + ctl.autonomyFail = true; + await refresh({ poll: true }); + await waitFor(b, `!document.querySelector('[data-autonomy-form]')&&/bus-unreachable|could not|refused/i.test(document.querySelector('[data-autonomy]')?.textContent??'')`, 'the autonomy read error'); + ctl.autonomyFail = false; + await refresh({ poll: true }); + await waitFor(b, `document.querySelector('[data-autonomy-form] select[name=merge]')`, 'the form after a good read'); });