diff --git a/packages/webui/src/public/bus.js b/packages/webui/src/public/bus.js index 502cd003b..b0ad1875e 100644 --- a/packages/webui/src/public/bus.js +++ b/packages/webui/src/public/bus.js @@ -496,7 +496,7 @@ return r?`

${txt(r.text)}${r.code?` (${txt(r.code)})`:''}

`:'';} function paintFireNote(){for(const e of document.querySelectorAll('[data-agents-receipt]'))e.innerHTML=fireNote(e.dataset.agentsReceipt||hireOrg());} const fireSel=(attr,name,org)=>`[${attr}="${CSS.escape(name)}"]${org?`[data-agent-fire-org="${CSS.escape(org)}"]`:':not([data-agent-fire-org])'}`; - async function fireAgent(name,named=null){ + async function fireAgent(name,named=null,orgName=named){ const org=named??hireOrg(); // A named organization was checked against its own config when the control was drawn (decidesIn). if(fireBusy||!org||out()||(!named&&!canDecide)||isTester)return; @@ -504,10 +504,12 @@ for(const b of document.querySelectorAll('[data-agent-fire-yes],[data-agent-fire-no]'))b.disabled=true; const epoch=orgEpoch; const {res,json}=await decidePost(`${decideBase({ org: named })}/decisions/hire/start`,{business:org,hire:{op:'fire',level:'org',name,reason:FIRE_REASON}}); - if(epoch!==orgEpoch){if(res?.type==='opaqueredirect')signOut();return;} + // SP5 review F7: a named organization's Fire went to that organization, so its outcome still + // stands after the opened organization changes. + if(epoch!==orgEpoch&&!named){if(res?.type==='opaqueredirect')signOut();return;} fireBusy=false;fireAsk=null; if(res?.type==='opaqueredirect'){signOut();return;} - const where=named?` in ${org}`:''; + const where=named?` in ${orgName}`:''; if(res?.ok && json?.op==='resolved'){ for(const c of document.querySelectorAll(`[data-org="${CSS.escape(org)}"] [data-agent-card="${CSS.escape(name)}"]`))c.remove(); for(const r of document.querySelectorAll(`[data-agents-org="${CSS.escape(org)}"] tr[data-agent="${CSS.escape(name)}"]`))r.remove(); @@ -519,28 +521,30 @@ const r=decideRefusal(res,json,'outcome-unknown'); fireReceipt={org,system:!!named,failed:true,text:`${name}${where} was not fired: ${r.message}`,code:r.code}; paintFireNote(); - for(const c of document.querySelectorAll(fireSel('data-agent-fire-confirm',name,named)))c.outerHTML=fireButton(name,named); + for(const c of document.querySelectorAll(fireSel('data-agent-fire-confirm',name,named)))c.outerHTML=fireButton(name,named,orgName); } } const fireOrgAttr=org=>org?` data-agent-fire-org="${esc(org)}"`:''; const fireFocus=(kind,name,org)=>esc(org?`${kind}:${org}:${name}`:`${kind}:${name}`); - const fireButton=(name,org=null)=>``; - const fireConfirm=(name,org=null)=>`Fire ${txt(name)}? `; + // SP5 review F5: the words name the organization by its label (`orgName`); its id addresses the control. + const fireNameAttr=(org,orgName)=>org&&orgName&&orgName!==org?` data-agent-fire-name="${esc(orgName)}"`:''; + const fireButton=(name,org=null,orgName=org)=>``; + const fireConfirm=(name,org=null,orgName=org)=>`Fire ${txt(name)}? `; // What the agent card shows for one agent: nothing without the decide right or outside the page's own // organization. SP5: `allowed` given means the caller read that organization's own config (decidesIn); // the control is then drawn for that organization, whichever one was opened last. - function fireControl(org,name,allowed){ + function fireControl(org,name,allowed,orgName=org){ const named=typeof allowed==='boolean'; if(isTester||(named?!allowed:(!canDecide||org!==hireOrg())))return ''; - return fireAsk===fireKey(org,name)?fireConfirm(name,named?org:null):fireButton(name,named?org:null); + return fireAsk===fireKey(org,name)?fireConfirm(name,named?org:null,named?orgName:null):fireButton(name,named?org:null,named?orgName:null); } document.addEventListener('click',e=>{ const t=e.target.closest?.('[data-agent-fire],[data-agent-fire-yes],[data-agent-fire-no]'); if(!t)return; - const named=t.dataset.agentFireOrg??null,org=named??hireOrg(); - if(t.hasAttribute('data-agent-fire')){const name=t.dataset.agentFire;fireAsk=fireKey(org,name);t.outerHTML=fireConfirm(name,named);document.querySelector(fireSel('data-agent-fire-yes',name,named))?.focus();} - else if(t.hasAttribute('data-agent-fire-no')){const name=t.dataset.agentFireNo;fireAsk=null;t.closest('[data-agent-fire-confirm]').outerHTML=fireButton(name,named);announce('Cancelled. Nothing was fired.');} - else void fireAgent(t.dataset.agentFireYes,named); + const named=t.dataset.agentFireOrg??null,org=named??hireOrg(),orgName=t.closest('[data-agent-fire-name]')?.dataset.agentFireName??named; + if(t.hasAttribute('data-agent-fire')){const name=t.dataset.agentFire;fireAsk=fireKey(org,name);t.outerHTML=fireConfirm(name,named,orgName);document.querySelector(fireSel('data-agent-fire-yes',name,named))?.focus();} + else if(t.hasAttribute('data-agent-fire-no')){const name=t.dataset.agentFireNo;fireAsk=null;t.closest('[data-agent-fire-confirm]').outerHTML=fireButton(name,named,orgName);document.querySelector(fireSel('data-agent-fire',name,named))?.focus();announce('Cancelled. Nothing was fired.');} + else void fireAgent(t.dataset.agentFireYes,named,orgName); }); window.MosaicDecide=Object.freeze({fireControl,fireNote}); document.addEventListener('click',e=>{if(e.target.closest?.('[data-hire-load]'))hireSubmit(true);}); diff --git a/packages/webui/src/public/shell.css b/packages/webui/src/public/shell.css index 80a498fe7..d5eb75c6c 100644 --- a/packages/webui/src/public/shell.css +++ b/packages/webui/src/public/shell.css @@ -699,6 +699,7 @@ html[data-organizations] .ws-agt td.ws-act>*+*{margin-left:6px} html[data-organizations] .ws-agt td.ws-act .btn.small{min-height:0;padding:4px 10px;font:600 13px/1.4 var(--font);border-radius:var(--r)} html[data-organizations] .ws-agt .ws-fire-confirm{display:inline-flex;flex-wrap:wrap;justify-content:flex-end;align-items:center;gap:6px;white-space:normal} html[data-organizations] .ws-agt .t2.mono{overflow-wrap:anywhere} +html[data-organizations] .ws-agt tr.on{background:color-mix(in srgb,var(--action) 10%,var(--surface))} html[data-organizations] .ws-ochip{background:color-mix(in srgb,var(--org,var(--muted)) 14%,var(--surface))} html[data-organizations] .ws-ochip i{width:8px;height:8px;border-radius:999px;background:var(--org,var(--muted));flex:none} html[data-organizations] .ws-msrc{align-items:center;gap:5px;font-size:12px;color:var(--muted);margin-bottom:2px} diff --git a/packages/webui/src/public/workspace.js b/packages/webui/src/public/workspace.js index 3d82f4265..f3fd6aeca 100644 --- a/packages/webui/src/public/workspace.js +++ b/packages/webui/src/public/workspace.js @@ -902,7 +902,9 @@ const AGENT_STATE = [['working', 'Working'], ['waiting', 'Waiting on you'], ['idle', 'Idle'], ['running', 'Running'], ['not-running', 'Not running'], ['cant-launch', "Can't launch"]]; const AGENT_STATE_KEY = { working: 'working', 'waiting on you': 'waiting', idle: 'idle', running: 'running', 'not running': 'not-running', "can't launch": 'cant-launch' }; async function agentsPage(q, opts = {}) { - const scope = typeof opts.scope === 'string' && ORG_ID.test(opts.scope) ? opts.scope : null; + // SP5 review F11: a scope that is not an organization id is refused, never widened to the system list. + if (opts.scope != null && !(typeof opts.scope === 'string' && ORG_ID.test(opts.scope))) return '

Agents

That is not an organization address.

'; + const scope = opts.scope ?? null; const org = scope ? [scope] : listParam(q, 'org', id => ORG_ID.test(id)); const want = String(q?.get?.('state') || ''); const stateF = AGENT_STATE.some(([k]) => k === want) ? want : null; @@ -942,19 +944,24 @@ const counts = new Map(); for (const a of all) counts.set(a.org, (counts.get(a.org) || 0) + 1); const inOrg = all.filter(a => !org || org.includes(a.org)).sort((x, y) => x.org.localeCompare(y.org) || x.name.localeCompare(y.name) || x.key.localeCompare(y.key)); - const rows = inOrg.filter(a => !stateF || a.state === stateF); + const filteredRows = inOrg.filter(a => !stateF || a.state === stateF); + // SP5 review F1/F2: Open addresses one agent (`#/orgs//agents?agent=`). The scoped page puts + // that agent first and marks it, and draws no Open of its own, which would only link to itself. + const picked = scope ? String(q?.get?.('agent') || '') : ''; + const hit = picked ? filteredRows.find(a => a.key === picked) || null : null; + const rows = hit ? [hit, ...filteredRows.filter(a => a !== hit)] : filteredRows; const akey = a => `${a.org}:${a.key}`; const orgChip = a => `${esc(o.name(a.org))}`; const projOf = a => a.project || a.s?.project || null; const projChip = a => (projOf(a)?.name ? `${esc(projOf(a).name)}` : ''); const work = a => (a.s?.task ? `${esc(a.s.task.title || a.s.task.ref)}${a.s.task.title && a.s.task.ref ? `${esc(a.s.task.ref)}` : ''}` : 'nothing reported'); const LEVEL_WORD = { root: 'root agent copy', project: 'project agent' }; - const row = a => ` + const row = a => ` ${esc(a.name)}${a.id ? `${esc(a.id)}` : ''}${orgChip(a)}${projChip(a)}${esc(a.role)}${a.s?.task ? `Working on ${esc(a.s.task.title || a.s.task.ref)}` : 'Working on nothing reported'} ${esc(a.role)}${a.level !== 'org' ? `${esc(LEVEL_WORD[a.level])}` : ''} ${dot(...a.dot, true)} ${work(a)} - Open ${esc(a.name)} in ${esc(o.name(a.org))}${a.fire ? window.MosaicDecide?.fireControl(a.org, a.name, true) ?? '' : ''} + ${scope ? '' : `Open ${esc(a.name)} in ${esc(o.name(a.org))}${a.fire ? window.MosaicDecide?.fireControl(a.org, a.name, true, o.name(a.org)) ?? '' : ''}`} `; // An organization that did not answer is named first, with the reason its read gave; one that // answered but whose instances read failed is named too, never counted as having no agents. @@ -967,7 +974,7 @@ const anyRead = reads.some(r => r.inst && (!org || org.includes(r.id))); const filtered = (org && !scope) || stateF; const table = rows.length - ? `
${rows.map(row).join('')}
Agents
AgentRoleStateWorking onActions
` + ? `
${scope ? '' : ''}${rows.map(row).join('')}
Agents
AgentRoleStateWorking onActions
` : `

${!anyRead && o.rows.length ? 'Agents not read' : filtered ? 'Nothing matches these filters' : 'No agents'}

`; const stateCounts = new Map(); for (const a of inOrg) stateCounts.set(a.state, (stateCounts.get(a.state) || 0) + 1); diff --git a/packages/webui/tests/system-agents-browser.test.mjs b/packages/webui/tests/system-agents-browser.test.mjs index b429e079f..be3941798 100644 --- a/packages/webui/tests/system-agents-browser.test.mjs +++ b/packages/webui/tests/system-agents-browser.test.mjs @@ -126,7 +126,7 @@ test('#/agents lists both organizations\' agents in one table, each row with its assert.match(await text('[data-ws-agent="beta:research-2"] .t2'), new RegExp(`org:hire-${HIRE}`)); assert.match(await text('[data-ws-agent="acme:chief"] [data-ws-role]'), /Chief of staff/); // Each row opens the agent in its own organization. - assert.equal(await b.evaluate('document.querySelector(\'[data-ws-agent="beta:pm"] a[data-ws-open]\')?.getAttribute("href")'), '#/orgs/beta/agents'); + assert.equal(await b.evaluate('document.querySelector(\'[data-ws-agent="beta:pm"] a[data-ws-open]\')?.getAttribute("href")'), '#/orgs/beta/agents?agent=pm'); }); test('the organization and state filters live in the hash and survive a reload', async t => { @@ -164,6 +164,8 @@ test('firing beta\'s agent from #/agents while acme was opened last reaches beta await b.evaluate('window.__answers.push({ body: { id: "dec-fire-1", op: "resolved", choice: "yes" } })'); await b.evaluate('document.querySelector(\'[data-ws-agent="beta:pm"] [data-agent-fire-yes]\').click()'); await wait('/Fired pm/.test(document.querySelector("[data-ws-agents] [data-agents-receipt]")?.textContent ?? "")'); + // The receipt names the organization by its label, not its id. + assert.match(await text('[data-ws-agents] [data-agents-receipt]'), /^Fired pm in Beta Works\./); const posts = await b.evaluate('window.__posts'); assert.deepEqual(posts.map(p => p.url), ['/api/orgs/beta/decisions/hire/start']); assert.deepEqual(JSON.parse(posts[0].body), { business: 'beta', hire: { op: 'fire', level: 'org', name: 'pm', reason: 'Fired from the Agents page.' } }); @@ -208,3 +210,33 @@ test('#/orgs//agents shows that organization\'s rows only, with a link to ev await wait('document.querySelector("[data-org-agents]")'); assert.equal(await b.evaluate('document.querySelectorAll("#bus-view h1").length'), 1); }); + +test('Open addresses the agent: the scoped page puts it first, marks it and draws no Open of its own', async t => { + const { b, wait } = await setup(t); + await wait(are(ALL)); + assert.equal(await b.evaluate('document.querySelector(\'[data-ws-agent="beta:research-2"] [data-ws-open]\').getAttribute("href")'), '#/orgs/beta/agents?agent=research-2'); + await b.evaluate('document.querySelector(\'[data-ws-agent="beta:research-2"] [data-ws-open]\').click()'); + await wait(are(['beta:research-2', 'beta:pm'])); + assert.equal(await b.evaluate('location.hash'), '#/orgs/beta/agents?agent=research-2'); + assert.equal(await b.evaluate('document.querySelector(\'[data-ws-agent="beta:research-2"]\').getAttribute("aria-current")'), 'true'); + assert.equal(await b.evaluate('document.querySelector(\'[data-ws-agent="beta:pm"]\').hasAttribute("aria-current")'), false); + // No self-link on the scoped page. + assert.equal(await b.evaluate('document.querySelectorAll("[data-ws-agents] [data-ws-open]").length'), 0); + assert.equal(await b.evaluate('document.querySelectorAll("[data-ws-agents] thead th").length'), 4); + // An agent the organization does not have marks nothing. + await b.evaluate('location.hash = "#/orgs/beta/agents?agent=nobody"'); + await wait(`${are(['beta:pm', 'beta:research-2'])} && !document.querySelector('[data-ws-agents] [aria-current]')`); +}); + +test('Cancel puts focus back on the row\'s Fire, which is labelled with the organization\'s name', async t => { + const { b, wait } = await setup(t, { stub: true }); + await wait(`${are(ALL)} && document.querySelector('[data-ws-agent="beta:pm"] [data-agent-fire]')`); + const fire = '\'[data-ws-agent="beta:pm"] [data-agent-fire]\''; + assert.equal(await b.evaluate(`document.querySelector(${fire}).getAttribute('aria-label')`), 'Fire pm in Beta Works'); + await b.evaluate(`document.querySelector(${fire}).click()`); + await wait('document.querySelector(\'[data-ws-agent="beta:pm"] [data-agent-fire-no]\')'); + assert.equal(await b.evaluate('document.querySelector(\'[data-ws-agent="beta:pm"] [data-agent-fire-confirm]\').getAttribute("aria-label")'), 'Fire pm in Beta Works?'); + await b.evaluate('document.querySelector(\'[data-ws-agent="beta:pm"] [data-agent-fire-no]\').click()'); + await wait(`document.activeElement === document.querySelector(${fire})`); + assert.deepEqual(await b.evaluate('window.__posts'), []); +});