diff --git a/agents/honker/work/queue-194/BUILD.md b/agents/honker/work/queue-194/BUILD.md index 3c3574fa1..ca2771a9e 100644 --- a/agents/honker/work/queue-194/BUILD.md +++ b/agents/honker/work/queue-194/BUILD.md @@ -24,7 +24,7 @@ brief commit). Slices that don't need row 191 build first. S6 waits for row 191' | S1 | business: vocabulary (role.hire, role.fire hard waits, HIRE_ALLOWED), role.mjs `hireable`, business.mjs keep/noHire/retired, instance-settings delete and veto ops, chief-of-staff.json `hireable: []` | T6 (business), T7, T8c, T17, D2.2 writer | done, see below | | S2 | bus: reloadBusiness and runtime callback, instance.launch/end/regenerate requests, instances.set delete, veto.set, instance.hire/fire, keep checks, D4 hook, tester checks | T8-T10, T9c, T12-T14, T18 | S2a (D3), S2b (veto, hook, keep, T13), S2c (requests, terminate, delete) and S2d (hire, fire, W-N1) and S2e (PRD 1.7 routing, sage-1471) done | | S3 | cli: launcher lifecyclePass, host ops, `business instance` verbs | T1-T6, T11 | S3a (launcher pass) and S3b (verbs, veto, end to end, T2a, T4, T6, T11) done | -| S4 | harness: launch tool schema, hire and fire tools | T8 (D3 item 5) | | +| S4 | harness: launch tool schema, hire and fire tools | T8 (D3 item 5) | done, see S4 notes | | S5 | webui: Agents section and routes | T16 | | | S6 | row 191 chain: halted checks, fire SIGKILL through row 191's stop | T9a, T9b, T15 | waits on row 191 | @@ -76,6 +76,12 @@ Listed here as the build changes them. answers `{transaction, retry: true}` with the failed delete's transaction, where S2c answered `{transaction: null, retry: true}`. The launcher needs it to name `.retired/I-`. The test checks the value and that the retry still neither checks nor writes the file. +- `packages/harness/tests/tools.test.mjs:48` (S4): the launch tool's `instance` was an enum of the + start-time listing. It is now `{type: "string", description, pattern: ID_PATTERN.source}`, and the + listing moves to the tool description, which the test matches. `:107` (the pm test's chief check) + asserts there is no enum and the description names `chief.launch.instances`. +- `packages/business/tests/chief-of-staff.test.mjs` (S4): the contract test gains a match on the new + sentence that a hire is listed for `launch` at once but runs only after the human sets its route. ## S1 notes @@ -377,3 +383,32 @@ Listed here as the build changes them. runs task-cli 9/9, webui pinned-copy 10/10, service 31/31, views 3/3. The broker change adds no import. The four must-run files ran while the `instances` guard was being written, and the guard touches only the read. No test pins a hash of scripts/mosaic. + +## S4 notes (the launch tool takes a hire, D3 item 5) + +- `packages/harness/src/tools.mjs`: the launch tool's `instance` is a string with the ID_PATTERN + pattern, not an enum fixed when the bundle was built. The start-time listing stays in the + description. The host checks the listing at each launch (`instance-not-listed`), so the chief + launches its own hire in the same session. The hire and fire tools came forward in S2e. +- The runner does not enforce a tool's schema pattern. A bad name reaches the host and refuses + `instance-not-listed`, so the end-to-end test asserts that. M15 (the enum kept) is killed by the + tools.test shape assertion, not by the end-to-end test. +- `packages/cli/tests/lifecycle.test.mjs` gains the T8 end to end: the chief's session hires + `sec-2` (definition chief-information-security-officer, the shipped definition `hireable` accepts: + message.send and record.write, no credential) with `TOOL hire`, a launch refuses `no-route`, the + human sets the route with `instances.set` op change, the same session launches it (launchedBy + chief), `NOT A NAME` and `reviewer` refuse `instance-not-listed`, then `TOOL fire` ends its run, + leaves it `launched: false` and a later launch refuses `instance-not-listed`. +- Route gap, raised with Sage (honker-0027): harness, model and thinking are agent-layer vars + (`business/src/vars.mjs:142-147`) and hire vars allow only model and limits.* (N3). So a hire runs + only after the human sets harness and thinking. D3 item 5 says the same session launches it, and + it does, after that human step. Letting hire vars carry the route would widen the chief's + authority, a policy change that is not mine to make. As built the human step stays, and + `roles/chief-of-staff.md` says so. +- `roles/chief-of-staff.md` (role policy, named in the manifest): one paragraph on the hire's route + step and on a fired instance staying in the file off the list. +- Singles, each run alone: business chief-of-staff 10/10, spec-review-contracts 3/3, role 19/19, + resolve 10/10, harness bundle 22/22, tools 23/23, runner 34/34, pi-session 8/8, seat sandbox + 10/10, cli launcher 29/29, thread-launch 11/11, git-socket 24/24, lifecycle 15/15, webui reads + 11/11. No test pins a hash of tools.mjs or roles/chief-of-staff.md. tools.mjs gains an import from + vocabulary.mjs, already in RUNNER_FILES, and the four must-run files passed after it. diff --git a/packages/business/tests/chief-of-staff.test.mjs b/packages/business/tests/chief-of-staff.test.mjs index e9cdfb0e8..35b9b8505 100644 --- a/packages/business/tests/chief-of-staff.test.mjs +++ b/packages/business/tests/chief-of-staff.test.mjs @@ -142,6 +142,8 @@ test("the contract names no duty to talk to the human and the pm contract no lau assert.doesNotMatch(chief, /main (agent|point of interaction)/i); assert.doesNotMatch(chief, /\bJason\b/); assert.match(chief, /never talk to the user/); + // Row 194 (#1692, D3 item 5, N3): a hire launches with no restart once the human sets its route. + assert.match(chief, /A hire is listed for `launch` at once, with no restart, but it\s+runs only after the human sets its route \(harness and thinking\)/); const pm = readFileSync(join(REPO_ROLES, "pm.md"), "utf8"); assert.doesNotMatch(pm, /^- Launch /m); assert.match(pm, /You don't launch sessions/); diff --git a/packages/cli/tests/lifecycle.test.mjs b/packages/cli/tests/lifecycle.test.mjs index 149d66c2c..41bf27efa 100644 --- a/packages/cli/tests/lifecycle.test.mjs +++ b/packages/cli/tests/lifecycle.test.mjs @@ -504,3 +504,46 @@ test("row 194 T11: two stack roots with the same business id; a launch, a termin assert.equal(b.launcher.registry.sessions.length, 0); assert.equal(Object.hasOwn(b.call("instances").instances, "sec-1"), true); }); + +// D3 item 5 and T8: the chief's session hires through its hire tool, and the same session launches +// the new name with no restart. Its launch tool takes a name, so a name absent from the start-time +// listing gets through to the host, which checks the file. +test("row 194 D3 item 5: the chief's session hires with its tool and launches its hire in the same session, then fires it", { skip, timeout: 90000 }, async (t) => { + const p = prepare(t); + // The shipped role file lists nothing hireable. The test's copy lists the CISO, which holds + // only message.send and record.write and no credential. + const roleFile = join(p.f.env.MOSAIC_ROLES_DIR, "chief-of-staff.json"); + const role = JSON.parse(readFileSync(roleFile, "utf8")); + role.hireable = ["chief-information-security-officer"]; + writeFileSync(roleFile, JSON.stringify(role, null, 2)); + const s = await start(t, p); + const chief = (await s.carry(s.call("instance.launch", { instance: "chief" }).request)).done.run; + await s.claimed(chief); + const ask = async (name, args) => { + const sent = s.call("message.send", { to: "chief", class: "REQUEST", body: `TOOL ${name} ${JSON.stringify(args)}` }).id; + return (await until(() => s.call("replies", { id: sent })[0], 30000)).body; + }; + const hired = JSON.parse(await ask("hire", { name: "sec-2", definition: "chief-information-security-officer", vars: { model: "claude-sonnet-5-5" }, reason: "a security lead for the review queue" })); + assert.ok(hired.transaction, JSON.stringify(hired)); + const listed = s.call("instances").instances["sec-2"]; + assert.equal(listed.launched, true); + // Hire vars set only model and limits.* (N3). harness and thinking are agent-layer route vars the + // human sets, so until then the launch refuses no-route. + assert.equal(await ask("launch", { instance: "sec-2" }), "refused: no-route"); + const route = { harness: "pi", model: "claude-sonnet-5-5", thinking: "high" }; + s.call("instances.set", { revision: s.call("instances").revision, instance: "sec-2", op: "change", vars: route }); + const launched = JSON.parse(await ask("launch", { instance: "sec-2" })); + assert.match(launched.run, /^r[0-9a-f]{12}$/); + assert.deepEqual(s.running("sec-2").map((x) => x.run), [launched.run]); + assert.equal(readSessions(p.f.dataRoot).find((x) => x.run === launched.run).launchedBy, "chief"); + // The schema's pattern guides the model. The host refuses any name its file doesn't list. + assert.equal(await ask("launch", { instance: "NOT A NAME" }), "refused: instance-not-listed"); + assert.equal(await ask("launch", { instance: "reviewer" }), "refused: instance-not-listed"); + await s.claimed(launched.run); + const fired = JSON.parse(await ask("fire", { name: "sec-2", reason: "the review is done" })); + assert.ok(fired.transaction, JSON.stringify(fired)); + await until(() => s.running("sec-2").length === 0, 30000); + // A fired instance stays in roles, off the listing, so the chief can't launch it again. + assert.equal(s.call("instances").instances["sec-2"].launched, false); + assert.equal(await ask("launch", { instance: "sec-2" }), "refused: instance-not-listed"); +}); diff --git a/packages/harness/src/tools.mjs b/packages/harness/src/tools.mjs index 58efc3ec9..65213c006 100644 --- a/packages/harness/src/tools.mjs +++ b/packages/harness/src/tools.mjs @@ -11,7 +11,7 @@ // name against this list and forwards the call with its own capability. import { connect } from "node:net"; -import { ACTIONS, THREAD_DEFINITION } from "../../business/src/vocabulary.mjs"; +import { ACTIONS, ID_PATTERN, THREAD_DEFINITION } from "../../business/src/vocabulary.mjs"; const str = (description, extra = {}) => ({ type: "string", description, ...extra }); const obj = (properties, required = []) => ({ type: "object", properties, required, additionalProperties: false }); @@ -113,9 +113,11 @@ export function typedTools({ resolved, arbiter = false, tracker = false, project decision: COMMON.decision, }, ["task_ref", "roles"])); } + // Row 194 (#1692, D3 item 5): the instance is a name, not an enum fixed when the bundle was built, + // so the chief launches its own hire without a restart. The host checks the listing at each launch. if (resolved.launch) { - add("launch", "launch", "role.launch", `Launch a managed session for one of these role instances: ${resolved.launch.instances.join(", ")}. The host refuses an instance that is already running, a full model family, or a launch while launches are switched off.`, obj({ - instance: str("Role instance to launch", { enum: [...resolved.launch.instances] }), + add("launch", "launch", "role.launch", `Launch a managed session for a role instance the organization lists for you to launch. When this session started they were: ${resolved.launch.instances.join(", ")}. An instance you hired since is listed too. The host refuses an instance that isn't listed, one that is already running, a full model family, or a launch while launches are switched off.`, obj({ + instance: str("Role instance to launch", { pattern: ID_PATTERN.source }), }, ["instance"])); } if (tracker) { diff --git a/packages/harness/tests/tools.test.mjs b/packages/harness/tests/tools.test.mjs index 907706d4a..0e58ce748 100644 --- a/packages/harness/tests/tools.test.mjs +++ b/packages/harness/tests/tools.test.mjs @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import { callTool, ORGANIZATION_KEYS, REPLY_CAP, replyText, typedTools, untypedActions } from "../src/tools.mjs"; import { ORGANIZATION_KEYS as BROKER_ORGANIZATION_KEYS, TASK_PAGE, TASK_PAGE_MAX } from "../../bus/src/broker.mjs"; import { SETTINGS_KEYS } from "../../business/src/launch-settings.mjs"; +import { ID_PATTERN } from "../../business/src/vocabulary.mjs"; import { fakeToolSocket, resolvedFor, scratch } from "./helpers.mjs"; const names = (tools) => tools.map((t) => t.name); @@ -44,7 +45,10 @@ test("the chief of staff gets launch, send_message, end_run, hire, fire, change_ const launch = tools.find((x) => x.name === "launch"); assert.equal(launch.verb, "launch"); assert.equal(launch.action, "role.launch"); - assert.deepEqual(launch.parameters.properties.instance.enum, ["coder", "reviewer"]); + // Row 194 (#1692, D3 item 5): a name with the id pattern, not an enum fixed at bundle time, so a hire + // launches with no restart. The start-time listing stays in the description, and the host checks it. + assert.deepEqual(launch.parameters.properties.instance, { type: "string", description: "Role instance to launch", pattern: ID_PATTERN.source }); + assert.match(launch.description, /When this session started they were: coder, reviewer\. An instance you hired since is listed too\. The host refuses an instance that isn't listed,/); // Row 132 (#1628): role.end and organization.change have their tools. Row 141 (#1635) granted // instance.name, and row 155 (#1649) gives it set_name, so no held action is untyped. Row 194 (#1692, // PRD 1.7 REQ-COS-3) granted role.hire and role.fire, with hire and fire. @@ -100,7 +104,8 @@ test("request_staff is the pm's alone: staff.request over message.send, roles an assert.ok(!names(tools).includes("launch")); const chief = resolvedFor(t, "chief"); const launch = typedTools({ resolved: chief, tracker: true }).find((x) => x.name === "launch"); - assert.deepEqual(launch.parameters.properties.instance.enum, chief.launch.instances); + assert.equal(launch.parameters.properties.instance.enum, undefined); + assert.ok(launch.description.includes(`they were: ${chief.launch.instances.join(", ")}.`)); }); // Row 132 (#1628, REQ-ROLE-2, REQ-COS-1, REQ-LIMIT-1): each tool maps to the broker verb and action row 123 added. diff --git a/roles/chief-of-staff.md b/roles/chief-of-staff.md index 4210170db..96e18fc56 100644 --- a/roles/chief-of-staff.md +++ b/roles/chief-of-staff.md @@ -34,8 +34,11 @@ product's code, you don't review it and you don't create tasks yourself. the `change_organization` tool. Hire an instance with `hire` and fire one with `fire`, each with a reason; you can hire only a definition your role file lists as hireable, and never an instance the human - kept. To change what an instance may do, raise a `policy.change` - decision. + kept. A hire is listed for `launch` at once, with no restart, but it + runs only after the human sets its route (harness and thinking); until + then `launch` refuses it `no-route`. A fired instance stays in the file + off the list, so `launch` refuses it. To change what an instance may + do, raise a `policy.change` decision. - You and the human are the ones who configure and stop the assistant. Change its vars, its tracker or its definition with `change_organization` and `assistants`; its holder and credentials