Q107: a task approval covers the content it was given; edits need a fresh one
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c695fdb0f6
commit
4765683b86
12 files changed
+114
-52
No files matched your search
@@ -0,0 +1,12 @@
|
||||
// Existing task approvals are taken to cover the task as it stands now; from here on an edit needs a fresh one.
|
||||
export const approvalSubjectSnapshotSchema = `
|
||||
alter table approvals add column subject_snapshot text;
|
||||
update approvals set subject_snapshot = (
|
||||
select json_object('title', t.title, 'description', t.description, 'assignee', t.assignee_agent_id, 'risk', t.risk, 'workspace', t.workspace_id, 'filesystem', t.requires_filesystem)
|
||||
from tasks t where t.id = approvals.source_entity_id
|
||||
)
|
||||
where source_entity_type = 'task' and kind in ('task_risk', 'task_execution');
|
||||
|
||||
insert into schema_migrations(version, name, applied_at)
|
||||
values (85, 'approval_subject_snapshot', datetime('now'));
|
||||
`;
|
||||
@@ -83,6 +83,7 @@ import { connectorExecutionsSchema } from "./migrations/081-connector-executions
|
||||
import { connectorPendingAccountSchema } from "./migrations/082-connector-pending-account";
|
||||
import { connectorAccountLabelsSchema } from "./migrations/083-connector-account-labels";
|
||||
import { connectorGrantLimitsSchema } from "./migrations/084-connector-grant-limits";
|
||||
import { approvalSubjectSnapshotSchema } from "./migrations/085-approval-subject-snapshot";
|
||||
|
||||
interface Migration {
|
||||
version: number;
|
||||
@@ -175,6 +176,7 @@ const MIGRATIONS: Migration[] = [
|
||||
{ version: 82, name: "connector_pending_account", sql: connectorPendingAccountSchema },
|
||||
{ version: 83, name: "connector_account_labels", sql: connectorAccountLabelsSchema },
|
||||
{ version: 84, name: "connector_grant_limits", sql: connectorGrantLimitsSchema },
|
||||
{ version: 85, name: "approval_subject_snapshot", sql: approvalSubjectSnapshotSchema },
|
||||
];
|
||||
|
||||
const SCHEMA_VERSION = MIGRATIONS[MIGRATIONS.length - 1].version;
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { DatabaseSync } from "node:sqlite";
|
||||
import { APPROVAL_STATE, RISK_LEVEL, TASK_STATUS, WRITING_MODE } from "../../shared/domain";
|
||||
import { APPROVAL_STATE, RISK_LEVEL, TASK_STATUS, WRITING_MODE, type RiskLevel } from "../../shared/domain";
|
||||
import { createId } from "../../shared/ids";
|
||||
import type { CreateAgentInput, CreateGoalInput, CreateOrganizationInput, CreateProjectInput, CreateTaskInput, CreateWorkspaceInput, DeleteTaskInput, UpdateAgentInput, UpdateEmployeeProfileInput, UpdateTaskInput } from "../../shared/commands";
|
||||
import { AuditService } from "./audit-service";
|
||||
@@ -12,6 +12,7 @@ import { TaskMutations } from "./task-mutations";
|
||||
import { normalizeAvatarColor, normalizeAvatarImage } from "./avatar-values";
|
||||
import { approvalDecision, autonomy } from "./org-approvals";
|
||||
import { admitAgent } from "./agent-admission";
|
||||
import { TASK_APPROVAL_KINDS_SQL, TASK_SNAPSHOT_SQL } from "./task-approval";
|
||||
|
||||
export class OrganizationService {
|
||||
private readonly audit: AuditService;
|
||||
@@ -23,7 +24,7 @@ export class OrganizationService {
|
||||
this.audit = new AuditService(db);
|
||||
this.policy = new PolicyService(db);
|
||||
this.governance = new OrgGovernanceService(db);
|
||||
this.tasks = new TaskMutations(db, (input) => this.createTask(input));
|
||||
this.tasks = new TaskMutations(db, (input) => this.createTask(input), (orgId, taskId) => this.requestTaskApproval(orgId, taskId));
|
||||
}
|
||||
|
||||
createOrganization(input: CreateOrganizationInput): string {
|
||||
@@ -262,35 +263,7 @@ export class OrganizationService {
|
||||
now,
|
||||
);
|
||||
const policy = this.policy.evaluateTaskCreation(input.orgId, input.risk);
|
||||
const approvalRequired = policy.approvalRequired || input.requiresFilesystem;
|
||||
if (approvalRequired) {
|
||||
const approvalId = createId("app");
|
||||
const approvalKind = policy.approvalRequired ? "task_risk" : "task_execution";
|
||||
const autoApprove = autonomy(this.db, input.orgId).executionMode === "ai_ceo";
|
||||
const decision = approvalDecision(this.db, input.orgId, autoApprove, now);
|
||||
this.db.prepare(`
|
||||
insert into approvals(
|
||||
id, org_id, kind, title, details, risk, requested_by_type,
|
||||
requested_by_id, source_entity_type, source_entity_id, state, decided_by_employee_id, created_at, decided_at
|
||||
) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
`).run(
|
||||
approvalId,
|
||||
input.orgId,
|
||||
approvalKind,
|
||||
input.title.trim(),
|
||||
input.description.trim(),
|
||||
RISK_LEVEL.HIGH,
|
||||
"system",
|
||||
null,
|
||||
"task",
|
||||
id,
|
||||
decision.state,
|
||||
decision.decidedBy,
|
||||
now,
|
||||
decision.decidedAt,
|
||||
);
|
||||
this.settleApproval(input.orgId, approvalId, approvalKind, RISK_LEVEL.HIGH, autoApprove);
|
||||
}
|
||||
this.requestTaskApproval(input.orgId, id);
|
||||
this.audit.record({
|
||||
orgId: input.orgId,
|
||||
entityType: "task",
|
||||
@@ -303,6 +276,37 @@ export class OrganizationService {
|
||||
return id;
|
||||
}
|
||||
|
||||
/** Asks for approval of a task's current content when policy or file access needs it and no review already covers that content. */
|
||||
requestTaskApproval(orgId: string, taskId: string): void {
|
||||
const task = this.db.prepare(`select t.title, t.description, t.risk, t.requires_filesystem, ${TASK_SNAPSHOT_SQL} as snapshot from tasks t where t.id = ? and t.org_id = ?`)
|
||||
.get(taskId, orgId) as { title: string; description: string; risk: RiskLevel; requires_filesystem: number; snapshot: string };
|
||||
const policy = this.policy.evaluateTaskCreation(orgId, task.risk);
|
||||
if (!policy.approvalRequired && task.requires_filesystem !== 1) return;
|
||||
const latest = this.db.prepare(`
|
||||
select subject_snapshot from approvals
|
||||
where org_id = ? and source_entity_type = 'task' and source_entity_id = ? and kind in ${TASK_APPROVAL_KINDS_SQL}
|
||||
order by rowid desc limit 1
|
||||
`).get(orgId, taskId) as { subject_snapshot: string | null } | undefined;
|
||||
if (latest?.subject_snapshot === task.snapshot) return;
|
||||
const superseded = this.db.prepare(`
|
||||
update approvals set state = ? where org_id = ? and source_entity_type = 'task' and source_entity_id = ? and kind in ${TASK_APPROVAL_KINDS_SQL} and state = ?
|
||||
`).run(APPROVAL_STATE.SUPERSEDED, orgId, taskId, APPROVAL_STATE.PENDING);
|
||||
if (superseded.changes > 0) console.info(`[approvals] task ${taskId} changed; its pending review was superseded`);
|
||||
const now = new Date().toISOString();
|
||||
const approvalId = createId("app");
|
||||
const approvalKind = policy.approvalRequired ? "task_risk" : "task_execution";
|
||||
const autoApprove = autonomy(this.db, orgId).executionMode === "ai_ceo";
|
||||
const decision = approvalDecision(this.db, orgId, autoApprove, now);
|
||||
this.db.prepare(`
|
||||
insert into approvals(
|
||||
id, org_id, kind, title, details, risk, requested_by_type, requested_by_id, source_entity_type, source_entity_id,
|
||||
state, decided_by_employee_id, created_at, decided_at, subject_snapshot
|
||||
) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
`).run(approvalId, orgId, approvalKind, task.title, task.description, RISK_LEVEL.HIGH, "system", null, "task", taskId,
|
||||
decision.state, decision.decidedBy, now, decision.decidedAt, task.snapshot);
|
||||
this.settleApproval(orgId, approvalId, approvalKind, RISK_LEVEL.HIGH, autoApprove);
|
||||
}
|
||||
|
||||
/** Routes a pending approval to its decider, or records the auto-approval in the audit trail. */
|
||||
private settleApproval(orgId: string, approvalId: string, kind: string, risk: typeof RISK_LEVEL[keyof typeof RISK_LEVEL], autoApproved: boolean): void {
|
||||
if (!autoApproved) {
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
/** The task content an approval covers, as SQL over a `tasks t` row; one expression for recording and checking keeps them comparable. */
|
||||
export const TASK_SNAPSHOT_SQL = "json_object('title', t.title, 'description', t.description, 'assignee', t.assignee_agent_id, 'risk', t.risk, 'workspace', t.workspace_id, 'filesystem', t.requires_filesystem)";
|
||||
|
||||
export const TASK_APPROVAL_KINDS_SQL = "('task_risk', 'task_execution')";
|
||||
@@ -23,7 +23,12 @@ interface TaskRow {
|
||||
export class TaskMutations {
|
||||
private readonly audit: AuditService;
|
||||
|
||||
constructor(private readonly db: DatabaseSync, private readonly createTask: (input: CreateTaskInput) => string) {
|
||||
constructor(
|
||||
private readonly db: DatabaseSync,
|
||||
private readonly createTask: (input: CreateTaskInput) => string,
|
||||
/** Asks for a fresh approval when an edit changed content an approval covered. */
|
||||
private readonly requestApproval: (orgId: string, taskId: string) => void,
|
||||
) {
|
||||
this.audit = new AuditService(db);
|
||||
}
|
||||
|
||||
@@ -48,6 +53,7 @@ export class TaskMutations {
|
||||
where id = ? and org_id = ?
|
||||
`).run(input.title.trim(), input.description.trim(), input.assigneeAgentId, input.risk, input.priority, input.parentTaskId, now, input.taskId, input.orgId);
|
||||
if (result.changes === 0) throw new Error("Task was not found.");
|
||||
this.requestApproval(input.orgId, input.taskId);
|
||||
this.audit.record({
|
||||
orgId: input.orgId,
|
||||
entityType: "task",
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { TASK_APPROVAL_KINDS_SQL, TASK_SNAPSHOT_SQL } from "./task-approval";
|
||||
import type { DatabaseSync } from "node:sqlite";
|
||||
import { APPROVAL_STATE, RISK_LEVEL } from "../../shared/domain";
|
||||
import { createId } from "../../shared/ids";
|
||||
@@ -11,11 +12,6 @@ import type {
|
||||
WorkspaceRow,
|
||||
} from "./task-prompts";
|
||||
|
||||
interface TaskApprovalRow {
|
||||
id: string;
|
||||
state: string;
|
||||
}
|
||||
|
||||
export function requireTask(db: DatabaseSync, orgId: string, taskId: string): TaskExecutionTaskRow {
|
||||
const task = db.prepare("select * from tasks where id = ? and org_id = ?").get(taskId, orgId) as TaskExecutionTaskRow | undefined;
|
||||
if (!task) throw new Error("Task was not found.");
|
||||
@@ -45,16 +41,14 @@ export function assertExecutionApproval(db: DatabaseSync, orgId: string, task: T
|
||||
export function executionApprovalProblem(db: DatabaseSync, orgId: string, task: TaskExecutionTaskRow): string | null {
|
||||
if (task.risk !== RISK_LEVEL.HIGH && task.requires_filesystem !== 1) return null;
|
||||
const approval = db.prepare(`
|
||||
select id, state
|
||||
from approvals
|
||||
where org_id = ?
|
||||
and source_entity_type = 'task'
|
||||
and source_entity_id = ?
|
||||
and kind in ('task_risk', 'task_execution')
|
||||
order by created_at desc
|
||||
select a.state, a.subject_snapshot = (select ${TASK_SNAPSHOT_SQL} from tasks t where t.id = a.source_entity_id) as covers_current
|
||||
from approvals a
|
||||
where a.org_id = ? and a.source_entity_type = 'task' and a.source_entity_id = ? and a.kind in ${TASK_APPROVAL_KINDS_SQL}
|
||||
order by a.rowid desc
|
||||
limit 1
|
||||
`).get(orgId, task.id) as TaskApprovalRow | undefined;
|
||||
`).get(orgId, task.id) as { state: string; covers_current: number | null } | undefined;
|
||||
if (!approval || approval.state !== APPROVAL_STATE.APPROVED) return "Approved task execution approval is required.";
|
||||
if (approval.covers_current !== 1) return "The task changed after it was approved; the changed task needs a fresh approval.";
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
@@ -387,6 +387,7 @@ export const zh: Record<string, string> = {
|
||||
"Account label, e.g. sales": "账号标签,例如 sales",
|
||||
"Add account": "添加账号",
|
||||
"limited": "受限",
|
||||
"Replaced by an edit": "已因编辑被替换",
|
||||
"Revoke {name}": "撤销 {name}",
|
||||
"Grant {name} to": "把 {name} 授予",
|
||||
"Grant to…": "授予给…",
|
||||
|
||||
@@ -276,7 +276,8 @@ function DecidedRow({
|
||||
last: boolean;
|
||||
}) {
|
||||
const approved = approval.state === APPROVAL_STATE.APPROVED;
|
||||
const decidedLabel = approved ? t("Approved") : t("Declined");
|
||||
const superseded = approval.state === APPROVAL_STATE.SUPERSEDED;
|
||||
const decidedLabel = approved ? t("Approved") : superseded ? t("Replaced by an edit") : t("Declined");
|
||||
return (
|
||||
<div
|
||||
className="row"
|
||||
@@ -287,11 +288,11 @@ function DecidedRow({
|
||||
borderBottom: last ? "none" : "1px solid var(--border-soft)",
|
||||
}}
|
||||
>
|
||||
<Icon name={approved ? "check" : "x"} size={14} color={approved ? "var(--green)" : "var(--red)"} />
|
||||
<Icon name={approved ? "check" : "x"} size={14} color={approved ? "var(--green)" : superseded ? "var(--text-3)" : "var(--red)"} />
|
||||
<div style={{ flex: 1, fontWeight: 480, overflow: "hidden", textOverflow: "ellipsis", whiteSpace: "nowrap" }}>{approval.title}</div>
|
||||
<Avatar name={requester} size="sm" bot />
|
||||
<div style={{ fontSize: 12, color: "var(--text-3)", width: 180, textAlign: "right", whiteSpace: "nowrap", overflow: "hidden", textOverflow: "ellipsis" }}>
|
||||
{decidedLabel} · {formatRelative(approval.decidedAt ?? approval.createdAt)} · {decider.split(" ")[0]}
|
||||
{decidedLabel} · {formatRelative(approval.decidedAt ?? approval.createdAt)}{superseded ? "" : ` · ${decider.split(" ")[0]}`}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -16,7 +16,8 @@ export { SEARCH_ENTITY_TYPES, type SearchEntityType, type SearchResultRecord } f
|
||||
export type { BrowserActionPolicyRecord, BrowserActionRecord, BrowserSessionContainerRecord, SocialAccountGrantRecord, SocialAccountRecord, SocialApprovalRouteRecord, SocialAuditRecord } from "./social-domain";
|
||||
export type { AppSnapshot, UiLanguage, UiPreferences, UiTheme } from "./snapshot-domain";
|
||||
export const TASK_STATUS = { BACKLOG: "backlog", IN_PROGRESS: "in_progress", REVIEW: "review", DONE: "done" } as const;
|
||||
export const APPROVAL_STATE = { PENDING: "pending", APPROVED: "approved", DECLINED: "declined" } as const;
|
||||
/** Superseded: a pending review replaced by one for newer content. */
|
||||
export const APPROVAL_STATE = { PENDING: "pending", APPROVED: "approved", DECLINED: "declined", SUPERSEDED: "superseded" } as const;
|
||||
export const RISK_LEVEL = { LOW: "low", MEDIUM: "medium", HIGH: "high" } as const;
|
||||
export const TASK_PRIORITY = { LOW: "low", NORMAL: "normal", HIGH: "high" } as const;
|
||||
/** How a task's output should read: conversational prose carries house style; technical and structured do not. */
|
||||
|
||||
@@ -24,7 +24,7 @@ describe("domain services", () => {
|
||||
const connection = createConnection();
|
||||
|
||||
const rows = connection.db.prepare("select version from schema_migrations order by version asc").all() as Array<{ version: number }>;
|
||||
expect(rows.map((row) => row.version)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84]);
|
||||
expect(rows.map((row) => row.version)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85]);
|
||||
connection.close();
|
||||
});
|
||||
|
||||
|
||||
@@ -8,7 +8,8 @@ import { OrganizationService } from "../../src/main/services/organization-servic
|
||||
import { ChatService } from "../../src/main/services/chat-service";
|
||||
import { dispatchAgentTool, type AgentToolContext } from "../../src/main/services/agent-tools";
|
||||
import { ApprovalService } from "../../src/main/services/approval-service";
|
||||
import { requireAssignedAgent } from "../../src/main/services/task-run-preparation";
|
||||
import { executionApprovalProblem, requireAssignedAgent } from "../../src/main/services/task-run-preparation";
|
||||
import { TASK_PRIORITY } from "../../src/shared/domain";
|
||||
import type { TaskExecutionTaskRow } from "../../src/main/services/task-prompts";
|
||||
|
||||
function call(name: string, args: Record<string, unknown>) {
|
||||
@@ -153,4 +154,40 @@ describe("execution modes", () => {
|
||||
expect([lifecycle(bo), identities(bo), memberships(bo)]).toEqual(["declined", 0, 0]);
|
||||
expect(() => runAs(bo)).toThrow("Bo's hire was declined; reassign the task.");
|
||||
});
|
||||
|
||||
test("a task approval covers the content it was given; an edit needs a fresh one", () => {
|
||||
const h = harness("ai_cofounder");
|
||||
const db = h.connection.db;
|
||||
const employeeId = (db.prepare("select id from employees where org_id = ?").get(h.orgId) as { id: string }).id;
|
||||
const approvals = new ApprovalService(db);
|
||||
const taskId = h.connection.transaction(() => h.deps.organization.createTask({
|
||||
orgId: h.orgId, title: "Wipe staging", description: "Drop the staging tables.", status: "backlog",
|
||||
assigneeAgentId: h.ceoId, risk: "high", projectId: null, goalId: null, workspaceId: null, priority: TASK_PRIORITY.NORMAL, requiresFilesystem: false,
|
||||
}));
|
||||
const task = () => db.prepare("select * from tasks where id = ?").get(taskId) as unknown as Parameters<typeof executionApprovalProblem>[2];
|
||||
const taskApprovals = () => db.prepare("select id, state, details from approvals where source_entity_id = ? order by rowid").all(taskId) as Array<{ id: string; state: string; details: string }>;
|
||||
const edit = (description: string) => h.connection.transaction(() => h.deps.organization.updateTask({
|
||||
orgId: h.orgId, taskId, title: "Wipe staging", description, assigneeAgentId: h.ceoId, risk: "high", priority: TASK_PRIORITY.NORMAL, parentTaskId: null,
|
||||
}));
|
||||
const decide = (id: string) => h.connection.transaction(() => approvals.decide({ orgId: h.orgId, approvalId: id, state: "approved", decidedByEmployeeId: employeeId }));
|
||||
|
||||
// Editing while the review waits replaces it with one for the new content.
|
||||
edit("Drop the staging tables, then reseed.");
|
||||
expect(taskApprovals().map((row) => [row.state, row.details])).toEqual([
|
||||
["superseded", "Drop the staging tables."],
|
||||
["pending", "Drop the staging tables, then reseed."],
|
||||
]);
|
||||
decide(taskApprovals()[1].id);
|
||||
expect(executionApprovalProblem(db, h.orgId, task())).toBeNull();
|
||||
|
||||
// An approved task edited afterwards is not covered by the old approval.
|
||||
edit("Drop the production tables.");
|
||||
expect(executionApprovalProblem(db, h.orgId, task())).toBe("Approved task execution approval is required.");
|
||||
expect(taskApprovals().map((row) => row.state)).toEqual(["superseded", "approved", "pending"]);
|
||||
// A save that changes nothing approved keeps the approval.
|
||||
decide(taskApprovals()[2].id);
|
||||
edit("Drop the production tables.");
|
||||
expect(taskApprovals()).toHaveLength(3);
|
||||
expect(executionApprovalProblem(db, h.orgId, task())).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -67,7 +67,7 @@ describe("task review", () => {
|
||||
test("a failed review hand-off leaves the task where it was; deleting reviewed work closes its review", () => {
|
||||
const h = setup();
|
||||
const taskId = h.task(h.devId, "Login page");
|
||||
const failing = new TaskMutations(h.connection.db, () => { throw new Error("review task could not be created"); });
|
||||
const failing = new TaskMutations(h.connection.db, () => { throw new Error("review task could not be created"); }, () => undefined);
|
||||
expect(() => failing.updateStatus(h.orgId, taskId, TASK_STATUS.REVIEW)).toThrow("review task could not be created");
|
||||
expect(h.connection.db.prepare("select status from tasks where id = ?").get(taskId)).toEqual({ status: "in_progress" });
|
||||
|
||||
|
||||
Reference in new issue
Block a user