mirror of
https://github.com/mvschwarz/openrig.git
synced 2026-11-07 05:56:21 -10:00
fix: tell a hosted member how it gets work, what it may read and how to close (#2187)
Three things a hosted member is told were misleading in a native test, and the member repeated or acted on each. A hosted member with no wake trigger fell through to the general unroutable teaching, "Fix the address, or register the human with `rig gateway human add`". It now reads that the member is a hosted member with no wake trigger, so nothing can push work to it, and that it reads the queue when its owner next starts a session. The result is still `unroutable` with the same prefix, so the stuck sweep still tells the member's person, and an address no roster lists keeps the general teaching. The member workflow, now `member-workflow/v2` and quoted in `docs/reference/hosted-members.md`, says to describe reach from the grant rather than the tool list, and to close a finished item with `no-follow-on`. `rig_queue_update`'s closure-reason description lists the reasons it takes. No new reason is accepted, and nothing else changes. 7 files, +74 −9 (source +40 −5, tests +33 −3, docs +1 −1). No new refusals.
This commit is contained in:
1 parent
30a3628b7d
commit
c823dfedb4
7 files changed
+74
-9
No files matched your search
@@ -52,7 +52,7 @@ member, so it checks its rows again.
|
||||
|
||||
The server tells the member this in its MCP instructions. The text is versioned, so you can check which version a tool received:
|
||||
|
||||
> Member workflow (member-workflow/v1). Your inbox is the items addressed to your seat: rig_queue_list with owned set. It's yours, not the team's queue. A wake, a ring or a message only points at it: list your inbox, then act. For each item: claim it (rig_queue_claim), do it, then close it done with the answer in resolution (rig_queue_update), or hand it on (rig_queue_handoff). Listing or reading an item doesn't take it. If the person connected you, that's basically authorized: claim the items in your inbox and do them without asking whether you may. When an item needs a person's approval before you can finish it, don't close it and don't take the action: keep the item and ask. If it came from a person (an address ending in @external that isn't a shared channel; the item's 'Shared channel:' line names one), set it blocked with blockedOn that person and, as its summary, the question with a short form of what you'll do: the person sees the summary and the item, not your note, which keeps the record. Link a longer draft as an https evidenceRef. Their answer returns the item to you in progress. Otherwise ask as a new item, to the seat it came from or, for a shared channel's item, to your rig's lead (rig_team shows it), with your question and draft. Then set yours blocked with blockedOn that new qitemId: when that item closes, yours returns to pending, to claim again. Nothing promises to wake you, so check your inbox again. Then finish the same item; don't open a new one.
|
||||
> Member workflow (member-workflow/v2). Your inbox is the items addressed to your seat: rig_queue_list with owned set. It's yours, not the team's queue. A wake, a ring or a message only points at it: list your inbox, then act. For each item: claim it (rig_queue_claim), do it, then close it done with closure reason no-follow-on and the answer in resolution (rig_queue_update), or hand it on (rig_queue_handoff). Listing or reading an item doesn't take it. Say what you can reach from your grant, not from the tool list: a listed tool isn't access. rig_artifact_read's answer for an item names the shared roots you may read, and none means no shared files. If the person connected you, that's basically authorized: claim the items in your inbox and do them without asking whether you may. When an item needs a person's approval before you can finish it, don't close it and don't take the action: keep the item and ask. If it came from a person (an address ending in @external that isn't a shared channel; the item's 'Shared channel:' line names one), set it blocked with blockedOn that person and, as its summary, the question with a short form of what you'll do: the person sees the summary and the item, not your note, which keeps the record. Link a longer draft as an https evidenceRef. Their answer returns the item to you in progress. Otherwise ask as a new item, to the seat it came from or, for a shared channel's item, to your rig's lead (rig_team shows it), with your question and draft. Then set yours blocked with blockedOn that new qitemId: when that item closes, yours returns to pending, to claim again. Nothing promises to wake you, so check your inbox again. Then finish the same item; don't open a new one.
|
||||
|
||||
## Ring it when work lands
|
||||
|
||||
|
||||
@@ -530,12 +530,13 @@ export const MCP_OPERATIONS: ReadonlyArray<{ tool: string; routes: readonly stri
|
||||
|
||||
/** How a member works its inbox, and asks a person while keeping the work: one versioned text, carried in this server's
|
||||
* instructions and quoted in docs/reference/hosted-members.md. Change it here, bump the version, and update the quote. */
|
||||
export const MEMBER_WORKFLOW_VERSION = "member-workflow/v1";
|
||||
export const MEMBER_WORKFLOW_VERSION = "member-workflow/v2";
|
||||
/** What the connection itself authorizes. */
|
||||
const MEMBER_AUTHORITY = "If the person connected you, that's basically authorized: claim the items in your inbox and do them without asking whether you may.";
|
||||
export const MEMBER_WORKFLOW = [
|
||||
`Member workflow (${MEMBER_WORKFLOW_VERSION}). Your inbox is the items addressed to your seat: rig_queue_list with owned set. It's yours, not the team's queue. A wake, a ring or a message only points at it: list your inbox, then act.`,
|
||||
"For each item: claim it (rig_queue_claim), do it, then close it done with the answer in resolution (rig_queue_update), or hand it on (rig_queue_handoff). Listing or reading an item doesn't take it.",
|
||||
"For each item: claim it (rig_queue_claim), do it, then close it done with closure reason no-follow-on and the answer in resolution (rig_queue_update), or hand it on (rig_queue_handoff). Listing or reading an item doesn't take it.",
|
||||
"Say what you can reach from your grant, not from the tool list: a listed tool isn't access. rig_artifact_read's answer for an item names the shared roots you may read, and none means no shared files.",
|
||||
MEMBER_AUTHORITY,
|
||||
"When an item needs a person's approval before you can finish it, don't close it and don't take the action: keep the item and ask.",
|
||||
"If it came from a person (an address ending in @external that isn't a shared channel; the item's 'Shared channel:' line names one), set it blocked with blockedOn that person and, as its summary, the question with a short form of what you'll do: the person sees the summary and the item, not your note, which keeps the record. Link a longer draft as an https evidenceRef. Their answer returns the item to you in progress.",
|
||||
@@ -1349,7 +1350,7 @@ export function createMcpServer(client: DaemonClient, options: { seat?: string;
|
||||
inputSchema: z.object({
|
||||
qitemId: z.string().describe("The queue item to update"),
|
||||
state: z.string().optional().describe("New state (e.g. in-progress, blocked, done)"),
|
||||
closureReason: z.string().optional().describe("Why it closed (as rig queue update --closure-reason)"),
|
||||
closureReason: z.string().optional().describe("Why it closed: no-follow-on for finished work, or handed_off_to, denied, canceled, escalation or superseded (as rig queue update --closure-reason)"),
|
||||
closureTarget: z.string().optional().describe("Closure target, when the reason needs one"),
|
||||
note: z.string().optional().describe("Transition note: progress, or why this transition happened (required to reopen)"),
|
||||
resolution: z.string().optional().describe("The outcome the item keeps, stored only by the update that closes an open item (as rig queue update --resolution)"),
|
||||
|
||||
@@ -537,8 +537,23 @@ describe("MCP Server", () => {
|
||||
expect(instructions).toContain(MEMBER_WORKFLOW);
|
||||
// The inbox, the claim before work, and keeping an item blocked while a person is asked.
|
||||
for (const part of ["rig_queue_list with owned set", "rig_queue_claim", "blockedOn that person", "blockedOn that new qitemId"]) expect(MEMBER_WORKFLOW).toContain(part);
|
||||
// v2: the closure reason a finished item takes, and scope described from the grant, not the tool list.
|
||||
expect(MEMBER_WORKFLOW_VERSION).toBe("member-workflow/v2");
|
||||
for (const part of ["closure reason no-follow-on", "a listed tool isn't access", "names the shared roots you may read"]) expect(MEMBER_WORKFLOW).toContain(part);
|
||||
const doc = fs.readFileSync(new URL("../../../docs/reference/hosted-members.md", import.meta.url), "utf8");
|
||||
expect(doc).toContain(`> ${MEMBER_WORKFLOW}`);
|
||||
expect(doc).not.toContain("member-workflow/v1");
|
||||
await cleanup();
|
||||
});
|
||||
|
||||
it("names the closure reasons rig_queue_update takes, finished work first", async () => {
|
||||
await setup();
|
||||
const { tools } = await mcpClient.listTools();
|
||||
const update = tools.find((tool) => tool.name === "rig_queue_update");
|
||||
const reason = (update?.inputSchema as { properties?: Record<string, { description?: string }> } | undefined)?.properties?.closureReason?.description ?? "";
|
||||
expect(reason).toContain("no-follow-on for finished work");
|
||||
for (const value of ["handed_off_to", "denied", "canceled", "escalation", "superseded"]) expect(reason).toContain(value);
|
||||
expect(reason).not.toContain("completed");
|
||||
await cleanup();
|
||||
});
|
||||
|
||||
|
||||
@@ -32,7 +32,8 @@
|
||||
// specimen class) is gateway-routable: a PANELLESS virtual identity whose
|
||||
// delivery rides the gateway, exactly like its @external address.
|
||||
// 4. A hosted member whose roster entry declares a trigger is hosted. One with no
|
||||
// trigger stays unroutable, so its person is told.
|
||||
// trigger stays unroutable, so its person is told; its teaching says it reads the
|
||||
// queue when its owner next starts a session, not to fix the address.
|
||||
// 5. Anything else is unroutable, with teaching that names BOTH checks.
|
||||
import { parseSessionName } from "../session-name.js";
|
||||
import type { RosterWake } from "../rosters.js";
|
||||
@@ -52,6 +53,8 @@ export interface ClassifyDeps {
|
||||
hasTerminalTransport: (destination: string) => boolean;
|
||||
/** The trigger a hosted member's roster entry declares, when it declares one. */
|
||||
hostedWake?: (destination: string) => RosterWake | null;
|
||||
/** Whether a roster lists the destination as a hosted member, trigger or not. */
|
||||
isHostedMember?: (destination: string) => boolean;
|
||||
}
|
||||
|
||||
export function classifyDestination(destination: string, deps: ClassifyDeps): DestinationClass {
|
||||
@@ -69,6 +72,14 @@ export function classifyDestination(destination: string, deps: ClassifyDeps): De
|
||||
}
|
||||
const wake = deps.hostedWake?.(destination) ?? null;
|
||||
if (wake) return { class: "hosted", wake };
|
||||
if (deps.isHostedMember?.(destination)) {
|
||||
// The member reads this teaching too: the address is right, and it isn't a person to register.
|
||||
return {
|
||||
class: "unroutable",
|
||||
teaching: `unroutable: '${destination}' is a hosted member with no wake trigger, so nothing can push work to it. ` +
|
||||
`It reads the queue when its owner next starts a session.`,
|
||||
};
|
||||
}
|
||||
return {
|
||||
class: "unroutable",
|
||||
teaching:
|
||||
|
||||
@@ -22,7 +22,7 @@ import { parseReplyToChoice, formatReplyToChoice, describeReplyToFallback, REPLY
|
||||
import { classifyDestination, type DestinationClass } from "./gateway/destination-resolver.js";
|
||||
import { join } from "node:path";
|
||||
import { SettingsStore } from "./user-settings/settings-store.js";
|
||||
import { hostedWakeFor, type RosterWake } from "./rosters.js";
|
||||
import { hostedWakeFor, isHostedMemberAt, type RosterWake } from "./rosters.js";
|
||||
import { ringHostedWake } from "./hosted-wake.js";
|
||||
import { effectiveGateRoles } from "./gate-predicate.js";
|
||||
import {
|
||||
@@ -566,6 +566,17 @@ function defaultHostedWake(destination: string): RosterWake | null {
|
||||
}
|
||||
}
|
||||
|
||||
/** Whether the authored rosters on this host list this address as a hosted member, trigger or not. False when they
|
||||
* can't be read: the address then gets the general unroutable teaching. */
|
||||
function defaultIsHostedMember(destination: string): boolean {
|
||||
try {
|
||||
const root = new SettingsStore().resolveOne("workspace.root").value;
|
||||
return typeof root === "string" && !!root && isHostedMemberAt(destination, join(root, "rosters"), getSelfHostId());
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export class QueueRepositoryError extends Error {
|
||||
readonly code: string;
|
||||
readonly meta: Record<string, unknown> | undefined;
|
||||
@@ -938,6 +949,8 @@ export class QueueRepository {
|
||||
private readonly sharedChannelsFn: () => ReadonlyArray<{ address: string }>;
|
||||
/** A hosted member's trigger, from the authored rosters on this host. */
|
||||
private readonly hostedWakeFn: (destination: string) => RosterWake | null;
|
||||
/** Whether the rosters list an address as a hosted member, trigger or not. */
|
||||
private readonly hostedMemberFn: (destination: string) => boolean;
|
||||
private readonly hostedWakeFetch: typeof fetch | undefined;
|
||||
/** OPR.0.4.6.WF3 FR-6 — injected by startup (never imported): the
|
||||
* workflow domain's is-live-frontier-packet predicate. */
|
||||
@@ -981,6 +994,7 @@ export class QueueRepository {
|
||||
loadHumanRegistry?: () => LoadResult;
|
||||
sharedChannels?: () => ReadonlyArray<{ address: string }>;
|
||||
hostedWake?: (destination: string) => RosterWake | null;
|
||||
isHostedMember?: (destination: string) => boolean;
|
||||
hostedWakeFetch?: typeof fetch;
|
||||
}
|
||||
) {
|
||||
@@ -997,6 +1011,7 @@ export class QueueRepository {
|
||||
this.loadHumanRegistryFn = opts?.loadHumanRegistry ?? (() => loadHumanRegistry());
|
||||
this.sharedChannelsFn = opts?.sharedChannels ?? (() => sharedChannelAddresses());
|
||||
this.hostedWakeFn = opts?.hostedWake ?? defaultHostedWake;
|
||||
this.hostedMemberFn = opts?.isHostedMember ?? defaultIsHostedMember;
|
||||
this.hostedWakeFetch = opts?.hostedWakeFetch;
|
||||
this.hasTargetRepoColumn = detectQueueColumn(db, "target_repo");
|
||||
this.hasSummaryColumn = detectQueueColumn(db, "summary");
|
||||
@@ -4356,6 +4371,7 @@ export class QueueRepository {
|
||||
})(),
|
||||
hasTerminalTransport: (dest) => this.hasTerminalTransport(dest),
|
||||
hostedWake: (dest) => this.hostedWakeFn(dest),
|
||||
isHostedMember: (dest) => this.hostedMemberFn(dest),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -102,3 +102,10 @@ export function hostedWakeFor(destination: string, folder: string, selfHostId: s
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Whether a roster on this host lists this address as a hosted member, with or without a trigger. */
|
||||
export function isHostedMemberAt(destination: string, folder: string, selfHostId: string | null): boolean {
|
||||
if (!selfHostId) return false;
|
||||
return readRosters(folder).rosters.some((roster) =>
|
||||
roster.members.some((m) => m.hosted === true && m.seat === destination && m.host === selfHostId));
|
||||
}
|
||||
@@ -12,7 +12,7 @@ import { migrate } from "../src/db/migrate.js";
|
||||
import { ALL_MIGRATIONS } from "../src/db/all-migrations.js";
|
||||
import { EventBus } from "../src/domain/event-bus.js";
|
||||
import { QueueRepository } from "../src/domain/queue-repository.js";
|
||||
import { hostedWakeFor, parseRoster, type RosterWake } from "../src/domain/rosters.js";
|
||||
import { hostedWakeFor, isHostedMemberAt, parseRoster, type RosterWake } from "../src/domain/rosters.js";
|
||||
import { ringHostedWake } from "../src/domain/hosted-wake.js";
|
||||
import { OutboxHandler, WAKE_INTENT_PREFIX } from "../src/domain/outbox-handler.js";
|
||||
import { createStuckSweepStatus, runStuckSweep, STUCK_SWEEP_FINDING_TAG } from "../src/domain/queue-stuck-sweep.js";
|
||||
@@ -98,6 +98,7 @@ describe("a hosted member with a webhook trigger is rung when work lands for it"
|
||||
loadHumanRegistry: () => ({ ok: true, entities: [] }),
|
||||
transport: { send: async () => { typed += 1; return { ok: true, verified: true }; } },
|
||||
hostedWake: (destination) => destination === "grok@lab" ? trigger : null,
|
||||
isHostedMember: (destination) => destination === "grok@lab",
|
||||
});
|
||||
expect(repo.classifyDestinationOf("grok@lab")).toEqual({ class: "hosted", wake: declared });
|
||||
// The create's own wake rings it (this repository has no outbox, so the wake runs inline).
|
||||
@@ -110,9 +111,15 @@ describe("a hosted member with a webhook trigger is rung when work lands for it"
|
||||
repo.claim({ qitemId: row.qitemId, destinationSession: "grok@lab" });
|
||||
expect(repo.getById(row.qitemId)).toMatchObject({ state: "in-progress", lastNudgeResult: expect.stringMatching(/^wake-accepted:/) });
|
||||
|
||||
// A hosted member with no trigger stays unroutable, so its person is told (step 1, unchanged).
|
||||
// A hosted member with no trigger stays unroutable, so its person is told (step 1, unchanged). The member reads this
|
||||
// teaching too: it says how the member gets work, not to fix the address or register a person.
|
||||
trigger = null;
|
||||
expect(repo.classifyDestinationOf("grok@lab").class).toBe("unroutable");
|
||||
const noTrigger = repo.classifyDestinationOf("grok@lab");
|
||||
expect(noTrigger).toEqual({ class: "unroutable", teaching: "unroutable: 'grok@lab' is a hosted member with no wake trigger, "
|
||||
+ "so nothing can push work to it. It reads the queue when its owner next starts a session." });
|
||||
// An address no roster lists keeps the general teaching.
|
||||
expect(repo.classifyDestinationOf("nobody@lab")).toMatchObject({ class: "unroutable",
|
||||
teaching: expect.stringContaining("Fix the address, or register the human with `rig gateway human add`.") });
|
||||
db.close();
|
||||
});
|
||||
|
||||
@@ -277,6 +284,14 @@ describe("a hosted member with a webhook trigger is rung when work lands for it"
|
||||
expect(hostedWakeFor("grok@lab", dir, "h2")).toBeNull();
|
||||
expect(hostedWakeFor("grok@lab", dir, null)).toBeNull();
|
||||
expect(hostedWakeFor("other@lab", dir, "h1")).toBeNull(); // not hosted
|
||||
// Listed as hosted, trigger or not, on this host only.
|
||||
expect(isHostedMemberAt("grok@lab", dir, "h1")).toBe(true);
|
||||
writeFileSync(join(dir, "lab.json"), JSON.stringify({ ...roster, members: [{ ...roster.members[0], wake: undefined }, roster.members[1]] }));
|
||||
expect(hostedWakeFor("grok@lab", dir, "h1")).toBeNull();
|
||||
expect(isHostedMemberAt("grok@lab", dir, "h1")).toBe(true);
|
||||
expect(isHostedMemberAt("grok@lab", dir, "h2")).toBe(false);
|
||||
expect(isHostedMemberAt("grok@lab", dir, null)).toBe(false);
|
||||
expect(isHostedMemberAt("other@lab", dir, "h1")).toBe(false);
|
||||
expect(() => parseRoster({ ...roster, members: [{ ...roster.members[0], wake: { webhook: { url: "https://example.invalid" } } }] }))
|
||||
.toThrow("A member's wake needs webhook.url_ref");
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user