C4 follow-up #1: split canFileCeoStrategy from canRequestHire for file_approval tool scope
Old internal-tool-selector attached `file_approval` to any agent with `reportsTo === null`, and attached both tools through the same management-keyword predicate chain. That let a VP or director with org-management authority walk away with `file_approval`, contradicting the C4 commit narrative that scoped `approve_ceo_strategy` as CEO-only. Introduce `canFileCeoStrategy(db, companyId, agentId)` as a dedicated predicate requiring both a CEO-equivalent role/title AND `reportsTo === null`. Using either half alone is too loose: role alone lets a demoted CEO with a virtual superior still file strategy, `reportsTo` alone lets a VP with no manager slip through. The two conditions together pin the predicate to a genuine top-level CEO. - `hire_agent` continues to gate on `canRequestHire` (managers hiring ICs is a legitimate flow and the original "CEO-only" framing in the C4 commit message was narrower than the production reality). - `file_approval` now gates on `canFileCeoStrategy`, orthogonal to hire authority. - New unit test covers 4 edges: genuine CEO, CEO title with virtual superior, VP without manager, and plain IC.
This commit is contained in:
1 parent
9a5823b242
commit
1fad88bad1
3 files changed
+198
-9
No files matched your search
@@ -102,6 +102,32 @@ export function canRequestHire(db: AppDatabase, companyId: string, agentId: stri
|
||||
return hasManagementAuthority(agent);
|
||||
}
|
||||
|
||||
const CEO_ROLE_PATTERN = /^(chief executive officer|ceo)$/i;
|
||||
const CEO_TITLE_PATTERN = /^ceo$/i;
|
||||
|
||||
/**
|
||||
* True only when the agent is a top-level CEO — CEO-equivalent role/title
|
||||
* AND no internal manager (`reportsTo === null`, i.e. reports to the Board).
|
||||
*
|
||||
* Intentionally orthogonal to {@link canRequestHire}: hiring authority spans
|
||||
* managers, directors, VPs, HR, etc., but the `approve_ceo_strategy` flow is
|
||||
* the CEO's alone. Using `reportsTo === null` by itself is too loose (a VP
|
||||
* with no manager would slip through) and using a keyword list by itself is
|
||||
* too loose (a "VP" matches the management keywords). Requiring both pins
|
||||
* the predicate to a genuine CEO even when someone inserts a virtual
|
||||
* superior above an executive.
|
||||
*/
|
||||
export function canFileCeoStrategy(db: AppDatabase, companyId: string, agentId: string): boolean {
|
||||
const agent = safeGetAgent(db, agentId);
|
||||
if (!agent || agent.companyId !== companyId) {
|
||||
return false;
|
||||
}
|
||||
if (agent.reportsTo !== null) {
|
||||
return false;
|
||||
}
|
||||
return CEO_ROLE_PATTERN.test(agent.role.trim()) || CEO_TITLE_PATTERN.test(agent.title.trim());
|
||||
}
|
||||
|
||||
function getDepartmentLeadCandidateId(
|
||||
db: AppDatabase,
|
||||
companyId: string,
|
||||
|
||||
@@ -6,12 +6,20 @@
|
||||
* re-derives this. Rule set, in order:
|
||||
*
|
||||
* - An agent that `canRequestHire` (CEO, HR, dept lead with reports,
|
||||
* anyone with org-management authority) receives `hire_agent`.
|
||||
* - A CEO (reportsTo null) additionally receives `file_approval` so
|
||||
* the `approve_ceo_strategy` flow works without the HTTP shim. Peer
|
||||
* anyone with org-management authority) receives `hire_agent`. A
|
||||
* manager hiring an IC is a legitimate flow.
|
||||
* - An agent that `canFileCeoStrategy` (genuine CEO: CEO-equivalent
|
||||
* role + `reportsTo === null`) receives `file_approval` so the
|
||||
* `approve_ceo_strategy` flow works without the HTTP shim. Peer
|
||||
* agents use the HTTP path until the file_approval surface is
|
||||
* generalised.
|
||||
*
|
||||
* The two predicates are intentionally orthogonal — a VP with hiring
|
||||
* authority should not inherit CEO-strategy approval scope, and a CEO
|
||||
* with a virtual superior inserted above them should not silently
|
||||
* lose strategy scope either (guarded by `canFileCeoStrategy`'s role
|
||||
* check, not just `reportsTo`).
|
||||
*
|
||||
* No hedging, no sentinel values: if the DB lookup throws the caller
|
||||
* is already in a broken state and should not silently fall through
|
||||
* to an empty tool set.
|
||||
@@ -19,7 +27,7 @@
|
||||
|
||||
import type { AppDatabase } from "@main/database";
|
||||
import type { AgentRecord } from "@shared/types";
|
||||
import { canRequestHire } from "@main/agent-permissions";
|
||||
import { canFileCeoStrategy, canRequestHire } from "@main/agent-permissions";
|
||||
|
||||
export type InternalToolName = "hire_agent" | "file_approval";
|
||||
|
||||
@@ -31,11 +39,7 @@ export function resolveInternalToolsForAgent(
|
||||
if (canRequestHire(db, agent.companyId, agent.id)) {
|
||||
tools.push("hire_agent");
|
||||
}
|
||||
// `file_approval` is currently scoped to the CEO (no manager) because
|
||||
// the canonical use case (`approve_ceo_strategy`) is CEO-only and
|
||||
// the broader approval surface ships via the HTTP route for now.
|
||||
// Widen the predicate when non-CEO agents need tool-call approvals.
|
||||
if (agent.reportsTo === null) {
|
||||
if (canFileCeoStrategy(db, agent.companyId, agent.id)) {
|
||||
tools.push("file_approval");
|
||||
}
|
||||
return tools;
|
||||
|
||||
@@ -0,0 +1,159 @@
|
||||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { AppDatabase } from "../../src/main/database";
|
||||
import { canFileCeoStrategy, canRequestHire } from "../../src/main/agent-permissions";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
async function createTempDir(prefix: string) {
|
||||
const dir = await mkdtemp(join(tmpdir(), prefix));
|
||||
tempDirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
async function setupCompany(prefix: string) {
|
||||
const profileDir = await createTempDir(prefix);
|
||||
const db = new AppDatabase(profileDir);
|
||||
db.init();
|
||||
|
||||
const companyId = db.company.saveCompany({
|
||||
name: "CEO Strategy Co",
|
||||
description: "Tests canFileCeoStrategy orthogonality",
|
||||
status: "active",
|
||||
});
|
||||
const workspaceId = db.workspacesMod.saveWorkspace({
|
||||
companyId,
|
||||
name: "Primary",
|
||||
localPath: profileDir,
|
||||
repoUrl: "",
|
||||
repoRef: "",
|
||||
isPrimary: true,
|
||||
});
|
||||
|
||||
return { db, companyId, workspaceId };
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
describe("canFileCeoStrategy", () => {
|
||||
it("returns true for a genuine CEO who reports to the board", async () => {
|
||||
const { db, companyId, workspaceId } = await setupCompany("agentcompany-ceo-strategy-real-");
|
||||
|
||||
const ceoId = db.agents.saveAgent({
|
||||
companyId,
|
||||
name: "Ada",
|
||||
role: "CEO",
|
||||
title: "Chief Executive Officer",
|
||||
status: "active",
|
||||
reportsTo: null,
|
||||
connectorId: "openai-codex",
|
||||
workspaceId,
|
||||
model: null,
|
||||
capabilities: "Lead the company",
|
||||
budgetMonthlyUsd: 250,
|
||||
});
|
||||
|
||||
expect(canFileCeoStrategy(db, companyId, ceoId)).toBe(true);
|
||||
|
||||
db.close();
|
||||
});
|
||||
|
||||
it("returns false for a CEO-titled agent with a virtual superior", async () => {
|
||||
const { db, companyId, workspaceId } = await setupCompany("agentcompany-ceo-strategy-virtual-");
|
||||
|
||||
const boardId = db.agents.saveAgent({
|
||||
companyId,
|
||||
name: "Board Proxy",
|
||||
role: "Board",
|
||||
title: "Board Representative",
|
||||
status: "active",
|
||||
reportsTo: null,
|
||||
connectorId: "openai-codex",
|
||||
workspaceId,
|
||||
model: null,
|
||||
capabilities: "Hold CEO accountable",
|
||||
budgetMonthlyUsd: 100,
|
||||
});
|
||||
const ceoId = db.agents.saveAgent({
|
||||
companyId,
|
||||
name: "Ada",
|
||||
role: "CEO",
|
||||
title: "Chief Executive Officer",
|
||||
status: "active",
|
||||
reportsTo: boardId,
|
||||
connectorId: "openai-codex",
|
||||
workspaceId,
|
||||
model: null,
|
||||
capabilities: "Lead the company",
|
||||
budgetMonthlyUsd: 250,
|
||||
});
|
||||
|
||||
expect(canFileCeoStrategy(db, companyId, ceoId)).toBe(false);
|
||||
|
||||
db.close();
|
||||
});
|
||||
|
||||
it("returns false for a VP with no manager (role is not CEO)", async () => {
|
||||
const { db, companyId, workspaceId } = await setupCompany("agentcompany-ceo-strategy-vp-");
|
||||
|
||||
const vpId = db.agents.saveAgent({
|
||||
companyId,
|
||||
name: "Kai",
|
||||
role: "VP Product",
|
||||
title: "Vice President of Product",
|
||||
status: "active",
|
||||
reportsTo: null,
|
||||
connectorId: "openai-codex",
|
||||
workspaceId,
|
||||
model: null,
|
||||
capabilities: "Own product strategy",
|
||||
budgetMonthlyUsd: 220,
|
||||
department: "product",
|
||||
});
|
||||
|
||||
expect(canFileCeoStrategy(db, companyId, vpId)).toBe(false);
|
||||
expect(canRequestHire(db, companyId, vpId)).toBe(true);
|
||||
|
||||
db.close();
|
||||
});
|
||||
|
||||
it("returns false for a plain IC", async () => {
|
||||
const { db, companyId, workspaceId } = await setupCompany("agentcompany-ceo-strategy-ic-");
|
||||
|
||||
const ceoId = db.agents.saveAgent({
|
||||
companyId,
|
||||
name: "Ada",
|
||||
role: "CEO",
|
||||
title: "Chief Executive Officer",
|
||||
status: "active",
|
||||
reportsTo: null,
|
||||
connectorId: "openai-codex",
|
||||
workspaceId,
|
||||
model: null,
|
||||
capabilities: "Lead the company",
|
||||
budgetMonthlyUsd: 250,
|
||||
});
|
||||
const icId = db.agents.saveAgent({
|
||||
companyId,
|
||||
name: "Eve",
|
||||
role: "Engineer",
|
||||
title: "Software Engineer",
|
||||
status: "active",
|
||||
reportsTo: ceoId,
|
||||
connectorId: "openai-codex",
|
||||
workspaceId,
|
||||
model: null,
|
||||
capabilities: "Build features",
|
||||
budgetMonthlyUsd: 180,
|
||||
department: "engineering",
|
||||
});
|
||||
|
||||
expect(canFileCeoStrategy(db, companyId, icId)).toBe(false);
|
||||
|
||||
db.close();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user