From 1f55bff862e0cc7c1cd51514e487f9f5066da4c4 Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 11:04:47 -0400 Subject: [PATCH 01/25] feat: define Codex delegation templates --- src/codex/delegation-agents-block.ts | 130 ++++++++++++++++++ src/codex/delegation-templates.ts | 65 +++++++++ src/skills/codexcommander-delegation/SKILL.md | 41 ++++++ tests/codex-delegation-templates.test.ts | 101 ++++++++++++++ 4 files changed, 337 insertions(+) create mode 100644 src/codex/delegation-agents-block.ts create mode 100644 src/codex/delegation-templates.ts create mode 100644 src/skills/codexcommander-delegation/SKILL.md create mode 100644 tests/codex-delegation-templates.test.ts diff --git a/src/codex/delegation-agents-block.ts b/src/codex/delegation-agents-block.ts new file mode 100644 index 0000000000..b939565411 --- /dev/null +++ b/src/codex/delegation-agents-block.ts @@ -0,0 +1,130 @@ +import { + DELEGATION_BEGIN_MARKER, + DELEGATION_END_MARKER, + type CodexDelegationMode, +} from "./delegation-templates"; + +export type DelegationAgentsInspection = + | { kind: "absent" } + | { + kind: "managed"; + start: number; + end: number; + content: string; + mode: CodexDelegationMode | null; + version: number | null; + } + | { kind: "conflict"; reason: "orphan_begin" | "orphan_end" | "duplicate" | "reversed" | "malformed_marker" }; + +interface PhysicalLine { + start: number; + end: number; + normalized: string; +} + +function physicalLines(content: string): PhysicalLine[] { + const lines: PhysicalLine[] = []; + let start = 0; + + for (let index = 0; index <= content.length; index += 1) { + if (index !== content.length && content[index] !== "\n") continue; + const end = index; + const raw = content.slice(start, end); + lines.push({ + start, + end: raw.endsWith("\r") ? end - 1 : end, + normalized: raw.endsWith("\r") ? raw.slice(0, -1) : raw, + }); + start = index + 1; + } + + return lines; +} + +function markerConflict(content: string, lines: PhysicalLine[]): DelegationAgentsInspection | null { + for (const line of lines) { + if ( + (line.normalized.includes(DELEGATION_BEGIN_MARKER) && line.normalized !== DELEGATION_BEGIN_MARKER) + || (line.normalized.includes(DELEGATION_END_MARKER) && line.normalized !== DELEGATION_END_MARKER) + ) { + return { kind: "conflict", reason: "malformed_marker" }; + } + } + + return null; +} + +function detectEol(content: string): "\n" | "\r\n" { + return content.includes("\r\n") ? "\r\n" : "\n"; +} + +function normalizeBlockEol(block: string, eol: "\n" | "\r\n"): string { + return block.replace(/\r?\n/g, eol).replace(/(?:\r?\n)+$/, ""); +} + +function readMode(content: string): CodexDelegationMode | null { + const match = /^Mode: (balanced|orchestrator)\r?$/m.exec(content); + return match?.[1] as CodexDelegationMode | undefined ?? null; +} + +function readVersion(content: string): number | null { + const match = /^\r?$/m.exec(content); + return match === null ? null : Number(match[1]); +} + +export function inspectDelegationAgentsBlock(content: string): DelegationAgentsInspection { + const lines = physicalLines(content); + const malformed = markerConflict(content, lines); + if (malformed !== null) return malformed; + + const begins = lines.filter((line) => line.normalized === DELEGATION_BEGIN_MARKER); + const ends = lines.filter((line) => line.normalized === DELEGATION_END_MARKER); + if (begins.length === 0 && ends.length === 0) return { kind: "absent" }; + if (begins.length === 0) return { kind: "conflict", reason: "orphan_end" }; + if (ends.length === 0) return { kind: "conflict", reason: "orphan_begin" }; + if (begins.length !== 1 || ends.length !== 1) return { kind: "conflict", reason: "duplicate" }; + + const begin = begins[0]; + const end = ends[0]; + if (end.start < begin.start) return { kind: "conflict", reason: "reversed" }; + + const managedContent = content.slice(begin.start, end.end); + return { + kind: "managed", + start: begin.start, + end: end.end, + content: managedContent, + mode: readMode(managedContent), + version: readVersion(managedContent), + }; +} + +export function upsertDelegationAgentsBlock(content: string, block: string): { content: string; changed: boolean } { + const inspection = inspectDelegationAgentsBlock(content); + if (inspection.kind === "conflict") return { content, changed: false }; + + const normalizedBlock = normalizeBlockEol(block, detectEol(content)); + if (inspection.kind === "managed") { + if (inspection.content === normalizedBlock) return { content, changed: false }; + return { + content: `${content.slice(0, inspection.start)}${normalizedBlock}${content.slice(inspection.end)}`, + changed: true, + }; + } + + if (content.length === 0) return { content: normalizedBlock, changed: true }; + const separator = content.endsWith("\n") ? "" : detectEol(content); + return { content: `${content}${separator}${normalizedBlock}`, changed: true }; +} + +export function removeDelegationAgentsBlock(content: string): { content: string; changed: boolean } { + const inspection = inspectDelegationAgentsBlock(content); + if (inspection.kind !== "managed") return { content, changed: false }; + + const suffix = content.slice(inspection.end); + const separator = suffix.startsWith("\r\n") ? 2 : suffix.startsWith("\n") ? 1 : 0; + return { + content: `${content.slice(0, inspection.start)}${suffix.slice(separator)}`, + changed: true, + }; +} diff --git a/src/codex/delegation-templates.ts b/src/codex/delegation-templates.ts new file mode 100644 index 0000000000..41ff051bce --- /dev/null +++ b/src/codex/delegation-templates.ts @@ -0,0 +1,65 @@ +import { readFileSync } from "node:fs"; + +export type CodexDelegationMode = "balanced" | "orchestrator"; + +export const CODEX_DELEGATION_SCHEMA_VERSION = 1 as const; +export const DELEGATION_BEGIN_MARKER = ""; +export const DELEGATION_END_MARKER = ""; + +export interface CodexDelegationBundle { + mode: CodexDelegationMode; + skillText: string; + agentsBlockText: string; + copyPrompt: string; +} + +const SKILL_URL = new URL("../skills/codexcommander-delegation/SKILL.md", import.meta.url); + +function canonicalSkillText(): string { + return readFileSync(SKILL_URL, "utf8"); +} + +function agentsBlock(mode: CodexDelegationMode): string { + const delegationSentence = mode === "balanced" + ? "Delegate substantial bounded parallel work when it will clearly help; the root may still implement and must synthesize." + : "The root delegates research and implementation, and focuses on decomposition, coordination, review, and synthesis. Work directly only when delegation is unavailable or clearly wasteful."; + + return [ + DELEGATION_BEGIN_MARKER, + ``, + "## CodexCommander delegation", + `Mode: ${mode}`, + "Before spawning subagents, use $codexcommander-delegation and reread its SKILL.md if its details were compacted.", + "Consult the active collaboration roster and spawn-tool contract. Never hardcode model IDs. Give workers self-contained tasks.", + delegationSentence, + "This guidance is advisory and does not create collaboration tools. User and repository instructions outrank it for whether to spawn.", + DELEGATION_END_MARKER, + ].join("\n"); +} + +export function renderCodexDelegationBundle(mode: CodexDelegationMode): CodexDelegationBundle { + const skillText = canonicalSkillText(); + const agentsBlockText = agentsBlock(mode); + const copyPrompt = [ + "Preview the following two writes exactly, then wait for my approval before changing any files.", + "", + "SKILL.md:", + skillText, + "", + "AGENTS.md block:", + agentsBlockText, + ].join("\n"); + + return { mode, skillText, agentsBlockText, copyPrompt }; +} + +export function isCodexCommanderManagedSkill(content: string): boolean { + const frontmatter = /^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/.exec(content)?.[1]; + if (frontmatter === undefined) return false; + + return [ + /^name:\s*codexcommander-delegation\s*\r?$/m, + /^\s{2}managed-by:\s*codexcommander\s*\r?$/m, + /^\s{2}managed-version:\s*"1"\s*\r?$/m, + ].every((pattern) => pattern.test(frontmatter)); +} diff --git a/src/skills/codexcommander-delegation/SKILL.md b/src/skills/codexcommander-delegation/SKILL.md new file mode 100644 index 0000000000..7d41218483 --- /dev/null +++ b/src/skills/codexcommander-delegation/SKILL.md @@ -0,0 +1,41 @@ +--- +name: codexcommander-delegation +description: Route and coordinate Codex multi-agent work through the live CodexCommander collaboration roster without hardcoding model IDs. Use when spawning subagents, splitting independent implementation or research, or coordinating parallel workers. Do not use for trivial single-step edits, ordinary Q&A, or sessions with no collaboration tools. +metadata: + managed-by: codexcommander + managed-version: "1" +--- + +# CodexCommander delegation + +## Read the live contract first + +Inspect the current `spawn_agent` schema and its companion collaboration tools before acting. Use only tools present in this session. The live injected collaboration guidance and tool contract win over this skill. + +## Honor the installed mode + +Read the CodexCommander global block in the applicable `AGENTS.md`. If that block is unavailable, use Balanced as the safe fallback. + +## When to delegate + +Balanced delegates substantial, bounded, independent work when that will clearly help. Orchestrator delegates research and implementation, while allowing direct work when delegation is unavailable or clearly wasteful. + +## Spawn contract + +Give every child a self-contained brief with its goal, paths or inputs, owned files, constraints, checks, and required output format. Do not assume a child inherits this transcript or this skill. + +## Model and effort + +Use only model IDs and effort levels advertised live. Prefer the current preferred worker when it fits. Omit overrides when the roster is stale, unknown, or uncertain, and never remember IDs. + +## Coordination + +The root verifies and synthesizes the result. Use bounded waits; do not invent an ACK or PING ritual. + +## Compaction + +If details were compacted, reread this skill and the live tool contract before the next spawn. + +## Do not + +Do not hardcode a roster, invent tools, claim delegation is forced, or expand scope beyond the user and repository instructions. diff --git a/tests/codex-delegation-templates.test.ts b/tests/codex-delegation-templates.test.ts new file mode 100644 index 0000000000..d05cddbd57 --- /dev/null +++ b/tests/codex-delegation-templates.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, test } from "bun:test"; +import { + DELEGATION_BEGIN_MARKER, + DELEGATION_END_MARKER, + isCodexCommanderManagedSkill, + renderCodexDelegationBundle, +} from "../src/codex/delegation-templates"; +import { + inspectDelegationAgentsBlock, + removeDelegationAgentsBlock, + upsertDelegationAgentsBlock, +} from "../src/codex/delegation-agents-block"; + +describe("Codex delegation templates", () => { + test("balanced is deterministic and carries no roster ids", () => { + const first = renderCodexDelegationBundle("balanced"); + const second = renderCodexDelegationBundle("balanced"); + expect(first).toEqual(second); + expect(first.skillText).toContain("name: codexcommander-delegation"); + expect(first.skillText).toContain("managed-by: codexcommander"); + expect(first.agentsBlockText).toContain("Mode: balanced"); + expect(first.copyPrompt).toContain(first.skillText); + expect(first.copyPrompt).toContain(first.agentsBlockText); + for (const frozenId of ["gpt-5.6", "kimi/", "xai/", "grok-4.6"]) { + expect(`${first.skillText}\n${first.agentsBlockText}`).not.toContain(frozenId); + } + }); + + test("orchestrator delegates execution but preserves wasteful-work exceptions", () => { + const bundle = renderCodexDelegationBundle("orchestrator"); + expect(bundle.agentsBlockText).toContain("Mode: orchestrator"); + expect(bundle.agentsBlockText).toContain("clearly wasteful"); + expect(bundle.agentsBlockText).toContain("review"); + expect(bundle.agentsBlockText).toContain("synthesis"); + }); + + test("managed skill ownership is carried by SKILL.md itself", () => { + const skill = renderCodexDelegationBundle("balanced").skillText; + expect(isCodexCommanderManagedSkill(skill)).toBe(true); + expect(isCodexCommanderManagedSkill(skill.replace("managed-by: codexcommander", "managed-by: someone-else"))).toBe(false); + }); +}); + +describe("Codex delegation AGENTS.md block transforms", () => { + const balancedBlock = renderCodexDelegationBundle("balanced").agentsBlockText; + const orchestratorBlock = renderCodexDelegationBundle("orchestrator").agentsBlockText; + + test("reports an absent block and inserts it as a separate final block", () => { + expect(inspectDelegationAgentsBlock("# Project\n")).toEqual({ kind: "absent" }); + expect(upsertDelegationAgentsBlock("# Project\n", balancedBlock)).toEqual({ + content: `# Project\n${balancedBlock}`, + changed: true, + }); + }); + + test("inserts into an empty file and preserves a missing final newline", () => { + expect(upsertDelegationAgentsBlock("", balancedBlock)).toEqual({ + content: balancedBlock, + changed: true, + }); + expect(upsertDelegationAgentsBlock("# Project", balancedBlock)).toEqual({ + content: `# Project\n${balancedBlock}`, + changed: true, + }); + }); + + test("updates a managed block idempotently and changes only the managed region", () => { + const source = `before\n${balancedBlock}\nafter\n`; + expect(upsertDelegationAgentsBlock(source, balancedBlock)).toEqual({ content: source, changed: false }); + expect(upsertDelegationAgentsBlock(source, orchestratorBlock)).toEqual({ + content: `before\n${orchestratorBlock}\nafter\n`, + changed: true, + }); + }); + + test("preserves CRLF and all prefix and suffix bytes", () => { + const crlfBlock = balancedBlock.replaceAll("\n", "\r\n"); + const source = `prefix\r\n${crlfBlock}\r\nsuffix\r\n`; + expect(inspectDelegationAgentsBlock(source)).toMatchObject({ kind: "managed", mode: "balanced", version: 1 }); + expect(upsertDelegationAgentsBlock(source, crlfBlock)).toEqual({ content: source, changed: false }); + expect(removeDelegationAgentsBlock(source)).toEqual({ content: "prefix\r\nsuffix\r\n", changed: true }); + }); + + test("removes only its immediately introduced separators", () => { + expect(removeDelegationAgentsBlock(balancedBlock)).toEqual({ content: "", changed: true }); + expect(removeDelegationAgentsBlock(`before\n${balancedBlock}`)).toEqual({ content: "before\n", changed: true }); + }); + + test.each([ + ["duplicate", `${balancedBlock}\n${balancedBlock}\n`], + ["orphan begin", `${DELEGATION_BEGIN_MARKER}\n# Project\n`], + ["orphan end", `# Project\n${DELEGATION_END_MARKER}\n`], + ["reversed", `${DELEGATION_END_MARKER}\n# Project\n${DELEGATION_BEGIN_MARKER}\n`], + ["begin substring", `prefix ${DELEGATION_BEGIN_MARKER}\n`], + ["end substring", `prefix ${DELEGATION_END_MARKER}\n`], + ])("refuses %s marker conflicts without changing the input", (_name, source) => { + expect(inspectDelegationAgentsBlock(source).kind).toBe("conflict"); + expect(upsertDelegationAgentsBlock(source, balancedBlock)).toEqual({ content: source, changed: false }); + expect(removeDelegationAgentsBlock(source)).toEqual({ content: source, changed: false }); + }); +}); From 06691dc65c707f090514084a30dad723cb046a2e Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 11:40:44 -0400 Subject: [PATCH 02/25] feat: safely manage Codex delegation setup --- src/codex/delegation-installer.ts | 680 +++++++++++++++++++++++ src/lib/test-home-guard.ts | 19 +- tests/codex-delegation-installer.test.ts | 421 ++++++++++++++ tests/test-home-guard.test.ts | 36 +- 4 files changed, 1148 insertions(+), 8 deletions(-) create mode 100644 src/codex/delegation-installer.ts create mode 100644 tests/codex-delegation-installer.test.ts diff --git a/src/codex/delegation-installer.ts b/src/codex/delegation-installer.ts new file mode 100644 index 0000000000..03d1d43a95 --- /dev/null +++ b/src/codex/delegation-installer.ts @@ -0,0 +1,680 @@ +import { + closeSync, + constants as fsConstants, + fchmodSync, + fstatSync, + fsyncSync, + lstatSync, + mkdirSync, + openSync, + readSync, + realpathSync, + rmdirSync, + unlinkSync, + writeFileSync, + type BigIntStats, +} from "node:fs"; +import { homedir } from "node:os"; +import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; +import { renameAtomicFile } from "../config"; +import { assertNotRealHomeUnderTest } from "../lib/test-home-guard"; +import { + inspectDelegationAgentsBlock, + removeDelegationAgentsBlock, + upsertDelegationAgentsBlock, + type DelegationAgentsInspection, +} from "./delegation-agents-block"; +import { + isCodexCommanderManagedSkill, + renderCodexDelegationBundle, + type CodexDelegationMode, +} from "./delegation-templates"; +import { getCodexHome } from "./paths"; + +export type DelegationArtifactState = "absent" | "current" | "outdated" | "foreign" | "unsafe"; +export type DelegationActivation = "effective" | "shadowed" | "unknown"; + +export interface DelegationArtifactStatus { + state: DelegationArtifactState; + displayPath: "$HOME/.agents/skills/codexcommander-delegation/SKILL.md" | "$CODEX_HOME/AGENTS.md"; + reason?: string; +} + +export interface CodexDelegationStatus { + schemaVersion: 1; + state: "not-installed" | "current" | "update-available" | "partial" | "conflict" | "unsafe"; + installedMode: CodexDelegationMode | null; + artifacts: { skill: DelegationArtifactStatus; agentsPolicy: DelegationArtifactStatus }; + override: { state: "absent" | "empty" | "active" | "unsafe" }; + activation: DelegationActivation; + previews: Record; + copyPrompts: Record; +} + +export type CodexDelegationMutation = + | { action: "install"; mode: CodexDelegationMode } + | { action: "uninstall" }; + +export type CodexDelegationMutationOutcome = + | { ok: true; changed: boolean; status: CodexDelegationStatus } + | { ok: false; changed: boolean; reason: MutationFailureReason; status: CodexDelegationStatus }; + +type MutationFailureReason = + | "foreign_skill" + | "ambiguous_agents_markers" + | "unsafe_path" + | "unreadable" + | "invalid_utf8" + | "too_large" + | "changed_during_mutation" + | "mutation_busy" + | "write_failed" + | "partial_write"; + +export interface CodexDelegationInstallerDeps { + userHome?: string; + codexHome?: string; + beforePublish?: (artifact: "skill" | "agents") => void; +} + +const SKILL_LIMIT = 256 * 1024; +const AGENTS_LIMIT = 1024 * 1024; +const SKILL_DISPLAY = "$HOME/.agents/skills/codexcommander-delegation/SKILL.md" as const; +const AGENTS_DISPLAY = "$CODEX_HOME/AGENTS.md" as const; +let mutationInProgress = false; +let tempSequence = 0; + +class DelegationFsError extends Error { + constructor(readonly reason: MutationFailureReason, message: string, readonly published = false, options?: ErrorOptions) { + super(message, options); + this.name = "DelegationFsError"; + } +} + +interface Paths { + userHome: string; + codexHome: string; + skillDir: string; + skillPath: string; + compatibilitySkillPath: string; + agentsPath: string; + overridePath: string; +} + +interface FileSnapshotAbsent { kind: "absent" } +interface FileSnapshotPresent { + kind: "file"; + bytes: Buffer; + text: string; + stat: BigIntStats; +} +type FileSnapshot = FileSnapshotAbsent | FileSnapshotPresent; + +interface InspectionContext { + paths: Paths; + skill: FileSnapshot; + agents: FileSnapshot; + agentsInspection: DelegationAgentsInspection; + status: CodexDelegationStatus; +} + +interface AppliedMutation { + artifact: "skill" | "agents"; + path: string; + before: FileSnapshot; + after: FileSnapshot; +} + +const ABSENT: FileSnapshotAbsent = { kind: "absent" }; + +function errorCode(error: unknown): string | undefined { + return (error as NodeJS.ErrnoException | undefined)?.code; +} + +function samePath(left: string, right: string): boolean { + const normalize = (path: string): string => process.platform === "win32" ? resolve(path).toLowerCase() : resolve(path); + return normalize(left) === normalize(right); +} + +function sameIdentity(left: BigIntStats, right: BigIntStats): boolean { + return left.dev === right.dev + && left.ino === right.ino + && left.nlink === right.nlink + && left.mode === right.mode + && left.size === right.size + && left.mtimeNs === right.mtimeNs + && left.ctimeNs === right.ctimeNs; +} + +function sameDirectoryIdentity(left: BigIntStats, right: BigIntStats): boolean { + return left.isDirectory() && right.isDirectory() + && left.dev === right.dev + && left.ino === right.ino + && left.nlink === right.nlink + && left.mode === right.mode; +} + +function sameDirectoryObject(left: BigIntStats, right: BigIntStats): boolean { + return left.isDirectory() && right.isDirectory() + && left.dev === right.dev + && left.ino === right.ino + && left.mode === right.mode; +} + +function containedBy(root: string, target: string): boolean { + const fromRoot = relative(root, target); + return fromRoot === "" || (!isAbsolute(fromRoot) && fromRoot !== ".." && !fromRoot.startsWith(`..${sep}`)); +} + +function classifyIoError(error: unknown): DelegationFsError { + if (error instanceof DelegationFsError) return error; + const code = errorCode(error); + if (code === "EACCES" || code === "EPERM") return new DelegationFsError("unreadable", "delegation path is unreadable", false, { cause: error }); + return new DelegationFsError("unsafe_path", "delegation path could not be inspected safely", false, { cause: error }); +} + +function canonicalRoot(input: string, label: string): string { + try { + const absolute = resolve(input); + const entry = lstatSync(absolute, { bigint: true }); + if (!entry.isDirectory() || entry.isSymbolicLink()) throw new DelegationFsError("unsafe_path", `${label} is not a physical directory`); + return realpathSync.native(absolute); + } catch (error) { + throw classifyIoError(error); + } +} + +function resolvePaths(deps: CodexDelegationInstallerDeps): Paths { + const userHome = canonicalRoot(deps.userHome ?? homedir(), "user home"); + const codexHome = canonicalRoot(deps.codexHome ?? getCodexHome(), "Codex home"); + const skillDir = join(userHome, ".agents", "skills", "codexcommander-delegation"); + return { + userHome, + codexHome, + skillDir, + skillPath: join(skillDir, "SKILL.md"), + compatibilitySkillPath: join(codexHome, "skills", "codexcommander-delegation", "SKILL.md"), + agentsPath: join(codexHome, "AGENTS.md"), + overridePath: join(codexHome, "AGENTS.override.md"), + }; +} + +function assertCanonicalRoot(root: string): void { + let before: BigIntStats; + try { before = lstatSync(root, { bigint: true }); } catch (error) { throw classifyIoError(error); } + if (!before.isDirectory() || before.isSymbolicLink()) { + throw new DelegationFsError("unsafe_path", "delegation root is not a physical directory"); + } + let physical: string; + try { physical = realpathSync.native(root); } catch (error) { throw classifyIoError(error); } + if (!samePath(physical, root)) throw new DelegationFsError("unsafe_path", "delegation root contains a reparse substitution"); + const after = lstatSync(root, { bigint: true }); + if (!sameDirectoryIdentity(before, after)) throw new DelegationFsError("changed_during_mutation", "delegation root changed during inspection"); +} + +function assertSafeExistingDirectories(root: string, targetParent: string): void { + assertCanonicalRoot(root); + if (!containedBy(root, targetParent)) throw new DelegationFsError("unsafe_path", "delegation target escaped its fixed root"); + const rel = relative(root, targetParent); + let current = root; + for (const segment of rel === "" ? [] : rel.split(sep)) { + current = join(current, segment); + let entry: BigIntStats; + try { + entry = lstatSync(current, { bigint: true }); + } catch (error) { + if (errorCode(error) === "ENOENT") return; + throw classifyIoError(error); + } + if (!entry.isDirectory() || entry.isSymbolicLink()) { + throw new DelegationFsError("unsafe_path", "delegation path contains a linked or non-directory descendant"); + } + let physical: string; + try { physical = realpathSync.native(current); } catch (error) { throw classifyIoError(error); } + if (!samePath(physical, current)) throw new DelegationFsError("unsafe_path", "delegation path contains a reparse descendant"); + const after = lstatSync(current, { bigint: true }); + if (!sameDirectoryIdentity(entry, after)) throw new DelegationFsError("changed_during_mutation", "delegation directory changed during inspection"); + } +} + +function ensureSafeParent(root: string, targetParent: string): void { + assertCanonicalRoot(root); + if (!containedBy(root, targetParent)) throw new DelegationFsError("unsafe_path", "delegation target escaped its fixed root"); + const rel = relative(root, targetParent); + let current = root; + for (const segment of rel === "" ? [] : rel.split(sep)) { + current = join(current, segment); + try { + mkdirSync(current, { mode: 0o700 }); + } catch (error) { + if (errorCode(error) !== "EEXIST") throw classifyIoError(error); + } + let entry: BigIntStats; + try { entry = lstatSync(current, { bigint: true }); } catch (error) { throw classifyIoError(error); } + if (!entry.isDirectory() || entry.isSymbolicLink()) { + throw new DelegationFsError("unsafe_path", "delegation path contains a linked or non-directory descendant"); + } + let physical: string; + try { physical = realpathSync.native(current); } catch (error) { throw classifyIoError(error); } + if (!samePath(physical, current)) throw new DelegationFsError("unsafe_path", "delegation path contains a reparse descendant"); + const after = lstatSync(current, { bigint: true }); + if (!sameDirectoryIdentity(entry, after)) throw new DelegationFsError("changed_during_mutation", "delegation directory changed during creation"); + } +} + +function readSnapshot(root: string, path: string, limit: number): FileSnapshot { + assertSafeExistingDirectories(root, dirname(path)); + let pathStat: BigIntStats; + try { + pathStat = lstatSync(path, { bigint: true }); + } catch (error) { + if (errorCode(error) === "ENOENT") return ABSENT; + throw classifyIoError(error); + } + if (!pathStat.isFile() || pathStat.isSymbolicLink() || pathStat.nlink !== 1n) { + throw new DelegationFsError("unsafe_path", "delegation leaf is not a regular single-link file"); + } + if (pathStat.size > BigInt(limit)) throw new DelegationFsError("too_large", "delegation file exceeds its read bound"); + + let descriptor: number | null = null; + try { + const flags = process.platform === "win32" ? fsConstants.O_RDONLY : fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW; + descriptor = openSync(path, flags); + const opened = fstatSync(descriptor, { bigint: true }); + if (!opened.isFile() || opened.nlink !== 1n || opened.dev !== pathStat.dev || opened.ino !== pathStat.ino) { + throw new DelegationFsError("unsafe_path", "delegation leaf changed while it was opened"); + } + const buffer = Buffer.allocUnsafe(limit + 1); + let offset = 0; + while (offset < buffer.length) { + const read = readSync(descriptor, buffer, offset, buffer.length - offset, offset); + if (read === 0) break; + offset += read; + } + if (offset > limit) throw new DelegationFsError("too_large", "delegation file exceeds its read bound"); + const after = lstatSync(path, { bigint: true }); + if (!sameIdentity(pathStat, after)) throw new DelegationFsError("changed_during_mutation", "delegation leaf changed while it was read"); + const bytes = Buffer.from(buffer.subarray(0, offset)); + let text: string; + try { text = new TextDecoder("utf-8", { fatal: true }).decode(bytes); } + catch (error) { throw new DelegationFsError("invalid_utf8", "delegation file is not valid UTF-8", false, { cause: error }); } + return { kind: "file", bytes, text, stat: pathStat }; + } catch (error) { + throw classifyIoError(error); + } finally { + if (descriptor !== null) try { closeSync(descriptor); } catch { /* read result already determined */ } + } +} + +function snapshotsEqual(left: FileSnapshot, right: FileSnapshot): boolean { + if (left.kind !== right.kind) return false; + if (left.kind === "absent" || right.kind === "absent") return true; + return sameIdentity(left.stat, right.stat) && left.bytes.equals(right.bytes); +} + +function contentEquals(snapshot: FileSnapshot, text: string): boolean { + return snapshot.kind === "file" && snapshot.bytes.equals(Buffer.from(text, "utf8")); +} + +function previewsAndPrompts(): Pick { + const balanced = renderCodexDelegationBundle("balanced"); + const orchestrator = renderCodexDelegationBundle("orchestrator"); + return { + previews: { + balanced: { skillText: balanced.skillText, agentsBlockText: balanced.agentsBlockText }, + orchestrator: { skillText: orchestrator.skillText, agentsBlockText: orchestrator.agentsBlockText }, + }, + copyPrompts: { balanced: balanced.copyPrompt, orchestrator: orchestrator.copyPrompt }, + }; +} + +function hasManagedSkillOwnership(content: string): boolean { + if (isCodexCommanderManagedSkill(content)) return true; + const frontmatter = /^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/.exec(content)?.[1]; + return frontmatter !== undefined + && /^name:\s*codexcommander-delegation\s*\r?$/m.test(frontmatter) + && /^\s{2}managed-by:\s*codexcommander\s*\r?$/m.test(frontmatter); +} + +function unsafeStatus(reason: string): CodexDelegationStatus { + return { + schemaVersion: 1, + state: "unsafe", + installedMode: null, + artifacts: { + skill: { state: "unsafe", displayPath: SKILL_DISPLAY, reason }, + agentsPolicy: { state: "unsafe", displayPath: AGENTS_DISPLAY, reason }, + }, + override: { state: "unsafe" }, + activation: "unknown", + ...previewsAndPrompts(), + }; +} + +function buildInspection(deps: CodexDelegationInstallerDeps): InspectionContext { + const paths = resolvePaths(deps); + const skill = readSnapshot(paths.userHome, paths.skillPath, SKILL_LIMIT); + const compatibilitySkill = readSnapshot(paths.codexHome, paths.compatibilitySkillPath, SKILL_LIMIT); + const agents = readSnapshot(paths.codexHome, paths.agentsPath, AGENTS_LIMIT); + let overrideState: CodexDelegationStatus["override"]["state"]; + try { + const override = readSnapshot(paths.codexHome, paths.overridePath, AGENTS_LIMIT); + overrideState = override.kind === "absent" ? "absent" : override.bytes.length === 0 ? "empty" : "active"; + } catch { + overrideState = "unsafe"; + } + const balanced = renderCodexDelegationBundle("balanced"); + const orchestrator = renderCodexDelegationBundle("orchestrator"); + + let skillState: DelegationArtifactState; + let skillReason: string | undefined; + if (compatibilitySkill.kind === "file") { + skillState = "foreign"; + skillReason = "same-name skill exists in the compatibility Codex skill root"; + } else if (skill.kind === "absent") { + skillState = "absent"; + } else if (!hasManagedSkillOwnership(skill.text)) { + skillState = "foreign"; + skillReason = "skill frontmatter does not prove CodexCommander ownership"; + } else { + skillState = contentEquals(skill, balanced.skillText) ? "current" : "outdated"; + } + + const agentsInspection = inspectDelegationAgentsBlock(agents.kind === "file" ? agents.text : ""); + let agentsState: DelegationArtifactState; + let agentsReason: string | undefined; + if (agentsInspection.kind === "absent") { + agentsState = "absent"; + } else if (agentsInspection.kind === "conflict") { + agentsState = "foreign"; + agentsReason = `ambiguous delegation markers: ${agentsInspection.reason}`; + } else if (agentsInspection.content === balanced.agentsBlockText || agentsInspection.content === orchestrator.agentsBlockText) { + agentsState = "current"; + } else { + agentsState = "outdated"; + } + + const installedMode = agentsInspection.kind === "managed" ? agentsInspection.mode : null; + let state: CodexDelegationStatus["state"]; + if (skillState === "foreign" || agentsState === "foreign") state = "conflict"; + else if (skillState === "absent" && agentsState === "absent") state = "not-installed"; + else if (skillState === "absent" || agentsState === "absent") state = "partial"; + else if (skillState === "current" && agentsState === "current" + && installedMode !== null + && (agentsInspection.kind !== "managed" || agentsInspection.content === (installedMode === "balanced" ? balanced.agentsBlockText : orchestrator.agentsBlockText))) { + state = "current"; + } else state = "update-available"; + + const status: CodexDelegationStatus = { + schemaVersion: 1, + state, + installedMode, + artifacts: { + skill: { state: skillState, displayPath: SKILL_DISPLAY, ...(skillReason ? { reason: skillReason } : {}) }, + agentsPolicy: { state: agentsState, displayPath: AGENTS_DISPLAY, ...(agentsReason ? { reason: agentsReason } : {}) }, + }, + override: { state: overrideState }, + activation: overrideState === "unsafe" ? "unknown" : overrideState === "active" ? "shadowed" : "effective", + ...previewsAndPrompts(), + }; + return { paths, skill, agents, agentsInspection, status }; +} + +export function inspectCodexDelegation(deps: CodexDelegationInstallerDeps = {}): CodexDelegationStatus { + try { + return buildInspection(deps).status; + } catch (error) { + const classified = classifyIoError(error); + return unsafeStatus(classified.reason); + } +} + +function rootForPath(paths: Paths, path: string): string { + if (containedBy(paths.userHome, path)) return paths.userHome; + if (containedBy(paths.codexHome, path)) return paths.codexHome; + throw new DelegationFsError("unsafe_path", "delegation mutation escaped its fixed roots"); +} + +function safeTempCleanup(path: string | null, identity: { dev: bigint; ino: bigint } | null): void { + if (path === null || identity === null) return; + try { + const current = lstatSync(path, { bigint: true }); + if (current.isFile() && current.nlink === 1n && current.dev === identity.dev && current.ino === identity.ino) unlinkSync(path); + } catch { /* changed or absent temp paths are not ours to remove */ } +} + +function safeWrite( + paths: Paths, + path: string, + expected: FileSnapshot, + text: string, + artifact: "skill" | "agents", + deps: CodexDelegationInstallerDeps, +): FileSnapshotPresent { + const root = rootForPath(paths, path); + ensureSafeParent(root, dirname(path)); + const parentBefore = lstatSync(dirname(path), { bigint: true }); + const currentBefore = readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT); + if (!snapshotsEqual(expected, currentBefore)) throw new DelegationFsError("changed_during_mutation", "delegation preimage changed before preparation"); + const bytes = Buffer.from(text, "utf8"); + const mode = expected.kind === "file" ? Number(expected.stat.mode & 0o777n) : 0o600; + const tempPath = join(dirname(path), `.${basename(path)}.ccx.${process.pid}.${++tempSequence}.tmp`); + let descriptor: number | null = null; + let tempIdentity: { dev: bigint; ino: bigint } | null = null; + let parentPrepared: BigIntStats | null = null; + let published = false; + try { + const flags = fsConstants.O_CREAT | fsConstants.O_EXCL | fsConstants.O_WRONLY + | (process.platform === "win32" ? 0 : fsConstants.O_NOFOLLOW); + descriptor = openSync(tempPath, flags, 0o600); + const created = fstatSync(descriptor, { bigint: true }); + if (!created.isFile() || created.nlink !== 1n) throw new DelegationFsError("unsafe_path", "delegation temp is not a regular single-link file"); + tempIdentity = { dev: created.dev, ino: created.ino }; + writeFileSync(descriptor, bytes); + try { fchmodSync(descriptor, mode); } catch { if (process.platform !== "win32") throw new DelegationFsError("write_failed", "delegation temp mode could not be preserved"); } + fsyncSync(descriptor); + closeSync(descriptor); + descriptor = null; + + parentPrepared = lstatSync(dirname(path), { bigint: true }); + if (!sameDirectoryObject(parentBefore, parentPrepared)) { + throw new DelegationFsError("changed_during_mutation", "delegation parent changed during preparation"); + } + + deps.beforePublish?.(artifact); + assertSafeExistingDirectories(root, dirname(path)); + const parentNow = lstatSync(dirname(path), { bigint: true }); + if (!sameDirectoryIdentity(parentPrepared, parentNow)) throw new DelegationFsError("changed_during_mutation", "delegation parent changed before publication"); + const current = readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT); + if (!snapshotsEqual(expected, current)) throw new DelegationFsError("changed_during_mutation", "delegation preimage changed before publication"); + const temp = readSnapshot(root, tempPath, bytes.length); + if (temp.kind !== "file" || temp.stat.dev !== tempIdentity.dev || temp.stat.ino !== tempIdentity.ino || !temp.bytes.equals(bytes)) { + throw new DelegationFsError("changed_during_mutation", "delegation temp changed before publication"); + } + renameAtomicFile(tempPath, path); + published = true; + const after = readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT); + if (after.kind !== "file" || after.stat.dev !== tempIdentity.dev || after.stat.ino !== tempIdentity.ino || !after.bytes.equals(bytes)) { + throw new DelegationFsError("partial_write", "delegation postimage verification failed", true); + } + try { + const parentFd = openSync(dirname(path), "r"); + try { fsyncSync(parentFd); } finally { closeSync(parentFd); } + } catch { /* not all platforms permit directory fsync */ } + return after; + } catch (error) { + if (error instanceof DelegationFsError) throw error; + throw new DelegationFsError(published ? "partial_write" : "write_failed", "delegation write failed", published, { cause: error }); + } finally { + if (descriptor !== null) try { closeSync(descriptor); } catch { /* primary error wins */ } + safeTempCleanup(tempPath, tempIdentity); + } +} + +function safeRemove( + paths: Paths, + path: string, + expected: FileSnapshotPresent, + artifact: "skill" | "agents", + deps: CodexDelegationInstallerDeps, +): FileSnapshotAbsent { + const root = rootForPath(paths, path); + const parentBefore = lstatSync(dirname(path), { bigint: true }); + const currentBefore = readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT); + if (!snapshotsEqual(expected, currentBefore)) throw new DelegationFsError("changed_during_mutation", "delegation preimage changed before removal"); + let published = false; + try { + deps.beforePublish?.(artifact); + assertSafeExistingDirectories(root, dirname(path)); + const parentNow = lstatSync(dirname(path), { bigint: true }); + if (!sameDirectoryIdentity(parentBefore, parentNow)) throw new DelegationFsError("changed_during_mutation", "delegation parent changed before removal"); + const current = readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT); + if (!snapshotsEqual(expected, current)) throw new DelegationFsError("changed_during_mutation", "delegation preimage changed before removal"); + unlinkSync(path); + published = true; + if (readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT).kind !== "absent") { + throw new DelegationFsError("partial_write", "delegation removal verification failed", true); + } + return ABSENT; + } catch (error) { + if (error instanceof DelegationFsError) throw error; + throw new DelegationFsError(published ? "partial_write" : "write_failed", "delegation removal failed", published, { cause: error }); + } +} + +function applyDesired( + context: InspectionContext, + artifact: "skill" | "agents", + before: FileSnapshot, + desired: string | null, + deps: CodexDelegationInstallerDeps, +): AppliedMutation | null { + const path = artifact === "skill" ? context.paths.skillPath : context.paths.agentsPath; + if (desired === null) { + if (before.kind === "absent") return null; + return { artifact, path, before, after: safeRemove(context.paths, path, before, artifact, deps) }; + } + if (contentEquals(before, desired)) return null; + return { artifact, path, before, after: safeWrite(context.paths, path, before, desired, artifact, deps) }; +} + +function compensate(context: InspectionContext, applied: AppliedMutation, deps: CodexDelegationInstallerDeps): void { + if (applied.before.kind === "absent") { + if (applied.after.kind === "file") safeRemove(context.paths, applied.path, applied.after, applied.artifact, deps); + } else { + safeWrite(context.paths, applied.path, applied.after, applied.before.text, applied.artifact, deps); + } +} + +function removeEmptySkillDir(paths: Paths, expected: BigIntStats): void { + try { + const entry = lstatSync(paths.skillDir, { bigint: true }); + if (!sameDirectoryObject(expected, entry) + || entry.isSymbolicLink() + || !samePath(realpathSync.native(paths.skillDir), paths.skillDir)) { + throw new DelegationFsError("partial_write", "skill directory became unsafe after uninstall", true); + } + const beforeRemove = lstatSync(paths.skillDir, { bigint: true }); + if (!sameDirectoryIdentity(entry, beforeRemove)) { + throw new DelegationFsError("partial_write", "skill directory changed before uninstall cleanup", true); + } + rmdirSync(paths.skillDir); + } catch (error) { + const code = errorCode(error); + if (code === "ENOENT" || code === "ENOTEMPTY") return; + if (error instanceof DelegationFsError) throw error; + throw new DelegationFsError("partial_write", "skill directory could not be removed safely", true, { cause: error }); + } +} + +function failureStatus(deps: CodexDelegationInstallerDeps, fallback: CodexDelegationStatus): CodexDelegationStatus { + try { return buildInspection(deps).status; } catch { return fallback; } +} + +export function mutateCodexDelegation( + mutation: CodexDelegationMutation, + deps: CodexDelegationInstallerDeps = {}, +): CodexDelegationMutationOutcome { + if (mutationInProgress) { + return { ok: false, changed: false, reason: "mutation_busy", status: inspectCodexDelegation(deps) }; + } + mutationInProgress = true; + let initialStatus = unsafeStatus("unsafe_path"); + try { + const paths = resolvePaths(deps); + assertNotRealHomeUnderTest(paths.skillPath); + assertNotRealHomeUnderTest(paths.agentsPath); + const context = buildInspection({ ...deps, userHome: paths.userHome, codexHome: paths.codexHome }); + initialStatus = context.status; + if (context.status.artifacts.skill.state === "foreign") { + return { ok: false, changed: false, reason: "foreign_skill", status: context.status }; + } + if (context.status.artifacts.agentsPolicy.state === "foreign") { + return { ok: false, changed: false, reason: "ambiguous_agents_markers", status: context.status }; + } + + const skillDirBefore = mutation.action === "uninstall" && context.skill.kind === "file" + ? lstatSync(context.paths.skillDir, { bigint: true }) + : null; + let desiredSkill: string | null; + let desiredAgents: string | null; + if (mutation.action === "install") { + const bundle = renderCodexDelegationBundle(mutation.mode); + desiredSkill = bundle.skillText; + desiredAgents = upsertDelegationAgentsBlock(context.agents.kind === "file" ? context.agents.text : "", bundle.agentsBlockText).content; + } else { + desiredSkill = null; + desiredAgents = context.agentsInspection.kind === "managed" + ? removeDelegationAgentsBlock(context.agents.kind === "file" ? context.agents.text : "").content + : context.agents.kind === "file" ? context.agents.text : null; + } + + const plan = mutation.action === "install" + ? [ + { artifact: "skill" as const, before: context.skill, desired: desiredSkill }, + { artifact: "agents" as const, before: context.agents, desired: desiredAgents }, + ] + : [ + { artifact: "agents" as const, before: context.agents, desired: desiredAgents }, + { artifact: "skill" as const, before: context.skill, desired: desiredSkill }, + ]; + + let first: AppliedMutation | null = null; + let changed = false; + for (let index = 0; index < plan.length; index += 1) { + const step = plan[index]; + try { + const applied = applyDesired(context, step.artifact, step.before, step.desired, deps); + if (applied === null) continue; + changed = true; + if (first === null) first = applied; + } catch (error) { + const classified = error instanceof DelegationFsError + ? error + : new DelegationFsError("write_failed", "delegation mutation failed", false, { cause: error }); + if (classified.published) { + return { ok: false, changed: true, reason: "partial_write", status: failureStatus(deps, initialStatus) }; + } + if (first !== null && index > 0) { + try { + compensate(context, first, deps); + return { ok: false, changed: false, reason: classified.reason, status: failureStatus(deps, initialStatus) }; + } catch { + return { ok: false, changed: true, reason: "partial_write", status: failureStatus(deps, initialStatus) }; + } + } + return { ok: false, changed: false, reason: classified.reason, status: failureStatus(deps, initialStatus) }; + } + } + + if (mutation.action === "uninstall" && skillDirBefore !== null) removeEmptySkillDir(context.paths, skillDirBefore); + const status = inspectCodexDelegation(deps); + if (status.state === "unsafe") return { ok: false, changed, reason: "partial_write", status }; + return { ok: true, changed, status }; + } catch (error) { + const classified = classifyIoError(error); + return { ok: false, changed: classified.published, reason: classified.reason, status: failureStatus(deps, initialStatus) }; + } finally { + mutationInProgress = false; + } +} diff --git a/src/lib/test-home-guard.ts b/src/lib/test-home-guard.ts index bd42cffe55..26fe9c373e 100644 --- a/src/lib/test-home-guard.ts +++ b/src/lib/test-home-guard.ts @@ -21,7 +21,7 @@ * how this incident happened. */ import { homedir } from "node:os"; -import { dirname, join, relative, resolve } from "node:path"; +import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { realpathSync } from "node:fs"; const GUARD_ENV = "CCX_TEST_HOME_GUARD"; @@ -60,7 +60,16 @@ function canonicalize(path: string): string { * guard would be perfectly inverted while its tests still looked green. */ const REAL_HOME = process.env[REAL_HOME_ENV]?.trim() || homedir(); -const PROTECTED_HOMES = [canonicalize(join(REAL_HOME, ".codexcommander"))] as const; +const PROTECTED_HOMES = [ + canonicalize(join(REAL_HOME, ".codexcommander")), + canonicalize(join(REAL_HOME, ".codex")), + canonicalize(join(REAL_HOME, ".agents")), +] as const; + +function isAtOrBelow(root: string, target: string): boolean { + const fromRoot = relative(root, target); + return fromRoot === "" || (!isAbsolute(fromRoot) && fromRoot !== ".." && !fromRoot.startsWith(`..${sep}`)); +} /** The production home this process protects. Exported for the guard's own tests. */ export function protectedHomeForTests(): string { @@ -87,10 +96,10 @@ export function isTestHomeGuardArmed(): boolean { export function assertNotRealHomeUnderTest(dir: string): void { if (!isTestHomeGuardArmed()) return; const target = canonicalize(dir); - if (!PROTECTED_HOMES.includes(target as (typeof PROTECTED_HOMES)[number])) return; + if (!PROTECTED_HOMES.some((root) => isAtOrBelow(root, target))) return; throw new Error( - `refusing to write a real CodexCommander home (${target}) from a test process. ` - + "Point CODEXCOMMANDER_HOME at a temp directory for this test, or inject persistence " + `refusing to write a protected user state home (${target}) from a test process. ` + + "Point the affected home at a temp directory for this test, or inject persistence " + "instead of calling the global writer.", ); } diff --git a/tests/codex-delegation-installer.test.ts b/tests/codex-delegation-installer.test.ts new file mode 100644 index 0000000000..66121cd434 --- /dev/null +++ b/tests/codex-delegation-installer.test.ts @@ -0,0 +1,421 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { + chmodSync, + existsSync, + linkSync, + lstatSync, + mkdirSync, + mkdtempSync, + readFileSync, + readdirSync, + renameSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { renderCodexDelegationBundle } from "../src/codex/delegation-templates"; +import { + inspectCodexDelegation, + mutateCodexDelegation, + type CodexDelegationInstallerDeps, +} from "../src/codex/delegation-installer"; + +interface Fixture { + root: string; + userHome: string; + codexHome: string; + deps: CodexDelegationInstallerDeps; + skillDir: string; + skillPath: string; + compatSkillPath: string; + agentsPath: string; + overridePath: string; + configPath: string; +} + +const fixtures: string[] = []; + +afterEach(() => { + for (const path of fixtures.splice(0)) rmSync(path, { recursive: true, force: true }); +}); + +function fixture(): Fixture { + const root = mkdtempSync(join(tmpdir(), "ccx-delegation-installer-")); + fixtures.push(root); + const userHome = join(root, "user"); + const codexHome = join(root, "codex"); + mkdirSync(userHome); + mkdirSync(codexHome); + const skillDir = join(userHome, ".agents", "skills", "codexcommander-delegation"); + return { + root, + userHome, + codexHome, + deps: { userHome, codexHome }, + skillDir, + skillPath: join(skillDir, "SKILL.md"), + compatSkillPath: join(codexHome, "skills", "codexcommander-delegation", "SKILL.md"), + agentsPath: join(codexHome, "AGENTS.md"), + overridePath: join(codexHome, "AGENTS.override.md"), + configPath: join(codexHome, "config.toml"), + }; +} + +function write(path: string, content: string | Uint8Array): void { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, content); +} + +function assertNoManagementResidue(fx: Fixture): void { + const names: string[] = []; + function visit(path: string): void { + if (!existsSync(path) || !lstatSync(path).isDirectory()) return; + for (const name of readdirSync(path)) { + names.push(name); + visit(join(path, name)); + } + } + visit(fx.userHome); + visit(fx.codexHome); + expect(names.some((name) => name === ".codexcommander-managed" || /(?:hash|manifest|lock)/i.test(name))).toBe(false); +} + +describe("Codex delegation installer", () => { + test("fresh balanced install creates only SKILL.md and the marked AGENTS block", () => { + const fx = fixture(); + const bundle = renderCodexDelegationBundle("balanced"); + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + + expect(outcome.ok).toBe(true); + expect(outcome.changed).toBe(true); + expect(readFileSync(fx.skillPath, "utf8")).toBe(bundle.skillText); + expect(readFileSync(fx.agentsPath, "utf8")).toBe(bundle.agentsBlockText); + expect(outcome.status.state).toBe("current"); + expect(outcome.status.installedMode).toBe("balanced"); + expect(outcome.status.artifacts.skill.displayPath).toBe("$HOME/.agents/skills/codexcommander-delegation/SKILL.md"); + expect(outcome.status.artifacts.agentsPolicy.displayPath).toBe("$CODEX_HOME/AGENTS.md"); + }); + + test("orchestrator install writes the same skill and only changes the global mode sentence", () => { + const balanced = fixture(); + const orchestrator = fixture(); + mutateCodexDelegation({ action: "install", mode: "balanced" }, balanced.deps); + mutateCodexDelegation({ action: "install", mode: "orchestrator" }, orchestrator.deps); + + expect(readFileSync(orchestrator.skillPath, "utf8")).toBe(readFileSync(balanced.skillPath, "utf8")); + const left = readFileSync(balanced.agentsPath, "utf8").split("\n"); + const right = readFileSync(orchestrator.agentsPath, "utf8").split("\n"); + expect(left.filter((line, index) => line !== right[index])).toEqual([ + "Mode: balanced", + "Delegate substantial bounded parallel work when it will clearly help; the root may still implement and must synthesize.", + ]); + }); + + test("reinstall is idempotent", () => { + const fx = fixture(); + expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps)).toMatchObject({ ok: true, changed: true }); + const skillMtime = lstatSync(fx.skillPath, { bigint: true }).mtimeNs; + const agentsMtime = lstatSync(fx.agentsPath, { bigint: true }).mtimeNs; + + expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps)).toMatchObject({ ok: true, changed: false }); + expect(lstatSync(fx.skillPath, { bigint: true }).mtimeNs).toBe(skillMtime); + expect(lstatSync(fx.agentsPath, { bigint: true }).mtimeNs).toBe(agentsMtime); + }); + + test("mode update replaces only bytes between AGENTS markers", () => { + const fx = fixture(); + const balanced = renderCodexDelegationBundle("balanced").agentsBlockText; + const orchestrator = renderCodexDelegationBundle("orchestrator").agentsBlockText; + write(fx.skillPath, renderCodexDelegationBundle("balanced").skillText); + write(fx.agentsPath, `prefix\r\n${balanced.replaceAll("\n", "\r\n")}\r\nsuffix\r\n`); + + const outcome = mutateCodexDelegation({ action: "install", mode: "orchestrator" }, fx.deps); + expect(outcome).toMatchObject({ ok: true, changed: true }); + expect(readFileSync(fx.agentsPath, "utf8")).toBe(`prefix\r\n${orchestrator.replaceAll("\n", "\r\n")}\r\nsuffix\r\n`); + }); + + test("uninstall removes AGENTS block first, then only SKILL.md, and rmdir only when empty", () => { + const fx = fixture(); + mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + const order: string[] = []; + + const outcome = mutateCodexDelegation({ action: "uninstall" }, { + ...fx.deps, + beforePublish: (artifact) => order.push(artifact), + }); + expect(outcome).toMatchObject({ ok: true, changed: true, status: { state: "not-installed" } }); + expect(order).toEqual(["agents", "skill"]); + expect(existsSync(fx.agentsPath)).toBe(true); + expect(readFileSync(fx.agentsPath, "utf8")).toBe(""); + expect(existsSync(fx.skillPath)).toBe(false); + expect(existsSync(fx.skillDir)).toBe(false); + }); + + test("uninstall preserves unexpected sibling files in the skill directory", () => { + const fx = fixture(); + mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + const sibling = join(fx.skillDir, "notes.txt"); + write(sibling, "user-owned"); + + expect(mutateCodexDelegation({ action: "uninstall" }, fx.deps)).toMatchObject({ ok: true, changed: true }); + expect(readFileSync(sibling, "utf8")).toBe("user-owned"); + expect(existsSync(fx.skillDir)).toBe(true); + }); + + test("uninstall preserves an unmarked user AGENTS file byte for byte", () => { + const fx = fixture(); + const userAgents = Buffer.from("# User policy\r\nNever touch this file.\r\n"); + write(fx.agentsPath, userAgents); + + expect(mutateCodexDelegation({ action: "uninstall" }, fx.deps)).toMatchObject({ ok: true, changed: false }); + expect(readFileSync(fx.agentsPath)).toEqual(userAgents); + }); + + test("foreign SKILL.md is never overwritten or deleted", () => { + const fx = fixture(); + write(fx.skillPath, "---\nname: codexcommander-delegation\n---\nforeign\n"); + for (const mutation of [{ action: "install", mode: "balanced" }, { action: "uninstall" }] as const) { + const outcome = mutateCodexDelegation(mutation, fx.deps); + expect(outcome).toMatchObject({ ok: false, changed: false, reason: "foreign_skill" }); + expect(readFileSync(fx.skillPath, "utf8")).toContain("foreign"); + expect(existsSync(fx.agentsPath)).toBe(false); + } + }); + + test("valid managed metadata permits update and uninstall without a separate ownership file", () => { + const fx = fixture(); + const canonical = renderCodexDelegationBundle("balanced").skillText; + write(fx.skillPath, canonical.replace(/\n# CodexCommander delegation/, "\nlegacy text\n# CodexCommander delegation")); + write(fx.agentsPath, renderCodexDelegationBundle("balanced").agentsBlockText.replace("schema: 1", "schema: 0")); + expect(inspectCodexDelegation(fx.deps).state).toBe("update-available"); + + expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps)).toMatchObject({ ok: true, changed: true }); + expect(readFileSync(fx.skillPath, "utf8")).toBe(canonical); + expect(mutateCodexDelegation({ action: "uninstall" }, fx.deps)).toMatchObject({ ok: true, changed: true }); + expect(existsSync(fx.skillPath)).toBe(false); + }); + + test("older managed skill metadata remains owned and reports update available", () => { + const fx = fixture(); + const older = renderCodexDelegationBundle("balanced").skillText.replace('managed-version: "1"', 'managed-version: "0"'); + write(fx.skillPath, older); + write(fx.agentsPath, renderCodexDelegationBundle("balanced").agentsBlockText); + + expect(inspectCodexDelegation(fx.deps)).toMatchObject({ + state: "update-available", + artifacts: { skill: { state: "outdated" }, agentsPolicy: { state: "current" } }, + }); + expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps)).toMatchObject({ ok: true, changed: true }); + }); + + test.each([ + ["duplicate", `${renderCodexDelegationBundle("balanced").agentsBlockText}\n${renderCodexDelegationBundle("balanced").agentsBlockText}`], + ["orphan", "\nuser"], + ["reversed", "\n"], + ["malformed", "prefix "], + ])("%s AGENTS markers refuse without writes", (_name, agents) => { + const fx = fixture(); + write(fx.agentsPath, agents); + const before = readFileSync(fx.agentsPath); + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + expect(outcome).toMatchObject({ ok: false, changed: false, reason: "ambiguous_agents_markers" }); + expect(readFileSync(fx.agentsPath)).toEqual(before); + expect(existsSync(fx.skillPath)).toBe(false); + }); + + test("a duplicate compatibility-root skill refuses without writes", () => { + const fx = fixture(); + write(fx.compatSkillPath, renderCodexDelegationBundle("balanced").skillText); + expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps)).toMatchObject({ + ok: false, + changed: false, + reason: "foreign_skill", + }); + expect(existsSync(fx.skillPath)).toBe(false); + expect(existsSync(fx.agentsPath)).toBe(false); + }); + + test.each(["symlink parent", "symlink leaf", "hardlink", "directory leaf"])("%s refuses as unsafe", (shape) => { + const fx = fixture(); + if (shape === "symlink parent") { + const outside = join(fx.root, "outside"); + mkdirSync(outside); + mkdirSync(join(fx.userHome, ".agents")); + symlinkSync(outside, join(fx.userHome, ".agents", "skills"), process.platform === "win32" ? "junction" : "dir"); + } else if (shape === "symlink leaf") { + write(join(fx.root, "outside-skill"), "foreign"); + mkdirSync(fx.skillDir, { recursive: true }); + symlinkSync(join(fx.root, "outside-skill"), fx.skillPath); + } else if (shape === "hardlink") { + write(fx.skillPath, renderCodexDelegationBundle("balanced").skillText); + linkSync(fx.skillPath, join(fx.root, "second-link")); + } else { + mkdirSync(fx.skillPath, { recursive: true }); + } + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + expect(outcome).toMatchObject({ ok: false, changed: false, reason: "unsafe_path" }); + expect(outcome.status.state).toBe("unsafe"); + }); + + test("nonregular leaf refuses as unsafe", () => { + if (process.platform === "win32") return; + const fx = fixture(); + mkdirSync(fx.skillDir, { recursive: true }); + const made = Bun.spawnSync(["mkfifo", fx.skillPath]); + expect(made.exitCode).toBe(0); + expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps)).toMatchObject({ + ok: false, + changed: false, + reason: "unsafe_path", + }); + }); + + test("invalid UTF-8 and oversized files refuse with their precise reasons", () => { + const invalid = fixture(); + write(invalid.agentsPath, new Uint8Array([0xc3, 0x28])); + expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, invalid.deps)).toMatchObject({ reason: "invalid_utf8" }); + + const large = fixture(); + write(large.skillPath, "x".repeat(256 * 1024 + 1)); + expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, large.deps)).toMatchObject({ reason: "too_large" }); + }); + + test("changed preimage before publish refuses", () => { + const fx = fixture(); + write(fx.agentsPath, "user preface\n"); + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, { + ...fx.deps, + beforePublish: (artifact) => { + if (artifact === "agents") writeFileSync(fx.agentsPath, "concurrent edit\n"); + }, + }); + expect(outcome).toMatchObject({ ok: false, changed: false, reason: "changed_during_mutation" }); + expect(readFileSync(fx.agentsPath, "utf8")).toBe("concurrent edit\n"); + expect(existsSync(fx.skillPath)).toBe(false); + }); + + test("a Codex root symlink swap between artifacts refuses before creating a temp", () => { + const fx = fixture(); + const movedRoot = join(fx.root, "moved-codex"); + const outside = join(fx.root, "outside-codex"); + mkdirSync(outside); + let swapped = false; + let agentsHookReached = false; + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, { + ...fx.deps, + beforePublish: (artifact) => { + if (artifact === "skill" && !swapped) { + swapped = true; + renameSync(fx.codexHome, movedRoot); + symlinkSync(outside, fx.codexHome, process.platform === "win32" ? "junction" : "dir"); + } else if (artifact === "agents") { + agentsHookReached = true; + } + }, + }); + + expect(outcome).toMatchObject({ ok: false, changed: false, reason: "unsafe_path" }); + expect(agentsHookReached).toBe(false); + expect(readdirSync(outside)).toEqual([]); + }); + + test("second-artifact failure compensates the first artifact", () => { + const fx = fixture(); + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, { + ...fx.deps, + beforePublish: (artifact) => { + if (artifact === "agents") throw new Error("injected second-artifact failure"); + }, + }); + expect(outcome).toMatchObject({ ok: false, changed: false, reason: "write_failed" }); + expect(existsSync(fx.skillPath)).toBe(false); + expect(existsSync(fx.agentsPath)).toBe(false); + }); + + test("failed compensation reports partial_write with changed true", () => { + const fx = fixture(); + let sabotaged = false; + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, { + ...fx.deps, + beforePublish: (artifact) => { + if (artifact === "agents" && !sabotaged) { + sabotaged = true; + writeFileSync(fx.skillPath, "concurrent replacement"); + throw new Error("injected second-artifact failure"); + } + }, + }); + expect(outcome).toMatchObject({ ok: false, changed: true, reason: "partial_write" }); + expect(readFileSync(fx.skillPath, "utf8")).toBe("concurrent replacement"); + }); + + test("active AGENTS.override.md reports shadowed without changing override bytes", () => { + const fx = fixture(); + const override = Buffer.from("# local override\r\nDo not delegate.\r\n"); + write(fx.overridePath, override); + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + expect(outcome).toMatchObject({ ok: true, status: { state: "current", activation: "shadowed", override: { state: "active" } } }); + expect(readFileSync(fx.overridePath)).toEqual(override); + }); + + test("unsafe AGENTS.override.md retains structural status and reports unknown activation", () => { + const fx = fixture(); + mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + const target = join(fx.root, "foreign-override"); + write(target, "foreign"); + symlinkSync(target, fx.overridePath); + + expect(inspectCodexDelegation(fx.deps)).toMatchObject({ + state: "current", + artifacts: { skill: { state: "current" }, agentsPolicy: { state: "current" } }, + override: { state: "unsafe" }, + activation: "unknown", + }); + }); + + test("config.toml and subagentDeveloperInstructions are identical before and after every mutation", () => { + const fx = fixture(); + const config = Buffer.from('model = "native"\nsubagentDeveloperInstructions = "user-owned"\n'); + write(fx.configPath, config); + for (const mutation of [ + { action: "install", mode: "balanced" }, + { action: "install", mode: "orchestrator" }, + { action: "uninstall" }, + ] as const) { + mutateCodexDelegation(mutation, fx.deps); + expect(readFileSync(fx.configPath)).toEqual(config); + } + }); + + test("no .codexcommander-managed file hash manifest or lock is created", () => { + const fx = fixture(); + mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + mutateCodexDelegation({ action: "install", mode: "orchestrator" }, fx.deps); + mutateCodexDelegation({ action: "uninstall" }, fx.deps); + assertNoManagementResidue(fx); + }); + + test("module-local single-flight refuses a reentrant mutation", () => { + const fx = fixture(); + let nested: ReturnType | undefined; + const outer = mutateCodexDelegation({ action: "install", mode: "balanced" }, { + ...fx.deps, + beforePublish: (artifact) => { + if (artifact === "skill" && nested === undefined) nested = mutateCodexDelegation({ action: "uninstall" }, fx.deps); + }, + }); + expect(outer.ok).toBe(true); + expect(nested).toMatchObject({ ok: false, changed: false, reason: "mutation_busy" }); + }); + + test("existing AGENTS mode is preserved", () => { + if (process.platform === "win32") return; + const fx = fixture(); + write(fx.agentsPath, "preface\n"); + chmodSync(fx.agentsPath, 0o640); + mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + expect(lstatSync(fx.agentsPath).mode & 0o777).toBe(0o640); + }); +}); diff --git a/tests/test-home-guard.test.ts b/tests/test-home-guard.test.ts index 3cf45f4f88..977b6e1979 100644 --- a/tests/test-home-guard.test.ts +++ b/tests/test-home-guard.test.ts @@ -72,7 +72,7 @@ describe("real-home write guard", () => { import { mutateStore } from "${REPO_ROOT_URL}src/oauth/store"; import { saveCodexAccountCredential } from "${REPO_ROOT_URL}src/codex/account-store"; const threw: string[] = []; - const REFUSAL = "refusing to write a real CodexCommander home"; + const REFUSAL = "refusing to write a protected user state home"; try { saveConfig({ port: 10100, multiAgentGuidanceEnabled: true, @@ -128,7 +128,7 @@ describe("real-home write guard", () => { const probe = runProbe(` import { saveConfig } from "${REPO_ROOT_URL}src/config"; - const REFUSAL = "refusing to write a real CodexCommander home"; + const REFUSAL = "refusing to write a protected user state home"; try { saveConfig({ port: 10100, @@ -189,7 +189,7 @@ describe("real-home write guard", () => { const probe = runProbe(` import { atomicWriteFile, writePid } from "${REPO_ROOT_URL}src/config"; - const REFUSAL = "refusing to write a real CodexCommander home"; + const REFUSAL = "refusing to write a protected user state home"; try { atomicWriteFile("${linkDir}/never-created.json", "x"); console.log("WRITE_SUCCEEDED"); @@ -265,6 +265,36 @@ describe("real-home write guard", () => { expect(probe.stdout.trim()).toBe("rejected"); }); + test("descendants of the real .codex and .agents homes are rejected", () => { + const { realHome } = sentinelHome(); + const probe = runProbe(` + import { assertNotRealHomeUnderTest } from "${REPO_ROOT_URL}src/lib/test-home-guard"; + const results: string[] = []; + for (const path of [ + ${JSON.stringify(join(realHome, ".codex", "AGENTS.md"))}, + ${JSON.stringify(join(realHome, ".agents", "skills", "managed", "SKILL.md"))}, + ]) { + try { assertNotRealHomeUnderTest(path); results.push("allowed"); } + catch (error) { results.push(String(error).includes("protected user state home") ? "rejected" : "wrong-error"); } + } + console.log(JSON.stringify(results)); + `, { CCX_TEST_HOME_GUARD: "1", CCX_REAL_HOME: realHome }); + + expect(JSON.parse(probe.stdout.trim())).toEqual(["rejected", "rejected"]); + }); + + test("an armed test sandbox outside every protected home succeeds", () => { + const { realHome } = sentinelHome(); + const sandbox = mkdtempSync(join(tmpdir(), "ccx-guard-safe-sandbox-")); + const probe = runProbe(` + import { assertNotRealHomeUnderTest } from "${REPO_ROOT_URL}src/lib/test-home-guard"; + try { assertNotRealHomeUnderTest(${JSON.stringify(join(sandbox, ".agents", "skills"))}); console.log("allowed"); } + catch { console.log("rejected"); } + `, { CCX_TEST_HOME_GUARD: "1", CCX_REAL_HOME: realHome }); + + expect(probe.stdout.trim()).toBe("allowed"); + }); + test("/var and /private/var spellings of one path agree", () => { // macOS hands out /var/folders/... whose realpath is /private/var/folders/...; // a lexical comparison would disagree with itself across those two spellings. From 84a89e74f15aa81ec5a48612fd3b5bbaea6ed764 Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 13:03:47 -0400 Subject: [PATCH 03/25] fix: harden delegation ownership inspection --- src/codex/delegation-installer.ts | 44 ++++++++++++----- tests/codex-delegation-installer.test.ts | 61 ++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 17 deletions(-) diff --git a/src/codex/delegation-installer.ts b/src/codex/delegation-installer.ts index 03d1d43a95..82edb0488d 100644 --- a/src/codex/delegation-installer.ts +++ b/src/codex/delegation-installer.ts @@ -25,7 +25,6 @@ import { type DelegationAgentsInspection, } from "./delegation-agents-block"; import { - isCodexCommanderManagedSkill, renderCodexDelegationBundle, type CodexDelegationMode, } from "./delegation-templates"; @@ -115,6 +114,7 @@ interface InspectionContext { skill: FileSnapshot; agents: FileSnapshot; agentsInspection: DelegationAgentsInspection; + compatibilityCollision: boolean; status: CodexDelegationStatus; } @@ -329,11 +329,23 @@ function previewsAndPrompts(): Pick line === "name: codexcommander-delegation").length !== 1) return false; + const metadataRows = lines.flatMap((line, index) => line === "metadata:" ? [index] : []); + if (metadataRows.length !== 1) return false; + + const entries: string[] = []; + for (let index = metadataRows[0] + 1; index < lines.length; index += 1) { + const line = lines[index]; + if (line.length === 0) continue; + if (!line.startsWith(" ")) break; + entries.push(line); + } + return entries.length === 2 + && entries.includes(" managed-by: codexcommander") + && entries.some((line) => line === ' managed-version: "0"' || line === ' managed-version: "1"'); } function unsafeStatus(reason: string): CodexDelegationStatus { @@ -365,13 +377,11 @@ function buildInspection(deps: CodexDelegationInstallerDeps): InspectionContext } const balanced = renderCodexDelegationBundle("balanced"); const orchestrator = renderCodexDelegationBundle("orchestrator"); + const compatibilityCollision = compatibilitySkill.kind === "file"; let skillState: DelegationArtifactState; let skillReason: string | undefined; - if (compatibilitySkill.kind === "file") { - skillState = "foreign"; - skillReason = "same-name skill exists in the compatibility Codex skill root"; - } else if (skill.kind === "absent") { + if (skill.kind === "absent") { skillState = "absent"; } else if (!hasManagedSkillOwnership(skill.text)) { skillState = "foreign"; @@ -381,6 +391,11 @@ function buildInspection(deps: CodexDelegationInstallerDeps): InspectionContext } const agentsInspection = inspectDelegationAgentsBlock(agents.kind === "file" ? agents.text : ""); + const agentsText = agents.kind === "file" ? agents.text : ""; + const balancedPolicyCurrent = agentsInspection.kind === "managed" + && !upsertDelegationAgentsBlock(agentsText, balanced.agentsBlockText).changed; + const orchestratorPolicyCurrent = agentsInspection.kind === "managed" + && !upsertDelegationAgentsBlock(agentsText, orchestrator.agentsBlockText).changed; let agentsState: DelegationArtifactState; let agentsReason: string | undefined; if (agentsInspection.kind === "absent") { @@ -388,7 +403,7 @@ function buildInspection(deps: CodexDelegationInstallerDeps): InspectionContext } else if (agentsInspection.kind === "conflict") { agentsState = "foreign"; agentsReason = `ambiguous delegation markers: ${agentsInspection.reason}`; - } else if (agentsInspection.content === balanced.agentsBlockText || agentsInspection.content === orchestrator.agentsBlockText) { + } else if (balancedPolicyCurrent || orchestratorPolicyCurrent) { agentsState = "current"; } else { agentsState = "outdated"; @@ -396,12 +411,12 @@ function buildInspection(deps: CodexDelegationInstallerDeps): InspectionContext const installedMode = agentsInspection.kind === "managed" ? agentsInspection.mode : null; let state: CodexDelegationStatus["state"]; - if (skillState === "foreign" || agentsState === "foreign") state = "conflict"; + if (compatibilityCollision || skillState === "foreign" || agentsState === "foreign") state = "conflict"; else if (skillState === "absent" && agentsState === "absent") state = "not-installed"; else if (skillState === "absent" || agentsState === "absent") state = "partial"; else if (skillState === "current" && agentsState === "current" && installedMode !== null - && (agentsInspection.kind !== "managed" || agentsInspection.content === (installedMode === "balanced" ? balanced.agentsBlockText : orchestrator.agentsBlockText))) { + && (installedMode === "balanced" ? balancedPolicyCurrent : orchestratorPolicyCurrent)) { state = "current"; } else state = "update-available"; @@ -417,7 +432,7 @@ function buildInspection(deps: CodexDelegationInstallerDeps): InspectionContext activation: overrideState === "unsafe" ? "unknown" : overrideState === "active" ? "shadowed" : "effective", ...previewsAndPrompts(), }; - return { paths, skill, agents, agentsInspection, status }; + return { paths, skill, agents, agentsInspection, compatibilityCollision, status }; } export function inspectCodexDelegation(deps: CodexDelegationInstallerDeps = {}): CodexDelegationStatus { @@ -609,6 +624,9 @@ export function mutateCodexDelegation( if (context.status.artifacts.skill.state === "foreign") { return { ok: false, changed: false, reason: "foreign_skill", status: context.status }; } + if (mutation.action === "install" && context.compatibilityCollision) { + return { ok: false, changed: false, reason: "foreign_skill", status: context.status }; + } if (context.status.artifacts.agentsPolicy.state === "foreign") { return { ok: false, changed: false, reason: "ambiguous_agents_markers", status: context.status }; } diff --git a/tests/codex-delegation-installer.test.ts b/tests/codex-delegation-installer.test.ts index 66121cd434..cadec36c54 100644 --- a/tests/codex-delegation-installer.test.ts +++ b/tests/codex-delegation-installer.test.ts @@ -136,6 +136,19 @@ describe("Codex delegation installer", () => { expect(readFileSync(fx.agentsPath, "utf8")).toBe(`prefix\r\n${orchestrator.replaceAll("\n", "\r\n")}\r\nsuffix\r\n`); }); + test("a current CRLF policy inspects as current", () => { + const fx = fixture(); + const bundle = renderCodexDelegationBundle("balanced"); + write(fx.skillPath, bundle.skillText); + write(fx.agentsPath, bundle.agentsBlockText.replaceAll("\n", "\r\n")); + + expect(inspectCodexDelegation(fx.deps)).toMatchObject({ + state: "current", + installedMode: "balanced", + artifacts: { skill: { state: "current" }, agentsPolicy: { state: "current" } }, + }); + }); + test("uninstall removes AGENTS block first, then only SKILL.md, and rmdir only when empty", () => { const fx = fixture(); mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); @@ -225,16 +238,56 @@ describe("Codex delegation installer", () => { expect(existsSync(fx.skillPath)).toBe(false); }); - test("a duplicate compatibility-root skill refuses without writes", () => { + test("a duplicate compatibility-root skill refuses install without reclassifying the managed user skill", () => { const fx = fixture(); - write(fx.compatSkillPath, renderCodexDelegationBundle("balanced").skillText); - expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps)).toMatchObject({ + mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + const compatibilityBytes = Buffer.from("compatibility collision"); + write(fx.compatSkillPath, compatibilityBytes); + + expect(inspectCodexDelegation(fx.deps)).toMatchObject({ + state: "conflict", + artifacts: { skill: { state: "current" }, agentsPolicy: { state: "current" } }, + }); + expect(mutateCodexDelegation({ action: "install", mode: "orchestrator" }, fx.deps)).toMatchObject({ ok: false, changed: false, reason: "foreign_skill", }); + expect(readFileSync(fx.skillPath, "utf8")).toBe(renderCodexDelegationBundle("balanced").skillText); + expect(readFileSync(fx.compatSkillPath)).toEqual(compatibilityBytes); + }); + + test("managed uninstall succeeds while a compatibility-root collision remains untouched", () => { + const fx = fixture(); + mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + const compatibilityBytes = Buffer.from("compatibility collision"); + write(fx.compatSkillPath, compatibilityBytes); + + expect(mutateCodexDelegation({ action: "uninstall" }, fx.deps)).toMatchObject({ ok: true, changed: true }); expect(existsSync(fx.skillPath)).toBe(false); - expect(existsSync(fx.agentsPath)).toBe(false); + expect(readFileSync(fx.agentsPath, "utf8")).toBe(""); + expect(readFileSync(fx.compatSkillPath)).toEqual(compatibilityBytes); + }); + + test.each([ + ["missing managed-version", "---\nname: codexcommander-delegation\nmetadata:\n managed-by: codexcommander\n---\nforeign\n"], + ["managed-by outside metadata", "---\nname: codexcommander-delegation\nownership:\n managed-by: codexcommander\nmetadata:\n managed-version: \"0\"\n---\nforeign\n"], + ["non-scalar metadata", "---\nname: codexcommander-delegation\nmetadata: codexcommander\n managed-by: codexcommander\n managed-version: \"0\"\n---\nforeign\n"], + ])("%s never proves skill ownership for install or uninstall", (_name, foreignSkill) => { + for (const mutation of [{ action: "install", mode: "balanced" }, { action: "uninstall" }] as const) { + const fx = fixture(); + write(fx.skillPath, foreignSkill); + const agents = renderCodexDelegationBundle("balanced").agentsBlockText; + write(fx.agentsPath, agents); + + expect(mutateCodexDelegation(mutation, fx.deps)).toMatchObject({ + ok: false, + changed: false, + reason: "foreign_skill", + }); + expect(readFileSync(fx.skillPath, "utf8")).toBe(foreignSkill); + expect(readFileSync(fx.agentsPath, "utf8")).toBe(agents); + } }); test.each(["symlink parent", "symlink leaf", "hardlink", "directory leaf"])("%s refuses as unsafe", (shape) => { From 681c6378c8d5f956536dabed861118d8660d59b4 Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 13:19:52 -0400 Subject: [PATCH 04/25] feat: expose Codex delegation management --- src/server/management-api.ts | 2 + src/server/management/context.ts | 4 + src/server/management/delegation-routes.ts | 150 +++++++++++++++++ tests/codex-delegation-api.test.ts | 184 +++++++++++++++++++++ tests/server-management-auth.test.ts | 102 ++++++++++++ 5 files changed, 442 insertions(+) create mode 100644 src/server/management/delegation-routes.ts create mode 100644 tests/codex-delegation-api.test.ts diff --git a/src/server/management-api.ts b/src/server/management-api.ts index 1fd4f557ff..aead685372 100644 --- a/src/server/management-api.ts +++ b/src/server/management-api.ts @@ -20,6 +20,7 @@ import { handleRoutingProfileRoutes } from "./management/routing-profile-routes" import { handleProviderRoutes } from "./management/provider-routes"; import { handleModelRoutes } from "./management/model-routes"; import { handleAgentSettingsRoutes } from "./management/agent-settings-routes"; +import { handleDelegationRoutes } from "./management/delegation-routes"; import { handleOauthAccountRoutes } from "./management/oauth-account-routes"; import { handleComboRoutes } from "./management/combo-routes"; import { handleSystemRoutes } from "./management/system-routes"; @@ -180,6 +181,7 @@ export async function handleManagementAPI( ?? (await handleModelRoutes(ctx)) ?? (await handleNativeIntegrationRoutes(ctx)) ?? (await handleAgentSettingsRoutes(ctx)) + ?? (await handleDelegationRoutes(ctx)) ?? (await handleCatalogActivationRoutes(ctx)) ?? (await handleOauthAccountRoutes(ctx)) ?? (await handleComboRoutes(ctx)) diff --git a/src/server/management/context.ts b/src/server/management/context.ts index 4d36e3de8f..b157eb2833 100644 --- a/src/server/management/context.ts +++ b/src/server/management/context.ts @@ -27,8 +27,12 @@ import type { ProxyLifecycleAuthority, } from "../proxy-lifecycle-authority"; import type { ProxyLifecycleLockLease } from "../proxy-lifecycle-protocol"; +import type { inspectCodexDelegation, mutateCodexDelegation } from "../../codex/delegation-installer"; export interface ManagementApiDeps { + /** Delegation installer seams keep management-route tests off user-owned homes. */ + inspectCodexDelegation?: typeof inspectCodexDelegation; + mutateCodexDelegation?: typeof mutateCodexDelegation; /** Shared lifecycle seams for Stop, native routing, and full-sync serialization tests. */ proxyStopLifecycle?: { acquireAuthority?: ( diff --git a/src/server/management/delegation-routes.ts b/src/server/management/delegation-routes.ts new file mode 100644 index 0000000000..532b5337f4 --- /dev/null +++ b/src/server/management/delegation-routes.ts @@ -0,0 +1,150 @@ +import type { + CodexDelegationMutationOutcome, + CodexDelegationStatus, + DelegationArtifactStatus, +} from "../../codex/delegation-installer"; +import { jsonResponse } from "../auth-cors"; +import { managementBodyTooLargeResponse, readManagementJsonBody } from "./body"; +import type { ManagementContext } from "./context"; +import { isPlainRecord } from "./shared"; + +const SKILL_DISPLAY_PATH = "$HOME/.agents/skills/codexcommander-delegation/SKILL.md" as const; +const AGENTS_DISPLAY_PATH = "$CODEX_HOME/AGENTS.md" as const; + +function noStore(response: Response): Response { + response.headers.set("Cache-Control", "no-store"); + return response; +} + +function response(ctx: ManagementContext, data: unknown, status = 200): Response { + return noStore(jsonResponse(data, status, ctx.req, ctx.config)); +} + +/** + * The installer is permitted to inspect user-owned files. Its API projection + * is not: paths remain symbolic and refusal detail is a fixed reason code. + */ +function projectArtifact( + artifact: DelegationArtifactStatus, + displayPath: DelegationArtifactStatus["displayPath"], +): DelegationArtifactStatus { + const reason = artifact.state === "foreign" ? "ownership_conflict" + : artifact.state === "unsafe" ? "unsafe_path" + : undefined; + return { state: artifact.state, displayPath, ...(reason ? { reason } : {}) }; +} + +function projectStatus(status: CodexDelegationStatus): CodexDelegationStatus { + return { + schemaVersion: 1, + state: status.state, + installedMode: status.installedMode, + artifacts: { + skill: projectArtifact(status.artifacts.skill, SKILL_DISPLAY_PATH), + agentsPolicy: projectArtifact(status.artifacts.agentsPolicy, AGENTS_DISPLAY_PATH), + }, + override: { state: status.override.state }, + activation: status.activation, + // These values are rendered from the bundled templates by Task 2; they do + // not reflect any AGENTS file content that was inspected on disk. + previews: status.previews, + copyPrompts: status.copyPrompts, + }; +} + +async function inspector(ctx: ManagementContext): Promise { + if (ctx.deps.inspectCodexDelegation) return ctx.deps.inspectCodexDelegation; + return (await import("../../codex/delegation-installer")).inspectCodexDelegation; +} + +async function mutator(ctx: ManagementContext): Promise { + if (ctx.deps.mutateCodexDelegation) return ctx.deps.mutateCodexDelegation; + return (await import("../../codex/delegation-installer")).mutateCodexDelegation; +} + +function outcomeResponse(ctx: ManagementContext, outcome: CodexDelegationMutationOutcome): Response { + const status = projectStatus(outcome.status); + if (outcome.ok) return response(ctx, { ok: true, changed: outcome.changed, status }); + + if (outcome.reason === "mutation_busy") { + const busy = response(ctx, { + ok: false, + changed: false, + reason: "mutation_busy", + status, + }, 503); + busy.headers.set("Retry-After", "1"); + return busy; + } + const statusCode = outcome.reason === "partial_write" || outcome.reason === "write_failed" ? 500 : 409; + return response(ctx, { + ok: false, + changed: outcome.changed, + reason: outcome.reason, + status, + }, statusCode); +} + +async function hasDeleteBody(ctx: ManagementContext): Promise { + if (ctx.req.body === null) return false; + try { + await readManagementJsonBody(ctx.req); + } catch (error) { + const tooLarge = managementBodyTooLargeResponse(error, ctx.req, ctx.config); + if (tooLarge) return noStore(tooLarge); + } + return true; +} + +export async function handleDelegationRoutes(ctx: ManagementContext): Promise { + if (ctx.url.pathname !== "/api/codex-delegation") return null; + + if (ctx.req.method === "GET") { + try { + return response(ctx, projectStatus((await inspector(ctx))())); + } catch { + return response(ctx, { error: "Codex delegation inspection failed.", reason: "unsafe_path" }, 500); + } + } + + if (ctx.req.method !== "PUT" && ctx.req.method !== "DELETE") return null; + // This check intentionally precedes body consumption: raw-admin callers and + // direct-dispatch fixtures are not allowed to probe or mutate user policy. + if (ctx.principal !== "confirmed-gui-session") { + return response(ctx, { error: "Codex delegation changes require a confirmed dashboard launch." }, 403); + } + + let mutation: Parameters>>[0]; + if (ctx.req.method === "PUT") { + let raw: unknown; + try { + raw = await readManagementJsonBody(ctx.req); + } catch (error) { + const tooLarge = managementBodyTooLargeResponse(error, ctx.req, ctx.config); + if (tooLarge) return noStore(tooLarge); + return response(ctx, { error: "invalid JSON body" }, 400); + } + if (!isPlainRecord(raw) || Object.keys(raw).length !== 1 || Object.keys(raw)[0] !== "mode") { + return response(ctx, { error: "body must contain only mode" }, 400); + } + if (raw.mode !== "balanced" && raw.mode !== "orchestrator") { + return response(ctx, { error: "mode must be balanced or orchestrator" }, 400); + } + mutation = { action: "install", mode: raw.mode }; + } else { + try { + const body = await hasDeleteBody(ctx); + if (body instanceof Response) return body; + if (body) return response(ctx, { error: "DELETE does not accept a request body" }, 400); + } catch { + return response(ctx, { error: "DELETE does not accept a request body" }, 400); + } + mutation = { action: "uninstall" }; + } + + try { + return outcomeResponse(ctx, (await mutator(ctx))(mutation)); + } catch { + return response(ctx, { error: "Codex delegation mutation failed.", changed: false, reason: "write_failed" }, 500); + } +} diff --git a/tests/codex-delegation-api.test.ts b/tests/codex-delegation-api.test.ts new file mode 100644 index 0000000000..4b01a80c4f --- /dev/null +++ b/tests/codex-delegation-api.test.ts @@ -0,0 +1,184 @@ +import { expect, test } from "bun:test"; +import { handleManagementAPI } from "../src/server/management-api"; +import type { + CodexDelegationMutation, + CodexDelegationMutationOutcome, + CodexDelegationStatus, +} from "../src/codex/delegation-installer"; +import type { CodexCommanderConfig } from "../src/types"; + +const config: CodexCommanderConfig = { + port: 10100, + hostname: "127.0.0.1", + defaultProvider: "test", + providers: { + test: { + adapter: "openai-chat", + baseUrl: "https://example.test/v1", + disabled: true, + models: ["gpt-test"], + }, + }, +}; + +function status(overrides: Partial = {}): CodexDelegationStatus { + return { + schemaVersion: 1, + state: "not-installed", + installedMode: null, + artifacts: { + skill: { + state: "absent", + displayPath: "$HOME/.agents/skills/codexcommander-delegation/SKILL.md", + }, + agentsPolicy: { state: "absent", displayPath: "$CODEX_HOME/AGENTS.md" }, + }, + override: { state: "absent" }, + activation: "effective", + previews: { + balanced: { skillText: "managed skill", agentsBlockText: "managed AGENTS block" }, + orchestrator: { skillText: "managed skill", agentsBlockText: "managed AGENTS block" }, + }, + copyPrompts: { balanced: "managed copy prompt", orchestrator: "managed copy prompt" }, + ...overrides, + }; +} + +function successStatus(): CodexDelegationMutationOutcome { + return { ok: true, changed: true, status: status({ state: "current", installedMode: "orchestrator" }) }; +} + +function failure(reason: Extract ["reason"], changed = false): CodexDelegationMutationOutcome { + return { ok: false, changed, reason, status: status({ state: reason === "foreign_skill" ? "conflict" : "unsafe" }) }; +} + +function request(method: string, body?: unknown): Request { + const headers = new Headers({ Host: "127.0.0.1:10100" }); + if (body !== undefined) headers.set("content-type", "application/json"); + return new Request("http://127.0.0.1:10100/api/codex-delegation", { + method, + headers, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }); +} + +function makeDispatch(outcome: CodexDelegationMutationOutcome = successStatus()) { + const mutations: CodexDelegationMutation[] = []; + return { + mutations, + dispatch: async ( + method: string, + body: unknown, + principal?: "admin-token" | "confirmed-gui-session", + ): Promise => { + const req = request(method, body); + const response = await handleManagementAPI(req, new URL(req.url), config, { + inspectCodexDelegation: () => status(), + mutateCodexDelegation: mutation => { + mutations.push(mutation); + return outcome; + }, + }, principal); + if (!response) throw new Error("delegation route was not registered"); + return response; + }, + }; +} + +test("GET is read-only and no-store", async () => { + const { dispatch, mutations } = makeDispatch(); + const response = await dispatch("GET", undefined, "admin-token"); + expect(response.status).toBe(200); + expect(response.headers.get("Cache-Control")).toBe("no-store"); + expect(mutations).toEqual([]); +}); + +test.each(["admin-token", undefined] as const)("PUT rejects %s principal", async principal => { + const { dispatch, mutations } = makeDispatch(); + const response = await dispatch("PUT", { mode: "balanced" }, principal); + expect(response.status).toBe(403); + expect(response.headers.get("Cache-Control")).toBe("no-store"); + expect(mutations).toEqual([]); +}); + +test("PUT rejects an unconfirmed principal before parsing its body", async () => { + const { mutations } = makeDispatch(); + const req = new Request("http://127.0.0.1:10100/api/codex-delegation", { + method: "PUT", + headers: { Host: "127.0.0.1:10100", "content-type": "application/json" }, + body: "not JSON", + }); + const response = await handleManagementAPI(req, new URL(req.url), config, { + inspectCodexDelegation: () => status(), + mutateCodexDelegation: mutation => { + mutations.push(mutation); + return successStatus(); + }, + }, "admin-token"); + expect(response?.status).toBe(403); + expect(mutations).toEqual([]); +}); + +test("confirmed GUI may install an exact mode", async () => { + const { dispatch, mutations } = makeDispatch(); + const response = await dispatch("PUT", { mode: "orchestrator" }, "confirmed-gui-session"); + expect(response.status).toBe(200); + expect(mutations).toEqual([{ action: "install", mode: "orchestrator" }]); +}); + +test("confirmed GUI may uninstall without a body", async () => { + const { dispatch, mutations } = makeDispatch(); + const response = await dispatch("DELETE", undefined, "confirmed-gui-session"); + expect(response.status).toBe(200); + expect(mutations).toEqual([{ action: "uninstall" }]); +}); + +test("PUT rejects a non-exact body before mutation", async () => { + for (const body of [null, [], {}, { mode: "unknown" }, { mode: "balanced", extra: true }]) { + const { dispatch, mutations } = makeDispatch(); + expect((await dispatch("PUT", body, "confirmed-gui-session")).status).toBe(400); + expect(mutations).toEqual([]); + } +}); + +test("DELETE rejects a non-empty body", async () => { + const { dispatch, mutations } = makeDispatch(); + expect((await dispatch("DELETE", {}, "confirmed-gui-session")).status).toBe(400); + expect(mutations).toEqual([]); +}); + +test.each([ + ["foreign_skill", 409, null], + ["unsafe_path", 409, null], + ["changed_during_mutation", 409, null], + ["mutation_busy", 503, "1"], + ["partial_write", 500, null], +] as const)("projects mutation refusal %s", async (reason, expectedStatus, retryAfter) => { + const { dispatch } = makeDispatch(failure(reason, reason === "partial_write")); + const response = await dispatch("PUT", { mode: "balanced" }, "confirmed-gui-session"); + expect(response.status).toBe(expectedStatus); + expect(response.headers.get("Retry-After")).toBe(retryAfter); + const body = await response.json() as { changed?: boolean }; + if (reason === "partial_write") expect(body.changed).toBe(true); +}); + +test("responses keep fixed paths and never include inspected AGENTS content", async () => { + const rawAgents = "untrusted AGENTS content that must never be returned"; + const rawHome = "/Users/example/private-home"; + const req = request("GET"); + const response = await handleManagementAPI(req, new URL(req.url), config, { + inspectCodexDelegation: () => status({ + artifacts: { + skill: { state: "unsafe", displayPath: "$HOME/.agents/skills/codexcommander-delegation/SKILL.md", reason: rawHome }, + agentsPolicy: { state: "unsafe", displayPath: "$CODEX_HOME/AGENTS.md", reason: rawAgents }, + }, + }), + mutateCodexDelegation: () => successStatus(), + }, "admin-token"); + expect(response?.status).toBe(200); + const text = await response?.text(); + expect(text).not.toContain(rawHome); + expect(text).not.toContain(rawAgents); + expect(text).toContain("$HOME/.agents/skills/codexcommander-delegation/SKILL.md"); + expect(text).toContain("$CODEX_HOME/AGENTS.md"); +}); diff --git a/tests/server-management-auth.test.ts b/tests/server-management-auth.test.ts index 4cdaa7c35d..5dab64b489 100644 --- a/tests/server-management-auth.test.ts +++ b/tests/server-management-auth.test.ts @@ -31,6 +31,7 @@ import { } from "../src/lib/local-management-attestation"; import { ATTESTATION_CHALLENGE_HEADER, ATTESTATION_PROOF_HEADER } from "../src/identity"; import { createCodexRuntimeFixture } from "./helpers/codex-runtime-fixture"; +import type { CodexDelegationMutation, CodexDelegationStatus } from "../src/codex/delegation-installer"; const previousHome = process.env.CODEXCOMMANDER_HOME; const previousDataToken = process.env.CODEXCOMMANDER_API_AUTH_TOKEN; @@ -55,6 +56,25 @@ function remoteConfig(): CodexCommanderConfig { }; } +function delegationStatusForManagementAuthTest(): CodexDelegationStatus { + return { + schemaVersion: 1, + state: "not-installed", + installedMode: null, + artifacts: { + skill: { state: "absent", displayPath: "$HOME/.agents/skills/codexcommander-delegation/SKILL.md" }, + agentsPolicy: { state: "absent", displayPath: "$CODEX_HOME/AGENTS.md" }, + }, + override: { state: "absent" }, + activation: "effective", + previews: { + balanced: { skillText: "managed skill", agentsBlockText: "managed AGENTS block" }, + orchestrator: { skillText: "managed skill", agentsBlockText: "managed AGENTS block" }, + }, + copyPrompts: { balanced: "managed copy prompt", orchestrator: "managed copy prompt" }, + }; +} + function websocketHandshakeOpens(url: URL, token: string): Promise { return new Promise(resolve => { const target = new URL("/v1/responses", url); @@ -814,6 +834,88 @@ describe("management and data-plane credential separation", () => { } }); + test("Codex delegation changes require both a confirmed GUI session and its origin/CSRF proof", async () => { + delete process.env.CODEXCOMMANDER_API_AUTH_TOKEN; + const config = remoteConfig(); + config.hostname = "127.0.0.1"; + saveConfig(config); + const state = initializeManagementAuthState(config); + const mutations: CodexDelegationMutation[] = []; + const server = startServer(0, { + managementAuthState: state, + managementApi: { + inspectCodexDelegation: delegationStatusForManagementAuthTest, + mutateCodexDelegation: mutation => { + mutations.push(mutation); + return { + ok: true, + changed: true, + status: delegationStatusForManagementAuthTest(), + }; + }, + }, + }); + try { + const mintedResponse = await fetch(new URL("/api/gui-launch-ticket", server.url), { + method: "POST", + headers: { + "content-type": "application/json", + "x-codexcommander-api-key": "admin-secret", + }, + body: JSON.stringify({ route: "subagents" }), + }); + expect(mintedResponse.status).toBe(200); + const minted = await mintedResponse.json() as { ticket: string; route: string }; + const exchangeResponse = await fetch(new URL("/api/gui-launch-exchange", server.url), { + method: "POST", + headers: { Origin: server.url.origin, "content-type": "application/json" }, + body: JSON.stringify({ ticket: minted.ticket, route: minted.route }), + }); + expect(exchangeResponse.status).toBe(200); + const exchanged = await exchangeResponse.json() as { + session: { token: string; csrfToken: string; origin: string }; + }; + + const missingBrowserProof = await fetch(new URL("/api/codex-delegation", server.url), { + method: "PUT", + headers: { + "content-type": "application/json", + "x-codexcommander-api-key": exchanged.session.token, + }, + body: JSON.stringify({ mode: "balanced" }), + }); + expect(missingBrowserProof.status).toBe(401); + expect(mutations).toEqual([]); + + const confirmed = await fetch(new URL("/api/codex-delegation", server.url), { + method: "PUT", + headers: { + Origin: exchanged.session.origin, + "content-type": "application/json", + "x-codexcommander-api-key": exchanged.session.token, + "x-codexcommander-gui-origin": exchanged.session.origin, + "x-codexcommander-csrf-token": exchanged.session.csrfToken, + }, + body: JSON.stringify({ mode: "orchestrator" }), + }); + expect(confirmed.status).toBe(200); + expect(mutations).toEqual([{ action: "install", mode: "orchestrator" }]); + + const rawAdmin = await fetch(new URL("/api/codex-delegation", server.url), { + method: "PUT", + headers: { + "content-type": "application/json", + "x-codexcommander-api-key": "admin-secret", + }, + body: JSON.stringify({ mode: "balanced" }), + }); + expect(rawAdmin.status).toBe(403); + expect(mutations).toEqual([{ action: "install", mode: "orchestrator" }]); + } finally { + await server.stop(true); + } + }); + test("all local credential shapes are rejected by the upstream-forwarding guard", () => { const config = remoteConfig(); config.apiKeys = [{ From 0291a8a8a87cc0939f59d66d4d4d4d13b3ed784d Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 13:49:01 -0400 Subject: [PATCH 05/25] fix: harden Codex delegation management --- src/server/management/body.ts | 37 +++++++ src/server/management/delegation-routes.ts | 30 ++++-- tests/codex-delegation-api.test.ts | 119 +++++++++++++++++++++ 3 files changed, 178 insertions(+), 8 deletions(-) diff --git a/src/server/management/body.ts b/src/server/management/body.ts index 717f94f7bb..ee91211296 100644 --- a/src/server/management/body.ts +++ b/src/server/management/body.ts @@ -11,6 +11,43 @@ export function readManagementJsonBody(req: Request): Promise { return readBoundedJsonRequestBody(req, MANAGEMENT_JSON_BODY_MAX_BYTES) as Promise; } +/** Read a management body exactly as received, without imposing JSON semantics. */ +export async function readManagementRawBody(req: Request): Promise { + const declaredLength = Number(req.headers.get("content-length") ?? ""); + if (Number.isFinite(declaredLength) && declaredLength > MANAGEMENT_JSON_BODY_MAX_BYTES) { + throw new DecompressedBodyTooLargeError(declaredLength, MANAGEMENT_JSON_BODY_MAX_BYTES); + } + if (req.body === null) return new Uint8Array(); + + const reader = req.body.getReader(); + const chunks: Uint8Array[] = []; + let length = 0; + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + if (!value) continue; + length += value.byteLength; + if (length > MANAGEMENT_JSON_BODY_MAX_BYTES) { + throw new DecompressedBodyTooLargeError(length, MANAGEMENT_JSON_BODY_MAX_BYTES); + } + chunks.push(value); + } + } catch (error) { + try { await reader.cancel(); } catch { /* the original body error wins */ } + throw error; + } finally { + reader.releaseLock(); + } + const body = new Uint8Array(length); + let offset = 0; + for (const chunk of chunks) { + body.set(chunk, offset); + offset += chunk.byteLength; + } + return body; +} + export function managementBodyTooLargeResponse( error: unknown, req: Request, diff --git a/src/server/management/delegation-routes.ts b/src/server/management/delegation-routes.ts index 532b5337f4..4601143f06 100644 --- a/src/server/management/delegation-routes.ts +++ b/src/server/management/delegation-routes.ts @@ -3,13 +3,18 @@ import type { CodexDelegationStatus, DelegationArtifactStatus, } from "../../codex/delegation-installer"; +import { renderCodexDelegationBundle } from "../../codex/delegation-templates"; import { jsonResponse } from "../auth-cors"; -import { managementBodyTooLargeResponse, readManagementJsonBody } from "./body"; +import { managementBodyTooLargeResponse, readManagementJsonBody, readManagementRawBody } from "./body"; import type { ManagementContext } from "./context"; import { isPlainRecord } from "./shared"; const SKILL_DISPLAY_PATH = "$HOME/.agents/skills/codexcommander-delegation/SKILL.md" as const; const AGENTS_DISPLAY_PATH = "$CODEX_HOME/AGENTS.md" as const; +const CANONICAL_BUNDLES = { + balanced: renderCodexDelegationBundle("balanced"), + orchestrator: renderCodexDelegationBundle("orchestrator"), +}; function noStore(response: Response): Response { response.headers.set("Cache-Control", "no-store"); @@ -45,10 +50,20 @@ function projectStatus(status: CodexDelegationStatus): CodexDelegationStatus { }, override: { state: status.override.state }, activation: status.activation, - // These values are rendered from the bundled templates by Task 2; they do - // not reflect any AGENTS file content that was inspected on disk. - previews: status.previews, - copyPrompts: status.copyPrompts, + previews: { + balanced: { + skillText: CANONICAL_BUNDLES.balanced.skillText, + agentsBlockText: CANONICAL_BUNDLES.balanced.agentsBlockText, + }, + orchestrator: { + skillText: CANONICAL_BUNDLES.orchestrator.skillText, + agentsBlockText: CANONICAL_BUNDLES.orchestrator.agentsBlockText, + }, + }, + copyPrompts: { + balanced: CANONICAL_BUNDLES.balanced.copyPrompt, + orchestrator: CANONICAL_BUNDLES.orchestrator.copyPrompt, + }, }; } @@ -86,14 +101,13 @@ function outcomeResponse(ctx: ManagementContext, outcome: CodexDelegationMutatio } async function hasDeleteBody(ctx: ManagementContext): Promise { - if (ctx.req.body === null) return false; try { - await readManagementJsonBody(ctx.req); + return (await readManagementRawBody(ctx.req)).byteLength > 0; } catch (error) { const tooLarge = managementBodyTooLargeResponse(error, ctx.req, ctx.config); if (tooLarge) return noStore(tooLarge); + throw error; } - return true; } export async function handleDelegationRoutes(ctx: ManagementContext): Promise { diff --git a/tests/codex-delegation-api.test.ts b/tests/codex-delegation-api.test.ts index 4b01a80c4f..8e59f055a1 100644 --- a/tests/codex-delegation-api.test.ts +++ b/tests/codex-delegation-api.test.ts @@ -5,6 +5,7 @@ import type { CodexDelegationMutationOutcome, CodexDelegationStatus, } from "../src/codex/delegation-installer"; +import { renderCodexDelegationBundle } from "../src/codex/delegation-templates"; import type { CodexCommanderConfig } from "../src/types"; const config: CodexCommanderConfig = { @@ -62,6 +63,51 @@ function request(method: string, body?: unknown): Request { }); } +function streamedDelete(chunks: string[], headers: HeadersInit = {}): Request { + const encoder = new TextEncoder(); + const body = new ReadableStream({ + start(controller) { + for (const chunk of chunks) controller.enqueue(encoder.encode(chunk)); + controller.close(); + }, + }); + return new Request("http://127.0.0.1:10100/api/codex-delegation", { + method: "DELETE", + headers: { Host: "127.0.0.1:10100", ...headers }, + body, + }); +} + +function bytesDelete(chunks: Uint8Array[]): Request { + const body = new ReadableStream({ + start(controller) { + for (const chunk of chunks) controller.enqueue(chunk); + controller.close(); + }, + }); + return new Request("http://127.0.0.1:10100/api/codex-delegation", { + method: "DELETE", + headers: { Host: "127.0.0.1:10100" }, + body, + }); +} + +async function dispatchRequest( + req: Request, + mutations: CodexDelegationMutation[], + outcome: CodexDelegationMutationOutcome = successStatus(), +): Promise { + const response = await handleManagementAPI(req, new URL(req.url), config, { + inspectCodexDelegation: () => status(), + mutateCodexDelegation: mutation => { + mutations.push(mutation); + return outcome; + }, + }, "confirmed-gui-session"); + if (!response) throw new Error("delegation route was not registered"); + return response; +} + function makeDispatch(outcome: CodexDelegationMutationOutcome = successStatus()) { const mutations: CodexDelegationMutation[] = []; return { @@ -147,6 +193,40 @@ test("DELETE rejects a non-empty body", async () => { expect(mutations).toEqual([]); }); +test.each([ + ["an explicit zero-byte stream", streamedDelete([])], + ["Content-Length: 0", new Request("http://127.0.0.1:10100/api/codex-delegation", { + method: "DELETE", + headers: { Host: "127.0.0.1:10100", "content-length": "0" }, + })], + ["a chunked empty stream", streamedDelete([], { "transfer-encoding": "chunked" })], +] as const)("DELETE accepts %s", async (_name, req) => { + const mutations: CodexDelegationMutation[] = []; + const response = await dispatchRequest(req, mutations); + expect(response.status).toBe(200); + expect(response.headers.get("Cache-Control")).toBe("no-store"); + expect(mutations).toEqual([{ action: "uninstall" }]); +}); + +test.each([ + ["JSON", streamedDelete(["{}"], { "content-type": "application/json" })], + ["non-JSON", streamedDelete(["not JSON"], { "content-type": "text/plain" })], +] as const)("DELETE rejects non-empty %s payloads", async (_name, req) => { + const mutations: CodexDelegationMutation[] = []; + const response = await dispatchRequest(req, mutations); + expect(response.status).toBe(400); + expect(response.headers.get("Cache-Control")).toBe("no-store"); + expect(mutations).toEqual([]); +}); + +test("DELETE returns no-store 413 when its streamed body exceeds the management bound", async () => { + const mutations: CodexDelegationMutation[] = []; + const response = await dispatchRequest(bytesDelete([new Uint8Array(4 * 1024 * 1024 + 1)]), mutations); + expect(response.status).toBe(413); + expect(response.headers.get("Cache-Control")).toBe("no-store"); + expect(mutations).toEqual([]); +}); + test.each([ ["foreign_skill", 409, null], ["unsafe_path", 409, null], @@ -182,3 +262,42 @@ test("responses keep fixed paths and never include inspected AGENTS content", as expect(text).toContain("$HOME/.agents/skills/codexcommander-delegation/SKILL.md"); expect(text).toContain("$CODEX_HOME/AGENTS.md"); }); + +test("GET and mutation outcomes replace injected previews and prompts with canonical templates", async () => { + const secret = "injected-secret-preview"; + const rawAgents = "injected-raw-agents"; + const rawPrompt = "injected-prompt"; + const injected = status({ + previews: { + balanced: { skillText: secret, agentsBlockText: rawAgents }, + orchestrator: { skillText: secret, agentsBlockText: rawAgents }, + }, + copyPrompts: { balanced: rawPrompt, orchestrator: rawPrompt }, + }); + const get = request("GET"); + const getResponse = await handleManagementAPI(get, new URL(get.url), config, { + inspectCodexDelegation: () => injected, + mutateCodexDelegation: () => ({ ok: true, changed: true, status: injected }), + }, "admin-token"); + const put = request("PUT", { mode: "balanced" }); + const putResponse = await handleManagementAPI(put, new URL(put.url), config, { + inspectCodexDelegation: () => injected, + mutateCodexDelegation: () => ({ ok: true, changed: true, status: injected }), + }, "confirmed-gui-session"); + const canonical = renderCodexDelegationBundle("balanced"); + const canonicalOrchestrator = renderCodexDelegationBundle("orchestrator"); + for (const response of [getResponse, putResponse]) { + expect(response?.status).toBe(200); + const body = await response?.json() as CodexDelegationStatus | { status: CodexDelegationStatus }; + const projected = "status" in body ? body.status : body; + const text = JSON.stringify(body); + expect(text).not.toContain(secret); + expect(text).not.toContain(rawAgents); + expect(text).not.toContain(rawPrompt); + expect(projected.previews.balanced.skillText).toBe(canonical.skillText); + expect(projected.previews.balanced.agentsBlockText).toBe(canonical.agentsBlockText); + expect(projected.copyPrompts.balanced).toBe(canonical.copyPrompt); + expect(projected.previews.orchestrator.agentsBlockText).toBe(canonicalOrchestrator.agentsBlockText); + expect(projected.copyPrompts.orchestrator).toBe(canonicalOrchestrator.copyPrompt); + } +}); From a8203f9be1d3a18fdaf74cd4a51a9081f0456c6c Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 14:09:00 -0400 Subject: [PATCH 06/25] feat: add Codex delegation setup card --- .../CodexDelegationSetupCard.tsx | 88 ++++++++ .../SubagentsWorkspace.tsx | 5 + gui/src/i18n/de.ts | 37 ++++ gui/src/i18n/en.ts | 37 ++++ gui/src/i18n/ja.ts | 37 ++++ gui/src/i18n/ko.ts | 37 ++++ gui/src/i18n/ru.ts | 37 ++++ gui/src/i18n/zh.ts | 37 ++++ gui/src/pages/Subagents.tsx | 3 + gui/src/pages/use-codex-delegation-setup.ts | 113 ++++++++++ gui/src/styles-subagents-workspace.css | 33 +++ gui/tests/codex-delegation-setup.test.tsx | 201 ++++++++++++++++++ gui/tests/subagents-classic.test.ts | 14 ++ 13 files changed, 679 insertions(+) create mode 100644 gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx create mode 100644 gui/src/pages/use-codex-delegation-setup.ts create mode 100644 gui/tests/codex-delegation-setup.test.tsx diff --git a/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx new file mode 100644 index 0000000000..3e5690caf2 --- /dev/null +++ b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx @@ -0,0 +1,88 @@ +import { useEffect, useRef, useState } from "react"; +import { useT, type TKey } from "../../i18n/shared"; +import { useCopyFeedback } from "../use-copy-feedback"; +import type { CodexDelegationSetupController, CodexDelegationStatus } from "../../pages/use-codex-delegation-setup"; + +function statusKey(status: CodexDelegationStatus): TKey { + if (status.state === "current" && status.activation === "effective") return "sub.delegationSetup.statusReady"; + if (status.state === "current" && status.activation === "shadowed") return "sub.delegationSetup.statusShadowed"; + const keys: Record = { + "not-installed": "sub.delegationSetup.statusNotInstalled", current: "sub.delegationSetup.statusInstalled", + "update-available": "sub.delegationSetup.statusUpdate", partial: "sub.delegationSetup.statusPartial", + conflict: "sub.delegationSetup.statusConflict", unsafe: "sub.delegationSetup.statusUnsafe", + }; + return keys[status.state]; +} + +function blockedReason(status: CodexDelegationStatus): TKey { + const reason = status.artifacts.skill.reason ?? status.artifacts.agentsPolicy.reason; + return reason === "ownership_conflict" ? "sub.delegationSetup.reasonConflict" : "sub.delegationSetup.reasonUnsafe"; +} + +export default function CodexDelegationSetupCard({ delegationSetup }: { delegationSetup: CodexDelegationSetupController }) { + const t = useT(); + const { loaded, status, selectedMode, busy, error, setSelectedMode, install, uninstall } = delegationSetup; + const [previewOpen, setPreviewOpen] = useState(false); + const [removeOpen, setRemoveOpen] = useState(false); + const [previewMode, setPreviewMode] = useState(selectedMode); + const previewTriggerRef = useRef(null); + const removeTriggerRef = useRef(null); + const copyFeedback = useCopyFeedback(); + const blocked = status?.state === "conflict" || status?.state === "unsafe"; + const canMutate = loaded && !!status && !blocked && !busy; + const installed = status?.state === "current"; + const primaryKey = status?.state === "update-available" ? "sub.delegationSetup.update" + : status?.state === "partial" ? "sub.delegationSetup.repair" : "sub.delegationSetup.install"; + const prompt = status?.copyPrompts[previewMode] ?? ""; + const copyOutcome = copyFeedback.outcomeFor(prompt); + + const closePreview = () => { setPreviewOpen(false); setTimeout(() => previewTriggerRef.current?.focus(), 0); }; + const closeRemove = () => { setRemoveOpen(false); setTimeout(() => removeTriggerRef.current?.focus(), 0); }; + useEffect(() => { + const onKeyDown = (event: KeyboardEvent) => { + if (event.key !== "Escape") return; + if (previewOpen) closePreview(); + if (removeOpen) closeRemove(); + }; + window.addEventListener("keydown", onKeyDown); + return () => window.removeEventListener("keydown", onKeyDown); + }); + + if (!loaded) return

{t("sub.delegationSetup.loading")}

; + + return ( +
+
+

{t("sub.delegationSetup.title")}

{t("sub.delegationSetup.subtitle")}

+ {status && {t(statusKey(status))}} +
+ {status &&
+
+ {t("sub.delegationSetup.modeLegend")} + {(["balanced", "orchestrator"] as const).map(mode => )} +
+

{t("sub.delegationSetup.liveRoster")}

+
    +
  • {t("sub.delegationSetup.skillArtifact")}{status.artifacts.skill.displayPath}
    • +
  • {t("sub.delegationSetup.agentsArtifact")}{status.artifacts.agentsPolicy.displayPath}
    • +
+ {blocked &&

{t(blockedReason(status))}

} + {error &&

{t("sub.delegationSetup.error")}

} +
+ + {!installed && } + {installed && } + {installed && } +
+ {busy &&

{t("sub.delegationSetup.working")}

} + {!busy && installed &&

{t("sub.delegationSetup.newTask")}

} +
{t("sub.delegationSetup.manual")}

{t("sub.delegationSetup.manualHint")}

+
} + {previewOpen && status &&
event.stopPropagation()}>
{status.previews[previewMode].skillText}
{status.previews[previewMode].agentsBlockText}
} + {removeOpen &&
event.stopPropagation()}>

{t("sub.delegationSetup.removeConfirm")}

} +
+ ); +} diff --git a/gui/src/components/subagents-workspace/SubagentsWorkspace.tsx b/gui/src/components/subagents-workspace/SubagentsWorkspace.tsx index 00d2f203a6..a468aabd80 100644 --- a/gui/src/components/subagents-workspace/SubagentsWorkspace.tsx +++ b/gui/src/components/subagents-workspace/SubagentsWorkspace.tsx @@ -16,6 +16,8 @@ import { formatNamespacedModelId, providerIconSrc } from "../../provider-icons"; import SubagentDelegationSection from "./SubagentDelegationSection"; import type { DelegationPatch, DelegationModelOption } from "../../pages/use-subagent-delegation"; import type { RosterReachability } from "../../pages/subagent-roster-reachability"; +import CodexDelegationSetupCard from "./CodexDelegationSetupCard"; +import type { CodexDelegationSetupController } from "../../pages/use-codex-delegation-setup"; export const FEATURED_MAX = 5; export const LONG_CONTEXT_MIN = 200_000; @@ -68,6 +70,7 @@ export interface SubagentsWorkspaceProps { onSave: (patch: DelegationPatch) => void | Promise; }; runPolicy?: React.ReactNode; + delegationSetup: CodexDelegationSetupController; } function providerFromSelector(selector: string): string { @@ -163,6 +166,7 @@ export default function SubagentsWorkspace({ onSave, delegation, runPolicy, + delegationSetup, }: SubagentsWorkspaceProps) { const t = useT(); const [query, setQuery] = useState(""); @@ -516,6 +520,7 @@ export default function SubagentsWorkspace({ /> )} + ); } diff --git a/gui/src/i18n/de.ts b/gui/src/i18n/de.ts index a83325f2fc..77144b39c9 100644 --- a/gui/src/i18n/de.ts +++ b/gui/src/i18n/de.ts @@ -2197,4 +2197,41 @@ export const de: Record = { "integrations.restoredSuccess": "Die ursprüngliche OpenCode-Konfiguration wurde wiederhergestellt.", "integrations.autoEnabled": "Automatische OpenCode-Verbindung aktiviert.", "integrations.autoDisabled": "Automatische OpenCode-Verbindung deaktiviert.", + "sub.delegationSetup.loading": "Delegationseinrichtung wird geladen…", + "sub.delegationSetup.title": "Codex für diese Liste einrichten", + "sub.delegationSetup.subtitle": "Installiert die aktuelle Listenanleitung für neue Codex-Aufgaben.", + "sub.delegationSetup.statusReady": "Bereit", + "sub.delegationSetup.statusInstalled": "Installiert", + "sub.delegationSetup.statusShadowed": "Installiert, aber AGENTS.override.md ist aktiv", + "sub.delegationSetup.statusNotInstalled": "Nicht installiert", + "sub.delegationSetup.statusUpdate": "Update verfügbar", + "sub.delegationSetup.statusPartial": "Reparatur nötig", + "sub.delegationSetup.statusConflict": "Konflikt", + "sub.delegationSetup.statusUnsafe": "Aufmerksamkeit nötig", + "sub.delegationSetup.modeLegend": "Delegationsmodus", + "sub.delegationSetup.mode.balanced": "Ausgewogen", + "sub.delegationSetup.mode.balancedDescription": "Verwendet einen ausgewogenen Delegationsansatz.", + "sub.delegationSetup.mode.orchestrator": "Orchestrator", + "sub.delegationSetup.mode.orchestratorDescription": "Verwendet einen orchestratorgeführten Delegationsansatz.", + "sub.delegationSetup.liveRoster": "Verwendet die Live-Liste; Modell-IDs werden nicht kopiert.", + "sub.delegationSetup.skillArtifact": "Delegations-Skill", + "sub.delegationSetup.agentsArtifact": "Codex-Anweisungen", + "sub.delegationSetup.preview": "Vorschau", + "sub.delegationSetup.install": "Installieren", + "sub.delegationSetup.update": "Aktualisieren", + "sub.delegationSetup.repair": "Reparieren", + "sub.delegationSetup.changeMode": "Modus ändern", + "sub.delegationSetup.remove": "Entfernen", + "sub.delegationSetup.removeTitle": "Delegationseinrichtung entfernen", + "sub.delegationSetup.removeConfirm": "Die verwaltete Delegationseinrichtung entfernen?", + "sub.delegationSetup.manual": "Installer nicht verfügbar? Manuelle Einrichtung zeigen", + "sub.delegationSetup.manualHint": "Kopieren Sie die vom Server bereitgestellte Eingabeaufforderung für den ausgewählten Modus.", + "sub.delegationSetup.copy": "Einrichtung kopieren", + "sub.delegationSetup.copied": "Kopiert", + "sub.delegationSetup.copyUnavailable": "Kopieren nicht verfügbar", + "sub.delegationSetup.newTask": "Starten Sie eine neue Codex-Aufgabe, um diese Einrichtung zu verwenden.", + "sub.delegationSetup.working": "Wird ausgeführt…", + "sub.delegationSetup.reasonConflict": "Diese Einrichtung kann nicht automatisch geändert werden, weil eine vorhandene Datei nicht von CodexCommander verwaltet wird.", + "sub.delegationSetup.reasonUnsafe": "Diese Einrichtung kann nicht automatisch geändert werden, weil ihre Dateien nicht sicher geprüft werden konnten.", + "sub.delegationSetup.error": "Die Anfrage zur Delegationseinrichtung ist fehlgeschlagen. Versuchen Sie es erneut.", }; diff --git a/gui/src/i18n/en.ts b/gui/src/i18n/en.ts index ac0d866d1d..5051229126 100644 --- a/gui/src/i18n/en.ts +++ b/gui/src/i18n/en.ts @@ -2225,6 +2225,43 @@ export const en = { "integrations.restoredSuccess": "OpenCode's original configuration was restored.", "integrations.autoEnabled": "Automatic OpenCode connection enabled.", "integrations.autoDisabled": "Automatic OpenCode connection disabled.", + "sub.delegationSetup.loading": "Loading delegation setup…", + "sub.delegationSetup.title": "Teach Codex to use this roster", + "sub.delegationSetup.subtitle": "Install the current roster guidance for new Codex tasks.", + "sub.delegationSetup.statusReady": "Ready", + "sub.delegationSetup.statusInstalled": "Installed", + "sub.delegationSetup.statusShadowed": "Installed, but AGENTS.override.md is active", + "sub.delegationSetup.statusNotInstalled": "Not installed", + "sub.delegationSetup.statusUpdate": "Update available", + "sub.delegationSetup.statusPartial": "Needs repair", + "sub.delegationSetup.statusConflict": "Conflict", + "sub.delegationSetup.statusUnsafe": "Needs attention", + "sub.delegationSetup.modeLegend": "Delegation mode", + "sub.delegationSetup.mode.balanced": "Balanced", + "sub.delegationSetup.mode.balancedDescription": "Use a balanced delegation approach.", + "sub.delegationSetup.mode.orchestrator": "Orchestrator", + "sub.delegationSetup.mode.orchestratorDescription": "Use an orchestrator-led delegation approach.", + "sub.delegationSetup.liveRoster": "Uses the live roster; model IDs are not copied into this setup.", + "sub.delegationSetup.skillArtifact": "Delegation skill", + "sub.delegationSetup.agentsArtifact": "Codex instructions", + "sub.delegationSetup.preview": "Preview", + "sub.delegationSetup.install": "Install", + "sub.delegationSetup.update": "Update", + "sub.delegationSetup.repair": "Repair", + "sub.delegationSetup.changeMode": "Change mode", + "sub.delegationSetup.remove": "Remove", + "sub.delegationSetup.removeTitle": "Remove delegation setup", + "sub.delegationSetup.removeConfirm": "Remove the managed delegation setup?", + "sub.delegationSetup.manual": "Installer unavailable? Show manual setup", + "sub.delegationSetup.manualHint": "Copy the server-provided setup prompt for the selected mode.", + "sub.delegationSetup.copy": "Copy setup", + "sub.delegationSetup.copied": "Copied", + "sub.delegationSetup.copyUnavailable": "Copy unavailable", + "sub.delegationSetup.newTask": "Start a new Codex task to use this setup.", + "sub.delegationSetup.working": "Working…", + "sub.delegationSetup.reasonConflict": "This setup can’t be changed automatically because an existing file is not managed by CodexCommander.", + "sub.delegationSetup.reasonUnsafe": "This setup can’t be changed automatically because its files could not be safely verified.", + "sub.delegationSetup.error": "The delegation setup request failed. Try again.", } as const; diff --git a/gui/src/i18n/ja.ts b/gui/src/i18n/ja.ts index 727448dfb9..47fb1db348 100644 --- a/gui/src/i18n/ja.ts +++ b/gui/src/i18n/ja.ts @@ -2217,4 +2217,41 @@ export const ja: Record = { "integrations.restoredSuccess": "OpenCode の元の設定を復元しました。", "integrations.autoEnabled": "OpenCode の自動接続を有効にしました。", "integrations.autoDisabled": "OpenCode の自動接続を無効にしました。", + "sub.delegationSetup.loading": "委任設定を読み込み中…", + "sub.delegationSetup.title": "このロスターを Codex に使わせる", + "sub.delegationSetup.subtitle": "新しい Codex タスクに現在のロスター指示をインストールします。", + "sub.delegationSetup.statusReady": "準備完了", + "sub.delegationSetup.statusInstalled": "インストール済み", + "sub.delegationSetup.statusShadowed": "インストール済みですが、AGENTS.override.md が有効です", + "sub.delegationSetup.statusNotInstalled": "未インストール", + "sub.delegationSetup.statusUpdate": "更新可能", + "sub.delegationSetup.statusPartial": "修復が必要", + "sub.delegationSetup.statusConflict": "競合", + "sub.delegationSetup.statusUnsafe": "注意が必要", + "sub.delegationSetup.modeLegend": "委任モード", + "sub.delegationSetup.mode.balanced": "バランス", + "sub.delegationSetup.mode.balancedDescription": "バランスの取れた委任方法を使用します。", + "sub.delegationSetup.mode.orchestrator": "オーケストレーター", + "sub.delegationSetup.mode.orchestratorDescription": "オーケストレーター主導の委任方法を使用します。", + "sub.delegationSetup.liveRoster": "ライブロスターを使用します。モデル ID はこの設定にコピーされません。", + "sub.delegationSetup.skillArtifact": "委任スキル", + "sub.delegationSetup.agentsArtifact": "Codex 指示", + "sub.delegationSetup.preview": "プレビュー", + "sub.delegationSetup.install": "インストール", + "sub.delegationSetup.update": "更新", + "sub.delegationSetup.repair": "修復", + "sub.delegationSetup.changeMode": "モードを変更", + "sub.delegationSetup.remove": "削除", + "sub.delegationSetup.removeTitle": "委任設定を削除", + "sub.delegationSetup.removeConfirm": "管理された委任設定を削除しますか？", + "sub.delegationSetup.manual": "インストーラーを利用できませんか？ 手動設定を表示", + "sub.delegationSetup.manualHint": "選択したモード用にサーバーが提供した設定プロンプトをコピーします。", + "sub.delegationSetup.copy": "設定をコピー", + "sub.delegationSetup.copied": "コピーしました", + "sub.delegationSetup.copyUnavailable": "コピーできません", + "sub.delegationSetup.newTask": "この設定を使用するには、新しい Codex タスクを開始してください。", + "sub.delegationSetup.working": "処理中…", + "sub.delegationSetup.reasonConflict": "既存のファイルが CodexCommander により管理されていないため、この設定を自動的に変更できません。", + "sub.delegationSetup.reasonUnsafe": "ファイルを安全に検証できないため、この設定を自動的に変更できません。", + "sub.delegationSetup.error": "委任設定リクエストに失敗しました。再試行してください。", }; diff --git a/gui/src/i18n/ko.ts b/gui/src/i18n/ko.ts index 33bdacc82a..e402f36cbc 100644 --- a/gui/src/i18n/ko.ts +++ b/gui/src/i18n/ko.ts @@ -2217,5 +2217,42 @@ export const ko: Record = { "integrations.restoredSuccess": "OpenCode의 원래 설정을 복원했습니다.", "integrations.autoEnabled": "OpenCode 자동 연결을 활성화했습니다.", "integrations.autoDisabled": "OpenCode 자동 연결을 비활성화했습니다.", + "sub.delegationSetup.loading": "위임 설정을 불러오는 중…", + "sub.delegationSetup.title": "Codex가 이 로스터를 사용하도록 설정", + "sub.delegationSetup.subtitle": "새 Codex 작업에 현재 로스터 지침을 설치합니다.", + "sub.delegationSetup.statusReady": "준비됨", + "sub.delegationSetup.statusInstalled": "설치됨", + "sub.delegationSetup.statusShadowed": "설치됨, 하지만 AGENTS.override.md가 활성화됨", + "sub.delegationSetup.statusNotInstalled": "설치되지 않음", + "sub.delegationSetup.statusUpdate": "업데이트 가능", + "sub.delegationSetup.statusPartial": "복구 필요", + "sub.delegationSetup.statusConflict": "충돌", + "sub.delegationSetup.statusUnsafe": "주의 필요", + "sub.delegationSetup.modeLegend": "위임 모드", + "sub.delegationSetup.mode.balanced": "균형", + "sub.delegationSetup.mode.balancedDescription": "균형 잡힌 위임 방식을 사용합니다.", + "sub.delegationSetup.mode.orchestrator": "오케스트레이터", + "sub.delegationSetup.mode.orchestratorDescription": "오케스트레이터 중심 위임 방식을 사용합니다.", + "sub.delegationSetup.liveRoster": "실시간 로스터를 사용하며 모델 ID는 복사되지 않습니다.", + "sub.delegationSetup.skillArtifact": "위임 스킬", + "sub.delegationSetup.agentsArtifact": "Codex 지침", + "sub.delegationSetup.preview": "미리보기", + "sub.delegationSetup.install": "설치", + "sub.delegationSetup.update": "업데이트", + "sub.delegationSetup.repair": "복구", + "sub.delegationSetup.changeMode": "모드 변경", + "sub.delegationSetup.remove": "제거", + "sub.delegationSetup.removeTitle": "위임 설정 제거", + "sub.delegationSetup.removeConfirm": "관리되는 위임 설정을 제거할까요?", + "sub.delegationSetup.manual": "설치 프로그램을 사용할 수 없나요? 수동 설정 표시", + "sub.delegationSetup.manualHint": "선택한 모드의 서버 제공 설정 프롬프트를 복사하세요.", + "sub.delegationSetup.copy": "설정 복사", + "sub.delegationSetup.copied": "복사됨", + "sub.delegationSetup.copyUnavailable": "복사 불가", + "sub.delegationSetup.newTask": "이 설정을 사용하려면 새 Codex 작업을 시작하세요.", + "sub.delegationSetup.working": "작업 중…", + "sub.delegationSetup.reasonConflict": "기존 파일이 CodexCommander에서 관리되지 않아 이 설정을 자동으로 변경할 수 없습니다.", + "sub.delegationSetup.reasonUnsafe": "파일을 안전하게 확인할 수 없어 이 설정을 자동으로 변경할 수 없습니다.", + "sub.delegationSetup.error": "위임 설정 요청에 실패했습니다. 다시 시도하세요.", }; diff --git a/gui/src/i18n/ru.ts b/gui/src/i18n/ru.ts index bd91f38395..3db776f850 100644 --- a/gui/src/i18n/ru.ts +++ b/gui/src/i18n/ru.ts @@ -2219,4 +2219,41 @@ export const ru: Record = { "integrations.restoredSuccess": "Исходная конфигурация OpenCode восстановлена.", "integrations.autoEnabled": "Автоматическое подключение OpenCode включено.", "integrations.autoDisabled": "Автоматическое подключение OpenCode выключено.", + "sub.delegationSetup.loading": "Загрузка настройки делегирования…", + "sub.delegationSetup.title": "Настройте Codex для этого списка", + "sub.delegationSetup.subtitle": "Устанавливает текущие инструкции списка для новых задач Codex.", + "sub.delegationSetup.statusReady": "Готово", + "sub.delegationSetup.statusInstalled": "Установлено", + "sub.delegationSetup.statusShadowed": "Установлено, но AGENTS.override.md активен", + "sub.delegationSetup.statusNotInstalled": "Не установлено", + "sub.delegationSetup.statusUpdate": "Доступно обновление", + "sub.delegationSetup.statusPartial": "Требуется восстановление", + "sub.delegationSetup.statusConflict": "Конфликт", + "sub.delegationSetup.statusUnsafe": "Требует внимания", + "sub.delegationSetup.modeLegend": "Режим делегирования", + "sub.delegationSetup.mode.balanced": "Сбалансированный", + "sub.delegationSetup.mode.balancedDescription": "Использует сбалансированный подход к делегированию.", + "sub.delegationSetup.mode.orchestrator": "Оркестратор", + "sub.delegationSetup.mode.orchestratorDescription": "Использует подход с ведущим оркестратором.", + "sub.delegationSetup.liveRoster": "Использует актуальный список; идентификаторы моделей не копируются.", + "sub.delegationSetup.skillArtifact": "Навык делегирования", + "sub.delegationSetup.agentsArtifact": "Инструкции Codex", + "sub.delegationSetup.preview": "Просмотр", + "sub.delegationSetup.install": "Установить", + "sub.delegationSetup.update": "Обновить", + "sub.delegationSetup.repair": "Восстановить", + "sub.delegationSetup.changeMode": "Сменить режим", + "sub.delegationSetup.remove": "Удалить", + "sub.delegationSetup.removeTitle": "Удалить настройку делегирования", + "sub.delegationSetup.removeConfirm": "Удалить управляемую настройку делегирования?", + "sub.delegationSetup.manual": "Установщик недоступен? Показать ручную настройку", + "sub.delegationSetup.manualHint": "Скопируйте предоставленную сервером подсказку для выбранного режима.", + "sub.delegationSetup.copy": "Копировать настройку", + "sub.delegationSetup.copied": "Скопировано", + "sub.delegationSetup.copyUnavailable": "Копирование недоступно", + "sub.delegationSetup.newTask": "Начните новую задачу Codex, чтобы использовать эту настройку.", + "sub.delegationSetup.working": "Выполняется…", + "sub.delegationSetup.reasonConflict": "Эту настройку нельзя изменить автоматически, потому что существующий файл не управляется CodexCommander.", + "sub.delegationSetup.reasonUnsafe": "Эту настройку нельзя изменить автоматически, потому что её файлы не удалось безопасно проверить.", + "sub.delegationSetup.error": "Запрос настройки делегирования не выполнен. Повторите попытку.", }; diff --git a/gui/src/i18n/zh.ts b/gui/src/i18n/zh.ts index cac576b913..d0b040eecb 100644 --- a/gui/src/i18n/zh.ts +++ b/gui/src/i18n/zh.ts @@ -2217,4 +2217,41 @@ export const zh: Record = { "integrations.restoredSuccess": "已恢复 OpenCode 的原始配置。", "integrations.autoEnabled": "已启用 OpenCode 自动连接。", "integrations.autoDisabled": "已禁用 OpenCode 自动连接。", + "sub.delegationSetup.loading": "正在加载委派设置…", + "sub.delegationSetup.title": "让 Codex 使用此名册", + "sub.delegationSetup.subtitle": "为新的 Codex 任务安装当前名册指导。", + "sub.delegationSetup.statusReady": "就绪", + "sub.delegationSetup.statusInstalled": "已安装", + "sub.delegationSetup.statusShadowed": "已安装，但 AGENTS.override.md 处于活动状态", + "sub.delegationSetup.statusNotInstalled": "未安装", + "sub.delegationSetup.statusUpdate": "有可用更新", + "sub.delegationSetup.statusPartial": "需要修复", + "sub.delegationSetup.statusConflict": "冲突", + "sub.delegationSetup.statusUnsafe": "需要注意", + "sub.delegationSetup.modeLegend": "委派模式", + "sub.delegationSetup.mode.balanced": "平衡", + "sub.delegationSetup.mode.balancedDescription": "使用平衡的委派方式。", + "sub.delegationSetup.mode.orchestrator": "协调器", + "sub.delegationSetup.mode.orchestratorDescription": "使用协调器主导的委派方式。", + "sub.delegationSetup.liveRoster": "使用实时名册；模型 ID 不会复制到此设置中。", + "sub.delegationSetup.skillArtifact": "委派技能", + "sub.delegationSetup.agentsArtifact": "Codex 指令", + "sub.delegationSetup.preview": "预览", + "sub.delegationSetup.install": "安装", + "sub.delegationSetup.update": "更新", + "sub.delegationSetup.repair": "修复", + "sub.delegationSetup.changeMode": "更改模式", + "sub.delegationSetup.remove": "移除", + "sub.delegationSetup.removeTitle": "移除委派设置", + "sub.delegationSetup.removeConfirm": "移除受管理的委派设置？", + "sub.delegationSetup.manual": "安装程序不可用？显示手动设置", + "sub.delegationSetup.manualHint": "复制所选模式的服务器提供设置提示。", + "sub.delegationSetup.copy": "复制设置", + "sub.delegationSetup.copied": "已复制", + "sub.delegationSetup.copyUnavailable": "无法复制", + "sub.delegationSetup.newTask": "开始新的 Codex 任务以使用此设置。", + "sub.delegationSetup.working": "正在处理…", + "sub.delegationSetup.reasonConflict": "现有文件不由 CodexCommander 管理，因此无法自动更改此设置。", + "sub.delegationSetup.reasonUnsafe": "无法安全验证其文件，因此无法自动更改此设置。", + "sub.delegationSetup.error": "委派设置请求失败。请重试。", }; diff --git a/gui/src/pages/Subagents.tsx b/gui/src/pages/Subagents.tsx index eb88dd4e9b..2920e30dbf 100644 --- a/gui/src/pages/Subagents.tsx +++ b/gui/src/pages/Subagents.tsx @@ -12,6 +12,7 @@ import { useDataSurface } from "../data-surface"; import { DataSurfaceSkeleton } from "../components/data-surface"; import { useSubagentDelegation } from "./use-subagent-delegation"; import { useSubagentRunPolicy } from "./use-subagent-run-policy"; +import { useCodexDelegationSetup } from "./use-codex-delegation-setup"; import { deriveRosterReachability, type RosterProjections } from "./subagent-roster-reachability"; import SubagentRunPolicySection from "../components/subagents-workspace/SubagentRunPolicySection"; import { setClientResourceData } from "../client-resource"; @@ -251,6 +252,7 @@ export default function Subagents({ apiBase }: { apiBase: string }) { const busyRef = useRef(busy); const delegation = useSubagentDelegation(apiBase); const runPolicy = useSubagentRunPolicy(apiBase); + const delegationSetup = useCodexDelegationSetup(apiBase); const loadSubagents = useCallback(async (): Promise => { const rosterRequest = fetch(`${apiBase}/api/subagent-models`) @@ -797,6 +799,7 @@ export default function Subagents({ apiBase }: { apiBase: string }) { }} /> )} + delegationSetup={delegationSetup} /> {applyDialog && (
{ if (!busy) setApplyDialog(null); }}> diff --git a/gui/src/pages/use-codex-delegation-setup.ts b/gui/src/pages/use-codex-delegation-setup.ts new file mode 100644 index 0000000000..4ed7573efb --- /dev/null +++ b/gui/src/pages/use-codex-delegation-setup.ts @@ -0,0 +1,113 @@ +import { useCallback, useEffect, useRef, useState } from "react"; + +export type CodexDelegationMode = "balanced" | "orchestrator"; +export type CodexDelegationArtifactState = "absent" | "current" | "outdated" | "foreign" | "unsafe"; + +export interface CodexDelegationStatus { + schemaVersion: 1; + state: "not-installed" | "current" | "update-available" | "partial" | "conflict" | "unsafe"; + installedMode: CodexDelegationMode | null; + artifacts: { + skill: { state: CodexDelegationArtifactState; displayPath: string; reason?: string }; + agentsPolicy: { state: CodexDelegationArtifactState; displayPath: string; reason?: string }; + }; + override: { state: "absent" | "empty" | "active" | "unsafe" }; + activation: "effective" | "shadowed" | "unknown"; + previews: Record; + copyPrompts: Record; +} + +type MutationResponse = { ok?: boolean; status?: CodexDelegationStatus; error?: string }; + +export interface CodexDelegationSetupController { + loaded: boolean; + status: CodexDelegationStatus | null; + selectedMode: CodexDelegationMode; + busy: boolean; + error: string | null; + setSelectedMode(mode: CodexDelegationMode): void; + install(): Promise; + uninstall(): Promise; + reload(): Promise; +} + +function isMode(value: unknown): value is CodexDelegationMode { + return value === "balanced" || value === "orchestrator"; +} + +function isStatus(value: unknown): value is CodexDelegationStatus { + if (!value || typeof value !== "object") return false; + const data = value as Partial; + return data.schemaVersion === 1 && typeof data.state === "string" && (isMode(data.installedMode) || data.installedMode === null); +} + +async function responseError(response: Response): Promise { + try { + const data = await response.json() as MutationResponse; + return typeof data.error === "string" ? data.error : `status=${response.status}`; + } catch { + return `status=${response.status}`; + } +} + +export function useCodexDelegationSetup(apiBase: string): CodexDelegationSetupController { + const [loaded, setLoaded] = useState(false); + const [status, setStatus] = useState(null); + const [selectedMode, setSelectedMode] = useState("balanced"); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const busyRef = useRef(false); + const selectedModeRef = useRef("balanced"); + const chooseMode = useCallback((mode: CodexDelegationMode) => { + selectedModeRef.current = mode; + setSelectedMode(mode); + }, []); + + const reload = useCallback(async () => { + try { + const response = await fetch(`${apiBase}/api/codex-delegation`); + if (!response.ok) throw new Error(await responseError(response)); + const next: unknown = await response.json(); + if (!isStatus(next)) throw new Error("invalid delegation status"); + setStatus(next); + const nextMode = next.installedMode ?? (selectedModeRef.current === "orchestrator" ? "orchestrator" : "balanced"); + selectedModeRef.current = nextMode; + setSelectedMode(nextMode); + setError(null); + } catch (cause) { + setError(cause instanceof Error ? cause.message : String(cause)); + } finally { + setLoaded(true); + } + }, [apiBase]); + + useEffect(() => { + void (async () => { await reload(); })(); + }, [reload]); + + const mutate = useCallback(async (method: "PUT" | "DELETE"): Promise => { + if (busyRef.current) return false; + busyRef.current = true; + setBusy(true); + setError(null); + try { + const response = await fetch(`${apiBase}/api/codex-delegation`, method === "PUT" ? { + method, + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ mode: selectedModeRef.current }), + } : { method }); + const payload = await response.json() as MutationResponse; + if (!response.ok || payload.ok !== true) throw new Error(payload.error ?? `status=${response.status}`); + await reload(); + return true; + } catch (cause) { + setError(cause instanceof Error ? cause.message : String(cause)); + return false; + } finally { + busyRef.current = false; + setBusy(false); + } + }, [apiBase, reload]); + + return { loaded, status, selectedMode, busy, error, setSelectedMode: chooseMode, install: () => mutate("PUT"), uninstall: () => mutate("DELETE"), reload }; +} diff --git a/gui/src/styles-subagents-workspace.css b/gui/src/styles-subagents-workspace.css index 660879f879..50c4f21441 100644 --- a/gui/src/styles-subagents-workspace.css +++ b/gui/src/styles-subagents-workspace.css @@ -336,6 +336,39 @@ margin-top: 1px; } +/* Managed Codex delegation follows the policy card grammar: one clear primary + action, quiet supporting details, and no roster/model data duplicated here. */ +.swi-delegation-setup { grid-column: 1 / -1; } +.swi-delegation-body { display: flex; flex-direction: column; gap: var(--space-3); padding: 0 var(--space-4) var(--space-4); } +.swi-delegation-badge { align-self: flex-start; padding: var(--space-1) var(--space-2); border: 1px solid var(--border); border-radius: var(--radius-pill); color: var(--muted); background: var(--raised); font-size: var(--text-micro); font-weight: var(--weight-medium); } +.swi-delegation-badge--current { color: var(--green); } +.swi-delegation-badge--update-available, .swi-delegation-badge--partial { color: var(--amber); } +.swi-delegation-badge--conflict, .swi-delegation-badge--unsafe { color: var(--red); } +.swi-delegation-modes { display: grid; grid-template-columns: repeat(2, minmax(0, 1fr)); gap: var(--space-2); margin: 0; padding: 0; border: 0; } +.swi-delegation-mode { display: flex; gap: var(--space-2); min-height: var(--control-lg); padding: var(--space-3); border: 1px solid var(--border); border-radius: var(--radius-sm); cursor: pointer; } +.swi-delegation-mode.is-selected { border-color: var(--accent); background: var(--raised); } +.swi-delegation-mode:focus-within { outline: 2px solid var(--accent-ring); outline-offset: 2px; } +.swi-delegation-mode span { display: flex; flex-direction: column; gap: var(--space-1); color: var(--text); font-size: var(--text-label); } +.swi-delegation-mode small, .swi-delegation-note, .swi-delegation-working, .swi-delegation-manual p { color: var(--muted); font-size: var(--text-label); line-height: var(--leading-body); } +.swi-delegation-note, .swi-delegation-working, .swi-delegation-manual p { margin: 0; } +.swi-delegation-artifacts { display: grid; gap: var(--space-1); margin: 0; padding: 0; list-style: none; } +.swi-delegation-artifacts li { display: flex; justify-content: space-between; gap: var(--space-3); padding: var(--space-2); border: 1px solid var(--border-soft); border-radius: var(--radius-xs); color: var(--muted); font-size: var(--text-label); } +.swi-delegation-artifacts code { color: var(--text); overflow-wrap: anywhere; text-align: end; } +.swi-delegation-actions { display: flex; flex-wrap: wrap; gap: var(--space-2); } +.swi-delegation-remove { color: var(--red); } +.swi-delegation-blocked, .swi-delegation-error { margin: 0; padding: var(--space-2); border: 1px solid color-mix(in srgb, var(--red) 35%, var(--border)); border-radius: var(--radius-xs); color: var(--red); font-size: var(--text-label); } +.swi-delegation-manual { border-top: 1px solid var(--border-soft); } +.swi-delegation-manual > summary { min-height: var(--control-lg); padding-top: var(--space-2); color: var(--muted); cursor: pointer; font-size: var(--text-label); font-weight: var(--weight-medium); } +.swi-delegation-manual > summary:hover { color: var(--text); } +.swi-delegation-dialog { max-width: min(680px, calc(100vw - var(--space-6))); } +.swi-delegation-dialog pre { max-height: 230px; overflow: auto; white-space: pre-wrap; overflow-wrap: anywhere; } + +@media (max-width: 700px) { + .swi-delegation-modes { grid-template-columns: 1fr; } + .swi-delegation-artifacts li { align-items: flex-start; flex-direction: column; } + .swi-delegation-artifacts code { text-align: start; } +} + /* Split notice: one quiet info row between the roster and the footer, mirroring the .swi-card-footer rhythm (icon + text + inline action). */ .swi-roster-note { diff --git a/gui/tests/codex-delegation-setup.test.tsx b/gui/tests/codex-delegation-setup.test.tsx new file mode 100644 index 0000000000..1dbf8e31f5 --- /dev/null +++ b/gui/tests/codex-delegation-setup.test.tsx @@ -0,0 +1,201 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { Window } from "happy-dom"; +import { act } from "react"; +import type { Root } from "react-dom/client"; +import CodexDelegationSetupCard from "../src/components/subagents-workspace/CodexDelegationSetupCard"; +import { useCodexDelegationSetup, type CodexDelegationSetupController, type CodexDelegationStatus } from "../src/pages/use-codex-delegation-setup"; +import { LanguageProvider } from "../src/i18n/provider"; + +const globals = ["document", "window", "navigator", "localStorage", "sessionStorage", "fetch", "IS_REACT_ACT_ENVIRONMENT"] as const; +let previousGlobals: Record<(typeof globals)[number], unknown>; +let testWindow: Window; +let container: HTMLElement; +let root: Root | null = null; +let selectedMode: "balanced" | "orchestrator"; +let installed = 0; +let removed = 0; + +function status(state: CodexDelegationStatus["state"] = "not-installed", mode: "balanced" | "orchestrator" | null = null): CodexDelegationStatus { + return { + schemaVersion: 1, + state, + installedMode: mode, + artifacts: { + skill: { state: mode ? "current" : "absent", displayPath: "$HOME/.agents/skills/codexcommander-delegation/SKILL.md" }, + agentsPolicy: { state: mode ? "current" : "absent", displayPath: "$CODEX_HOME/AGENTS.md" }, + }, + override: { state: "absent" }, + activation: "effective", + previews: { + balanced: { skillText: "balanced skill", agentsBlockText: "balanced policy" }, + orchestrator: { skillText: "orchestrator skill", agentsBlockText: "orchestrator policy" }, + }, + copyPrompts: { balanced: "balanced manual prompt", orchestrator: "orchestrator manual prompt" }, + }; +} + +function controller(value: CodexDelegationStatus | null, busy = false): CodexDelegationSetupController { + selectedMode = value?.installedMode ?? "balanced"; + return { + loaded: value !== null, + status: value, + selectedMode, + busy, + error: null, + setSelectedMode: mode => { selectedMode = mode; }, + install: async () => { installed++; return true; }, + uninstall: async () => { removed++; return true; }, + reload: async () => {}, + }; +} + +beforeEach(() => { + previousGlobals = Object.fromEntries(globals.map(k => [k, Reflect.get(globalThis, k)])) as typeof previousGlobals; + testWindow = new Window({ url: "http://localhost/" }); + Object.defineProperty(testWindow.navigator, "language", { configurable: true, value: "en-US" }); + Object.defineProperties(globalThis, { + document: { configurable: true, value: testWindow.document }, window: { configurable: true, value: testWindow }, + navigator: { configurable: true, value: testWindow.navigator }, localStorage: { configurable: true, value: testWindow.localStorage }, + sessionStorage: { configurable: true, value: testWindow.sessionStorage }, + }); + (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + container = testWindow.document.createElement("div") as unknown as HTMLElement; + testWindow.document.body.append(container as never); + installed = 0; + removed = 0; +}); + +afterEach(async () => { + if (root) await act(async () => { root?.unmount(); root = null; }); + for (const key of globals) Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); +}); + +async function mount(value: CodexDelegationStatus | null, busy = false) { + const { createRoot } = await import("react-dom/client"); + await act(async () => { + root = createRoot(container); + root.render(); + }); +} + +function button(label: string): HTMLButtonElement { + const found = Array.from(container.querySelectorAll("button")).find(item => item.textContent?.trim() === label); + if (!found) throw new Error(`Missing button: ${label}`); + return found; +} + +test("loading never makes a false not-installed claim", async () => { + await mount(null); + expect(container.textContent).toContain("Loading"); + expect(container.textContent).not.toContain("Not installed"); +}); + +test("fresh setup uses Balanced and previews before the install mutation", async () => { + await mount(status()); + expect((container.querySelector('input[value="balanced"]') as HTMLInputElement).checked).toBe(true); + await act(async () => { button("Preview").click(); }); + expect(container.querySelector('[role="dialog"]')?.textContent).toContain("balanced skill"); + expect(installed).toBe(0); +}); + +test("orchestrator selection uses its server preview and install flow", async () => { + await mount(status()); + const radio = container.querySelector('input[value="orchestrator"]') as HTMLInputElement; + await act(async () => { radio.click(); }); + await act(async () => { button("Preview").click(); }); + expect(container.querySelector('[role="dialog"]')?.textContent).toContain("orchestrator skill"); +}); + +test("current effective setup exposes ready and installed controls", async () => { + await mount(status("current", "orchestrator")); + expect(container.textContent).toContain("Ready"); + expect(container.textContent).toContain("Orchestrator"); + expect(button("Change mode")).toBeTruthy(); + expect(button("Remove")).toBeTruthy(); +}); + +test("update and partial states use their respective primary actions", async () => { + await mount(status("update-available", "balanced")); + expect(button("Update")).toBeTruthy(); + await act(async () => { root?.unmount(); root = null; }); + await mount(status("partial", "balanced")); + expect(button("Repair")).toBeTruthy(); +}); + +test("conflict and unsafe state fail closed with the projected reason", async () => { + const conflict = status("conflict", null); + conflict.artifacts.skill.reason = "ownership_conflict"; + await mount(conflict); + expect(container.textContent).toContain("can’t be changed automatically"); + expect(button("Install").disabled).toBe(true); +}); + +test("shadowed install is not presented as ready", async () => { + const shadowed = status("current", "balanced"); + shadowed.activation = "shadowed"; + shadowed.override.state = "active"; + await mount(shadowed); + expect(container.textContent).toContain("Installed, but AGENTS.override.md is active"); + expect(container.textContent).not.toContain("Ready"); +}); + +test("remove waits for an accessible confirmation before DELETE", async () => { + await mount(status("current", "balanced")); + await act(async () => { button("Remove").click(); }); + const dialog = container.querySelector('[role="alertdialog"]'); + expect(dialog).toBeTruthy(); + expect(removed).toBe(0); + await act(async () => { Array.from(dialog!.querySelectorAll("button")).find(item => item.textContent?.trim() === "Remove")!.click(); }); + expect(removed).toBe(1); +}); + +test("manual setup stays collapsed and copies only selected server prompt", async () => { + await mount(status()); + const details = container.querySelector("details") as HTMLDetailsElement; + expect(details.open).toBe(false); + expect(container.textContent).toContain("Installer unavailable? Show manual setup"); +}); + +test("busy state disables every setup mutation and exposes a live status", async () => { + await mount(status("current", "balanced"), true); + expect(button("Preview").disabled).toBe(true); + expect(button("Change mode").disabled).toBe(true); + expect(button("Remove").disabled).toBe(true); + expect(container.querySelector('[aria-live="polite"]')?.textContent).toContain("Working"); +}); + +test("preview and remove dialogs restore focus to their triggers", async () => { + await mount(status("current", "balanced")); + const preview = button("Preview"); + preview.focus(); + await act(async () => { preview.click(); }); + await act(async () => { Array.from(container.querySelector('[role="dialog"]')!.querySelectorAll("button")).find(item => item.textContent?.trim() === "Close")!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); + expect(document.activeElement).toBe(preview); + const remove = button("Remove"); + remove.focus(); + await act(async () => { remove.click(); }); + await act(async () => { Array.from(container.querySelector('[role="alertdialog"]')!.querySelectorAll("button")).find(item => item.textContent?.trim() === "Cancel")!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); + expect(document.activeElement).toBe(remove); +}); + +test("hook sends the exact selected PUT body and re-reads the dedicated resource", async () => { + const requests: Array<{ url: string; init?: RequestInit }> = []; + Object.defineProperty(globalThis, "fetch", { + configurable: true, + value: async (url: string, init?: RequestInit) => { + requests.push({ url, init }); + return Response.json(init?.method === "PUT" ? { ok: true, status: status("current", "orchestrator") } : status()); + }, + }); + function Harness() { + const setup = useCodexDelegationSetup("/hook"); + return <>{setup.loaded ? "loaded" : "loading"}; + } + const { createRoot } = await import("react-dom/client"); + await act(async () => { root = createRoot(container); root.render(); await new Promise(resolve => setTimeout(resolve, 0)); }); + await act(async () => { button("Select orchestrator").click(); button("Hook install").click(); await new Promise(resolve => setTimeout(resolve, 0)); }); + const put = requests.find(request => request.init?.method === "PUT"); + expect(put?.url).toBe("/hook/api/codex-delegation"); + expect(put?.init?.body).toBe(JSON.stringify({ mode: "orchestrator" })); + expect(requests.filter(request => request.init?.method === undefined).length).toBe(2); +}); diff --git a/gui/tests/subagents-classic.test.ts b/gui/tests/subagents-classic.test.ts index 3da8391ce3..21a7591df5 100644 --- a/gui/tests/subagents-classic.test.ts +++ b/gui/tests/subagents-classic.test.ts @@ -61,3 +61,17 @@ test("Subagents workspace assets and i18n keys are present", async () => { expect(src).toContain("sub.workspace."); } }); + +test("Subagents places Codex delegation setup after Run Policy with a complete locale family", async () => { + const workspace = await Bun.file(new URL("../src/components/subagents-workspace/SubagentsWorkspace.tsx", import.meta.url)).text(); + const card = await Bun.file(new URL("../src/components/subagents-workspace/CodexDelegationSetupCard.tsx", import.meta.url)).text(); + expect(workspace).toContain('import CodexDelegationSetupCard'); + expect(workspace.indexOf("swi-policy")).toBeLessThan(workspace.indexOf("CodexDelegationSetupCard delegationSetup")); + expect(card).not.toContain(">Teach Codex"); + for (const locale of ["en", "ko", "ja", "de", "ru", "zh"]) { + const src = await Bun.file(new URL(`../src/i18n/${locale}.ts`, import.meta.url)).text(); + expect(src).toContain("sub.delegationSetup.title"); + expect(src).toContain("sub.delegationSetup.manual"); + expect(src).toContain("sub.delegationSetup.reasonUnsafe"); + } +}); From 25a0db8468c73bc25499215a4f9c66bcf731281e Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 14:10:32 -0400 Subject: [PATCH 07/25] test: cover delegation setup workspace --- gui/tests/subagents-classic.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gui/tests/subagents-classic.test.tsx b/gui/tests/subagents-classic.test.tsx index 76fea27727..2a8b8ff592 100644 --- a/gui/tests/subagents-classic.test.tsx +++ b/gui/tests/subagents-classic.test.tsx @@ -170,13 +170,13 @@ function removeButtons(): HTMLButtonElement[] { /^Remove /.test(b.getAttribute("aria-label") ?? "")) as unknown as HTMLButtonElement[]; } -test("renders one configured roster, one agent library, and one run-policy card", async () => { +test("renders one configured roster, one agent library, run policy, and delegation setup", async () => { await mount(); expect(container.querySelector(".subagents-workspace-shell")).toBeTruthy(); - expect(container.querySelectorAll(".subagents-command-card").length).toBe(3); + expect(container.querySelectorAll(".subagents-command-card").length).toBe(4); const headings = Array.from(container.querySelectorAll(".swi-card-title")) .map(node => node.textContent?.trim()); - expect(headings).toEqual(["Configured Roster", "Agent Library", "Run Policy"]); + expect(headings).toEqual(["Configured Roster", "Agent Library", "Run Policy", "Teach Codex to use this roster"]); expect(container.textContent).toContain("Use roster as worker guidance"); expect(container.textContent).toContain("No preferred model"); }); From 78e0ebc8e6ab3dc3c6e01a30187f4b18b28040bb Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 14:43:58 -0400 Subject: [PATCH 08/25] fix: harden Codex delegation setup flow --- .../CodexDelegationSetupCard.tsx | 35 ++++++++++------- gui/src/i18n/de.ts | 3 ++ gui/src/i18n/en.ts | 3 ++ gui/src/i18n/ja.ts | 3 ++ gui/src/i18n/ko.ts | 3 ++ gui/src/i18n/ru.ts | 3 ++ gui/src/i18n/zh.ts | 3 ++ gui/src/pages/use-codex-delegation-setup.ts | 38 +++++++++++++------ gui/tests/codex-delegation-setup.test.tsx | 8 +++- 9 files changed, 72 insertions(+), 27 deletions(-) diff --git a/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx index 3e5690caf2..01d3d0cd18 100644 --- a/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx +++ b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx @@ -24,8 +24,10 @@ export default function CodexDelegationSetupCard({ delegationSetup }: { delegati const { loaded, status, selectedMode, busy, error, setSelectedMode, install, uninstall } = delegationSetup; const [previewOpen, setPreviewOpen] = useState(false); const [removeOpen, setRemoveOpen] = useState(false); - const [previewMode, setPreviewMode] = useState(selectedMode); - const previewTriggerRef = useRef(null); + const [previewApply, setPreviewApply] = useState(false); + const [success, setSuccess] = useState(false); + const openerRef = useRef(null); + const previewConfirmRef = useRef(null); const removeTriggerRef = useRef(null); const copyFeedback = useCopyFeedback(); const blocked = status?.state === "conflict" || status?.state === "unsafe"; @@ -33,21 +35,27 @@ export default function CodexDelegationSetupCard({ delegationSetup }: { delegati const installed = status?.state === "current"; const primaryKey = status?.state === "update-available" ? "sub.delegationSetup.update" : status?.state === "partial" ? "sub.delegationSetup.repair" : "sub.delegationSetup.install"; - const prompt = status?.copyPrompts[previewMode] ?? ""; + const prompt = status?.copyPrompts[selectedMode] ?? ""; const copyOutcome = copyFeedback.outcomeFor(prompt); - const closePreview = () => { setPreviewOpen(false); setTimeout(() => previewTriggerRef.current?.focus(), 0); }; + const closePreview = () => { setPreviewOpen(false); setPreviewApply(false); setTimeout(() => openerRef.current?.focus(), 0); }; const closeRemove = () => { setRemoveOpen(false); setTimeout(() => removeTriggerRef.current?.focus(), 0); }; useEffect(() => { const onKeyDown = (event: KeyboardEvent) => { if (event.key !== "Escape") return; - if (previewOpen) closePreview(); - if (removeOpen) closeRemove(); + if (previewOpen) { event.preventDefault(); closePreview(); } + if (removeOpen) { event.preventDefault(); closeRemove(); } }; window.addEventListener("keydown", onKeyDown); return () => window.removeEventListener("keydown", onKeyDown); }); + useEffect(() => { if (previewOpen) setTimeout(() => (previewApply ? previewConfirmRef.current : document.querySelector(".swi-delegation-dialog button"))?.focus(), 0); }, [previewApply, previewOpen]); + const trap = (event: React.KeyboardEvent) => { if (event.key !== "Tab") return; const buttons = Array.from(event.currentTarget.querySelectorAll("button:not([disabled])")); if (!buttons.length) return; const first = buttons[0]!; const last = buttons.at(-1)!; if (event.shiftKey && document.activeElement === first) { event.preventDefault(); last.focus(); } else if (!event.shiftKey && document.activeElement === last) { event.preventDefault(); first.focus(); } }; + const openPreview = (event: React.MouseEvent, apply: boolean) => { openerRef.current = event.currentTarget; setPreviewApply(apply); setPreviewOpen(true); }; + const runInstall = async () => { if (await install()) { setSuccess(true); closePreview(); } }; + const runRemove = async () => { if (await uninstall()) { setSuccess(true); closeRemove(); } }; + if (!loaded) return

{t("sub.delegationSetup.loading")}

; return ( @@ -56,11 +64,12 @@ export default function CodexDelegationSetupCard({ delegationSetup }: { delegati

{t("sub.delegationSetup.title")}

{t("sub.delegationSetup.subtitle")}

{status && {t(statusKey(status))}}
+ {!status &&

{t("sub.delegationSetup.error")}

} {status &&
{t("sub.delegationSetup.modeLegend")} {(["balanced", "orchestrator"] as const).map(mode => )}
@@ -72,17 +81,17 @@ export default function CodexDelegationSetupCard({ delegationSetup }: { delegati {blocked &&

{t(blockedReason(status))}

} {error &&

{t("sub.delegationSetup.error")}

}
- - {!installed && } - {installed && } + + {!installed && } + {installed && } {installed && }
{busy &&

{t("sub.delegationSetup.working")}

} - {!busy && installed &&

{t("sub.delegationSetup.newTask")}

} + {success &&

{t("sub.delegationSetup.newTask")}

}
{t("sub.delegationSetup.manual")}

{t("sub.delegationSetup.manualHint")}

} - {previewOpen && status &&
event.stopPropagation()}>
{status.previews[previewMode].skillText}
{status.previews[previewMode].agentsBlockText}
} - {removeOpen &&
event.stopPropagation()}>

{t("sub.delegationSetup.removeConfirm")}

} + {previewOpen && status &&
event.stopPropagation()}>

{t("sub.delegationSetup.preview")}

{status.previews[selectedMode].skillText}
{status.previews[selectedMode].agentsBlockText}
{previewApply && }
} + {removeOpen &&
event.stopPropagation()}>

{t("sub.delegationSetup.removeTitle")}

{t("sub.delegationSetup.removeConfirm")}

} ); } diff --git a/gui/src/i18n/de.ts b/gui/src/i18n/de.ts index 77144b39c9..0da1bd2d83 100644 --- a/gui/src/i18n/de.ts +++ b/gui/src/i18n/de.ts @@ -2234,4 +2234,7 @@ export const de: Record = { "sub.delegationSetup.reasonConflict": "Diese Einrichtung kann nicht automatisch geändert werden, weil eine vorhandene Datei nicht von CodexCommander verwaltet wird.", "sub.delegationSetup.reasonUnsafe": "Diese Einrichtung kann nicht automatisch geändert werden, weil ihre Dateien nicht sicher geprüft werden konnten.", "sub.delegationSetup.error": "Die Anfrage zur Delegationseinrichtung ist fehlgeschlagen. Versuchen Sie es erneut.", + "sub.delegationSetup.retry": "Erneut versuchen", + "sub.delegationSetup.close": "Schließen", + "sub.delegationSetup.cancel": "Abbrechen", }; diff --git a/gui/src/i18n/en.ts b/gui/src/i18n/en.ts index 5051229126..14572c88be 100644 --- a/gui/src/i18n/en.ts +++ b/gui/src/i18n/en.ts @@ -2262,6 +2262,9 @@ export const en = { "sub.delegationSetup.reasonConflict": "This setup can’t be changed automatically because an existing file is not managed by CodexCommander.", "sub.delegationSetup.reasonUnsafe": "This setup can’t be changed automatically because its files could not be safely verified.", "sub.delegationSetup.error": "The delegation setup request failed. Try again.", + "sub.delegationSetup.retry": "Retry", + "sub.delegationSetup.close": "Close", + "sub.delegationSetup.cancel": "Cancel", } as const; diff --git a/gui/src/i18n/ja.ts b/gui/src/i18n/ja.ts index 47fb1db348..8cf6ecad6a 100644 --- a/gui/src/i18n/ja.ts +++ b/gui/src/i18n/ja.ts @@ -2254,4 +2254,7 @@ export const ja: Record = { "sub.delegationSetup.reasonConflict": "既存のファイルが CodexCommander により管理されていないため、この設定を自動的に変更できません。", "sub.delegationSetup.reasonUnsafe": "ファイルを安全に検証できないため、この設定を自動的に変更できません。", "sub.delegationSetup.error": "委任設定リクエストに失敗しました。再試行してください。", + "sub.delegationSetup.retry": "再試行", + "sub.delegationSetup.close": "閉じる", + "sub.delegationSetup.cancel": "キャンセル", }; diff --git a/gui/src/i18n/ko.ts b/gui/src/i18n/ko.ts index e402f36cbc..d39b4aa49e 100644 --- a/gui/src/i18n/ko.ts +++ b/gui/src/i18n/ko.ts @@ -2254,5 +2254,8 @@ export const ko: Record = { "sub.delegationSetup.reasonConflict": "기존 파일이 CodexCommander에서 관리되지 않아 이 설정을 자동으로 변경할 수 없습니다.", "sub.delegationSetup.reasonUnsafe": "파일을 안전하게 확인할 수 없어 이 설정을 자동으로 변경할 수 없습니다.", "sub.delegationSetup.error": "위임 설정 요청에 실패했습니다. 다시 시도하세요.", + "sub.delegationSetup.retry": "다시 시도", + "sub.delegationSetup.close": "닫기", + "sub.delegationSetup.cancel": "취소", }; diff --git a/gui/src/i18n/ru.ts b/gui/src/i18n/ru.ts index 3db776f850..c441e6d7bd 100644 --- a/gui/src/i18n/ru.ts +++ b/gui/src/i18n/ru.ts @@ -2256,4 +2256,7 @@ export const ru: Record = { "sub.delegationSetup.reasonConflict": "Эту настройку нельзя изменить автоматически, потому что существующий файл не управляется CodexCommander.", "sub.delegationSetup.reasonUnsafe": "Эту настройку нельзя изменить автоматически, потому что её файлы не удалось безопасно проверить.", "sub.delegationSetup.error": "Запрос настройки делегирования не выполнен. Повторите попытку.", + "sub.delegationSetup.retry": "Повторить", + "sub.delegationSetup.close": "Закрыть", + "sub.delegationSetup.cancel": "Отмена", }; diff --git a/gui/src/i18n/zh.ts b/gui/src/i18n/zh.ts index d0b040eecb..94fd6c659d 100644 --- a/gui/src/i18n/zh.ts +++ b/gui/src/i18n/zh.ts @@ -2254,4 +2254,7 @@ export const zh: Record = { "sub.delegationSetup.reasonConflict": "现有文件不由 CodexCommander 管理，因此无法自动更改此设置。", "sub.delegationSetup.reasonUnsafe": "无法安全验证其文件，因此无法自动更改此设置。", "sub.delegationSetup.error": "委派设置请求失败。请重试。", + "sub.delegationSetup.retry": "重试", + "sub.delegationSetup.close": "关闭", + "sub.delegationSetup.cancel": "取消", }; diff --git a/gui/src/pages/use-codex-delegation-setup.ts b/gui/src/pages/use-codex-delegation-setup.ts index 4ed7573efb..af85aebd99 100644 --- a/gui/src/pages/use-codex-delegation-setup.ts +++ b/gui/src/pages/use-codex-delegation-setup.ts @@ -31,14 +31,18 @@ export interface CodexDelegationSetupController { reload(): Promise; } -function isMode(value: unknown): value is CodexDelegationMode { - return value === "balanced" || value === "orchestrator"; -} +const modes = ["balanced", "orchestrator"] as const; +const states = ["not-installed", "current", "update-available", "partial", "conflict", "unsafe"] as const; +const artifacts = ["absent", "current", "outdated", "foreign", "unsafe"] as const; +const includes = (items: readonly T[], value: unknown): value is T => typeof value === "string" && items.includes(value as T); +const record = (value: unknown): Record | null => value !== null && typeof value === "object" && !Array.isArray(value) ? value as Record : null; function isStatus(value: unknown): value is CodexDelegationStatus { - if (!value || typeof value !== "object") return false; - const data = value as Partial; - return data.schemaVersion === 1 && typeof data.state === "string" && (isMode(data.installedMode) || data.installedMode === null); + const data = record(value); const artifactSet = record(data?.artifacts); const skill = record(artifactSet?.skill); const policy = record(artifactSet?.agentsPolicy); const previews = record(data?.previews); const balanced = record(previews?.balanced); const orchestrator = record(previews?.orchestrator); const prompts = record(data?.copyPrompts); const override = record(data?.override); + return data?.schemaVersion === 1 && includes(states, data.state) && (data.installedMode === null || includes(modes, data.installedMode)) + && !!skill && !!policy && includes(artifacts, skill.state) && includes(artifacts, policy.state) && typeof skill.displayPath === "string" && typeof policy.displayPath === "string" + && !!balanced && !!orchestrator && typeof balanced.skillText === "string" && typeof balanced.agentsBlockText === "string" && typeof orchestrator.skillText === "string" && typeof orchestrator.agentsBlockText === "string" + && !!prompts && typeof prompts.balanced === "string" && typeof prompts.orchestrator === "string" && !!override && includes(["absent", "empty", "active", "unsafe"] as const, override.state) && includes(["effective", "shadowed", "unknown"] as const, data.activation); } async function responseError(response: Response): Promise { @@ -58,31 +62,41 @@ export function useCodexDelegationSetup(apiBase: string): CodexDelegationSetupCo const [error, setError] = useState(null); const busyRef = useRef(false); const selectedModeRef = useRef("balanced"); + const selectedModeInitialized = useRef(false); + const requestRef = useRef(null); const chooseMode = useCallback((mode: CodexDelegationMode) => { selectedModeRef.current = mode; setSelectedMode(mode); }, []); const reload = useCallback(async () => { + requestRef.current?.abort(); + const controller = new AbortController(); + requestRef.current = controller; try { - const response = await fetch(`${apiBase}/api/codex-delegation`); + const response = await fetch(`${apiBase}/api/codex-delegation`, { signal: controller.signal }); if (!response.ok) throw new Error(await responseError(response)); const next: unknown = await response.json(); if (!isStatus(next)) throw new Error("invalid delegation status"); + if (controller.signal.aborted) return; setStatus(next); - const nextMode = next.installedMode ?? (selectedModeRef.current === "orchestrator" ? "orchestrator" : "balanced"); - selectedModeRef.current = nextMode; - setSelectedMode(nextMode); + if (!selectedModeInitialized.current) { + selectedModeInitialized.current = true; + const nextMode = next.installedMode ?? "balanced"; + selectedModeRef.current = nextMode; + setSelectedMode(nextMode); + } setError(null); } catch (cause) { - setError(cause instanceof Error ? cause.message : String(cause)); + if (!controller.signal.aborted) setError(cause instanceof Error ? cause.message : String(cause)); } finally { - setLoaded(true); + if (!controller.signal.aborted) setLoaded(true); } }, [apiBase]); useEffect(() => { void (async () => { await reload(); })(); + return () => requestRef.current?.abort(); }, [reload]); const mutate = useCallback(async (method: "PUT" | "DELETE"): Promise => { diff --git a/gui/tests/codex-delegation-setup.test.tsx b/gui/tests/codex-delegation-setup.test.tsx index 1dbf8e31f5..7a9aa60339 100644 --- a/gui/tests/codex-delegation-setup.test.tsx +++ b/gui/tests/codex-delegation-setup.test.tsx @@ -1,6 +1,6 @@ import { afterEach, beforeEach, expect, test } from "bun:test"; import { Window } from "happy-dom"; -import { act } from "react"; +import { act, useState } from "react"; import type { Root } from "react-dom/client"; import CodexDelegationSetupCard from "../src/components/subagents-workspace/CodexDelegationSetupCard"; import { useCodexDelegationSetup, type CodexDelegationSetupController, type CodexDelegationStatus } from "../src/pages/use-codex-delegation-setup"; @@ -72,9 +72,13 @@ afterEach(async () => { async function mount(value: CodexDelegationStatus | null, busy = false) { const { createRoot } = await import("react-dom/client"); + function Harness() { + const [mode, setMode] = useState(value?.installedMode ?? "balanced"); + return ; + } await act(async () => { root = createRoot(container); - root.render(); + root.render(); }); } From 4416feb533d6dc04550ce1a59ed677610b7e8c0a Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 14:50:50 -0400 Subject: [PATCH 09/25] fix: refine delegation setup dialogs --- .../subagents-workspace/CodexDelegationSetupCard.tsx | 6 +++--- gui/src/i18n/de.ts | 1 + gui/src/i18n/en.ts | 1 + gui/src/i18n/ja.ts | 1 + gui/src/i18n/ko.ts | 1 + gui/src/i18n/ru.ts | 1 + gui/src/i18n/zh.ts | 1 + 7 files changed, 9 insertions(+), 3 deletions(-) diff --git a/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx index 01d3d0cd18..050f20994a 100644 --- a/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx +++ b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx @@ -50,7 +50,7 @@ export default function CodexDelegationSetupCard({ delegationSetup }: { delegati return () => window.removeEventListener("keydown", onKeyDown); }); - useEffect(() => { if (previewOpen) setTimeout(() => (previewApply ? previewConfirmRef.current : document.querySelector(".swi-delegation-dialog button"))?.focus(), 0); }, [previewApply, previewOpen]); + useEffect(() => { const timer = setTimeout(() => { if (typeof document === "undefined") return; if (previewOpen) (previewApply ? previewConfirmRef.current : document.querySelector(".swi-delegation-dialog button"))?.focus(); if (removeOpen) document.querySelector('[role="alertdialog"] button')?.focus(); }, 0); return () => clearTimeout(timer); }, [previewApply, previewOpen, removeOpen]); const trap = (event: React.KeyboardEvent) => { if (event.key !== "Tab") return; const buttons = Array.from(event.currentTarget.querySelectorAll("button:not([disabled])")); if (!buttons.length) return; const first = buttons[0]!; const last = buttons.at(-1)!; if (event.shiftKey && document.activeElement === first) { event.preventDefault(); last.focus(); } else if (!event.shiftKey && document.activeElement === last) { event.preventDefault(); first.focus(); } }; const openPreview = (event: React.MouseEvent, apply: boolean) => { openerRef.current = event.currentTarget; setPreviewApply(apply); setPreviewOpen(true); }; const runInstall = async () => { if (await install()) { setSuccess(true); closePreview(); } }; @@ -90,8 +90,8 @@ export default function CodexDelegationSetupCard({ delegationSetup }: { delegati {success &&

{t("sub.delegationSetup.newTask")}

}
{t("sub.delegationSetup.manual")}

{t("sub.delegationSetup.manualHint")}

} - {previewOpen && status &&
event.stopPropagation()}>

{t("sub.delegationSetup.preview")}

{status.previews[selectedMode].skillText}
{status.previews[selectedMode].agentsBlockText}
{previewApply && }
} - {removeOpen &&
event.stopPropagation()}>

{t("sub.delegationSetup.removeTitle")}

{t("sub.delegationSetup.removeConfirm")}

} + {previewOpen && status &&
event.stopPropagation()}>

{t("sub.delegationSetup.preview")}

{status.previews[selectedMode].skillText}
{status.previews[selectedMode].agentsBlockText}
{previewApply && }
} + {removeOpen &&
event.stopPropagation()}>

{t("sub.delegationSetup.removeTitle")}

{t("sub.delegationSetup.removeConfirm")}

{error &&

{t("sub.delegationSetup.error")}

}
} ); } diff --git a/gui/src/i18n/de.ts b/gui/src/i18n/de.ts index 0da1bd2d83..c529b6c464 100644 --- a/gui/src/i18n/de.ts +++ b/gui/src/i18n/de.ts @@ -2237,4 +2237,5 @@ export const de: Record = { "sub.delegationSetup.retry": "Erneut versuchen", "sub.delegationSetup.close": "Schließen", "sub.delegationSetup.cancel": "Abbrechen", + "sub.delegationSetup.confirmChangeMode": "Modus ändern", }; diff --git a/gui/src/i18n/en.ts b/gui/src/i18n/en.ts index 14572c88be..ce870a6d22 100644 --- a/gui/src/i18n/en.ts +++ b/gui/src/i18n/en.ts @@ -2265,6 +2265,7 @@ export const en = { "sub.delegationSetup.retry": "Retry", "sub.delegationSetup.close": "Close", "sub.delegationSetup.cancel": "Cancel", + "sub.delegationSetup.confirmChangeMode": "Change mode", } as const; diff --git a/gui/src/i18n/ja.ts b/gui/src/i18n/ja.ts index 8cf6ecad6a..9356415b83 100644 --- a/gui/src/i18n/ja.ts +++ b/gui/src/i18n/ja.ts @@ -2257,4 +2257,5 @@ export const ja: Record = { "sub.delegationSetup.retry": "再試行", "sub.delegationSetup.close": "閉じる", "sub.delegationSetup.cancel": "キャンセル", + "sub.delegationSetup.confirmChangeMode": "モードを変更", }; diff --git a/gui/src/i18n/ko.ts b/gui/src/i18n/ko.ts index d39b4aa49e..4591b8c098 100644 --- a/gui/src/i18n/ko.ts +++ b/gui/src/i18n/ko.ts @@ -2257,5 +2257,6 @@ export const ko: Record = { "sub.delegationSetup.retry": "다시 시도", "sub.delegationSetup.close": "닫기", "sub.delegationSetup.cancel": "취소", + "sub.delegationSetup.confirmChangeMode": "모드 변경", }; diff --git a/gui/src/i18n/ru.ts b/gui/src/i18n/ru.ts index c441e6d7bd..ae044afab7 100644 --- a/gui/src/i18n/ru.ts +++ b/gui/src/i18n/ru.ts @@ -2259,4 +2259,5 @@ export const ru: Record = { "sub.delegationSetup.retry": "Повторить", "sub.delegationSetup.close": "Закрыть", "sub.delegationSetup.cancel": "Отмена", + "sub.delegationSetup.confirmChangeMode": "Сменить режим", }; diff --git a/gui/src/i18n/zh.ts b/gui/src/i18n/zh.ts index 94fd6c659d..b306e68c0a 100644 --- a/gui/src/i18n/zh.ts +++ b/gui/src/i18n/zh.ts @@ -2257,4 +2257,5 @@ export const zh: Record = { "sub.delegationSetup.retry": "重试", "sub.delegationSetup.close": "关闭", "sub.delegationSetup.cancel": "取消", + "sub.delegationSetup.confirmChangeMode": "更改模式", }; From dabd146916978e01d7788b3d2fedf25fd273a36f Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 14:56:07 -0400 Subject: [PATCH 10/25] fix: retry retained delegation status --- .../CodexDelegationSetupCard.tsx | 2 +- gui/tests/codex-delegation-setup.test.tsx | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx index 050f20994a..6f2f44046f 100644 --- a/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx +++ b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx @@ -79,7 +79,7 @@ export default function CodexDelegationSetupCard({ delegationSetup }: { delegati
  • {t("sub.delegationSetup.agentsArtifact")}{status.artifacts.agentsPolicy.displayPath}
    • {blocked &&

    {t(blockedReason(status))}

    } - {error &&

    {t("sub.delegationSetup.error")}

    } + {error &&

    {t("sub.delegationSetup.error")}

    }
    {!installed && } diff --git a/gui/tests/codex-delegation-setup.test.tsx b/gui/tests/codex-delegation-setup.test.tsx index 7a9aa60339..c3a442d87a 100644 --- a/gui/tests/codex-delegation-setup.test.tsx +++ b/gui/tests/codex-delegation-setup.test.tsx @@ -203,3 +203,20 @@ test("hook sends the exact selected PUT body and re-reads the dedicated resource expect(put?.init?.body).toBe(JSON.stringify({ mode: "orchestrator" })); expect(requests.filter(request => request.init?.method === undefined).length).toBe(2); }); + +test("retained truthful status exposes retry after a refresh error", async () => { + const value = status("current", "balanced"); + let reloads = 0; + await mount(value); + // Remount with the same truthful status and a controller error, then prove the + // visible retry reaches the supplied refresh boundary. + const { createRoot } = await import("react-dom/client"); + await act(async () => { + root?.unmount(); + root = createRoot(container); + root.render( { reloads++; } }} />); + }); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("request failed"); + await act(async () => { button("Retry").click(); }); + expect(reloads).toBe(1); +}); From 544ba3504592b2d9d9c60045a89cac66c834cbd5 Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 15:14:13 -0400 Subject: [PATCH 11/25] test: complete delegation setup coverage --- gui/tests/codex-delegation-setup.test.tsx | 436 +++++++++++++++------- gui/tests/subagents-classic.test.ts | 78 +++- 2 files changed, 365 insertions(+), 149 deletions(-) diff --git a/gui/tests/codex-delegation-setup.test.tsx b/gui/tests/codex-delegation-setup.test.tsx index c3a442d87a..a301cb732e 100644 --- a/gui/tests/codex-delegation-setup.test.tsx +++ b/gui/tests/codex-delegation-setup.test.tsx @@ -3,7 +3,12 @@ import { Window } from "happy-dom"; import { act, useState } from "react"; import type { Root } from "react-dom/client"; import CodexDelegationSetupCard from "../src/components/subagents-workspace/CodexDelegationSetupCard"; -import { useCodexDelegationSetup, type CodexDelegationSetupController, type CodexDelegationStatus } from "../src/pages/use-codex-delegation-setup"; +import { + useCodexDelegationSetup, + type CodexDelegationMode, + type CodexDelegationSetupController, + type CodexDelegationStatus, +} from "../src/pages/use-codex-delegation-setup"; import { LanguageProvider } from "../src/i18n/provider"; const globals = ["document", "window", "navigator", "localStorage", "sessionStorage", "fetch", "IS_REACT_ACT_ENVIRONMENT"] as const; @@ -11,46 +16,25 @@ let previousGlobals: Record<(typeof globals)[number], unknown>; let testWindow: Window; let container: HTMLElement; let root: Root | null = null; -let selectedMode: "balanced" | "orchestrator"; -let installed = 0; -let removed = 0; -function status(state: CodexDelegationStatus["state"] = "not-installed", mode: "balanced" | "orchestrator" | null = null): CodexDelegationStatus { +function makeStatus(state: CodexDelegationStatus["state"] = "not-installed", mode: CodexDelegationMode | null = null): CodexDelegationStatus { return { - schemaVersion: 1, - state, - installedMode: mode, + schemaVersion: 1, state, installedMode: mode, artifacts: { skill: { state: mode ? "current" : "absent", displayPath: "$HOME/.agents/skills/codexcommander-delegation/SKILL.md" }, agentsPolicy: { state: mode ? "current" : "absent", displayPath: "$CODEX_HOME/AGENTS.md" }, }, - override: { state: "absent" }, - activation: "effective", + override: { state: "absent" }, activation: "effective", previews: { - balanced: { skillText: "balanced skill", agentsBlockText: "balanced policy" }, - orchestrator: { skillText: "orchestrator skill", agentsBlockText: "orchestrator policy" }, + balanced: { skillText: "balanced skill from server", agentsBlockText: "balanced policy from server" }, + orchestrator: { skillText: "orchestrator skill from server", agentsBlockText: "orchestrator policy from server" }, }, - copyPrompts: { balanced: "balanced manual prompt", orchestrator: "orchestrator manual prompt" }, - }; -} - -function controller(value: CodexDelegationStatus | null, busy = false): CodexDelegationSetupController { - selectedMode = value?.installedMode ?? "balanced"; - return { - loaded: value !== null, - status: value, - selectedMode, - busy, - error: null, - setSelectedMode: mode => { selectedMode = mode; }, - install: async () => { installed++; return true; }, - uninstall: async () => { removed++; return true; }, - reload: async () => {}, + copyPrompts: { balanced: "balanced manual prompt from server", orchestrator: "orchestrator manual prompt from server" }, }; } beforeEach(() => { - previousGlobals = Object.fromEntries(globals.map(k => [k, Reflect.get(globalThis, k)])) as typeof previousGlobals; + previousGlobals = Object.fromEntries(globals.map(key => [key, Reflect.get(globalThis, key)])) as typeof previousGlobals; testWindow = new Window({ url: "http://localhost/" }); Object.defineProperty(testWindow.navigator, "language", { configurable: true, value: "en-US" }); Object.defineProperties(globalThis, { @@ -61,8 +45,6 @@ beforeEach(() => { (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; container = testWindow.document.createElement("div") as unknown as HTMLElement; testWindow.document.body.append(container as never); - installed = 0; - removed = 0; }); afterEach(async () => { @@ -70,153 +52,321 @@ afterEach(async () => { for (const key of globals) Object.defineProperty(globalThis, key, { configurable: true, value: previousGlobals[key] }); }); -async function mount(value: CodexDelegationStatus | null, busy = false) { +async function flush() { await new Promise(resolve => setTimeout(resolve, 0)); } + +async function render(node: React.ReactNode) { const { createRoot } = await import("react-dom/client"); - function Harness() { - const [mode, setMode] = useState(value?.installedMode ?? "balanced"); - return ; - } - await act(async () => { - root = createRoot(container); - root.render(); - }); + await act(async () => { root = createRoot(container); root.render({node}); await flush(); }); } -function button(label: string): HTMLButtonElement { - const found = Array.from(container.querySelectorAll("button")).find(item => item.textContent?.trim() === label); +function button(label: string, within: ParentNode = container): HTMLButtonElement { + const found = Array.from(within.querySelectorAll("button")).find(item => item.textContent?.trim() === label); if (!found) throw new Error(`Missing button: ${label}`); return found; } +function radio(mode: CodexDelegationMode): HTMLInputElement { + const found = container.querySelector(`input[value="${mode}"]`); + if (!found) throw new Error(`Missing radio: ${mode}`); + return found; +} + +function directController(value: CodexDelegationStatus | null, overrides: Partial = {}): CodexDelegationSetupController { + return { + loaded: value !== null, status: value, selectedMode: value?.installedMode ?? "balanced", busy: false, error: null, + setSelectedMode: () => {}, install: async () => true, uninstall: async () => true, reload: async () => {}, ...overrides, + }; +} + +async function mountDirect(value: CodexDelegationStatus | null, overrides: Partial = {}) { + function Harness() { + const [mode, setMode] = useState(overrides.selectedMode ?? value?.installedMode ?? "balanced"); + return ; + } + await render(); +} + +async function mountHook(apiBase = "/hook") { + function Harness() { return ; } + await render(); +} + +async function openApply(label: "Install" | "Update" | "Repair" | "Change mode") { + await act(async () => { button(label).click(); await flush(); }); + return container.querySelector('[role="dialog"]')!; +} + test("loading never makes a false not-installed claim", async () => { - await mount(null); - expect(container.textContent).toContain("Loading"); + await mountDirect(null); + expect(container.textContent).toContain("Loading delegation setup"); expect(container.textContent).not.toContain("Not installed"); }); -test("fresh setup uses Balanced and previews before the install mutation", async () => { - await mount(status()); - expect((container.querySelector('input[value="balanced"]') as HTMLInputElement).checked).toBe(true); - await act(async () => { button("Preview").click(); }); - expect(container.querySelector('[role="dialog"]')?.textContent).toContain("balanced skill"); - expect(installed).toBe(0); +test("fresh Install requires preview confirmation, sends exact Balanced PUT, and shows the new-task reminder", async () => { + const requests: Array<{ url: string; init?: RequestInit }> = []; + let current = makeStatus(); + globalThis.fetch = async (url, init) => { + requests.push({ url: String(url), init }); + if (init?.method === "PUT") { current = makeStatus("current", "balanced"); return Response.json({ ok: true, status: current }); } + return Response.json(current); + }; + await mountHook("/fresh"); + expect(radio("balanced").checked).toBe(true); + const dialog = await openApply("Install"); + expect(dialog.textContent).toContain("balanced skill from server"); + expect(requests.some(request => request.init?.method === "PUT")).toBe(false); + await act(async () => { button("Install", dialog).click(); await flush(); }); + const put = requests.find(request => request.init?.method === "PUT")!; + expect(put.url).toBe("/fresh/api/codex-delegation"); + expect(put.init?.headers).toEqual({ "Content-Type": "application/json" }); + expect(put.init?.body).toBe('{"mode":"balanced"}'); + expect(container.querySelector('[role="dialog"]')).toBeNull(); + expect(container.querySelector('[role="status"]')?.textContent).toContain("Start a new Codex task"); }); -test("orchestrator selection uses its server preview and install flow", async () => { - await mount(status()); - const radio = container.querySelector('input[value="orchestrator"]') as HTMLInputElement; - await act(async () => { radio.click(); }); - await act(async () => { button("Preview").click(); }); - expect(container.querySelector('[role="dialog"]')?.textContent).toContain("orchestrator skill"); +test("Orchestrator selection changes server preview and exact Install PUT", async () => { + const requests: RequestInit[] = []; + let current = makeStatus(); + globalThis.fetch = async (_url, init) => { + requests.push(init ?? {}); + if (init?.method === "PUT") { current = makeStatus("current", "orchestrator"); return Response.json({ ok: true, status: current }); } + return Response.json(current); + }; + await mountHook(); + await act(async () => { radio("orchestrator").click(); }); + const dialog = await openApply("Install"); + expect(dialog.textContent).toContain("orchestrator skill from server"); + expect(dialog.textContent).not.toContain("balanced skill from server"); + await act(async () => { button("Install", dialog).click(); await flush(); }); + expect(requests.find(init => init.method === "PUT")?.body).toBe('{"mode":"orchestrator"}'); }); -test("current effective setup exposes ready and installed controls", async () => { - await mount(status("current", "orchestrator")); +for (const [state, action] of [["update-available", "Update"], ["partial", "Repair"]] as const) { + test(`${action} confirms its preview and sends the installed mode PUT`, async () => { + const requests: RequestInit[] = []; + let current = makeStatus(state, "balanced"); + globalThis.fetch = async (_url, init) => { + requests.push(init ?? {}); + if (init?.method === "PUT") { current = makeStatus("current", "balanced"); return Response.json({ ok: true, status: current }); } + return Response.json(current); + }; + await mountHook(`/${action.toLowerCase()}`); + const dialog = await openApply(action); + expect(requests.some(init => init.method === "PUT")).toBe(false); + await act(async () => { button(action, dialog).click(); await flush(); }); + expect(requests.find(init => init.method === "PUT")?.body).toBe('{"mode":"balanced"}'); + }); +} + +test("installed Change mode confirms truthfully and sends the selected exact PUT", async () => { + const requests: RequestInit[] = []; + let current = makeStatus("current", "balanced"); + globalThis.fetch = async (_url, init) => { + requests.push(init ?? {}); + if (init?.method === "PUT") { current = makeStatus("current", "orchestrator"); return Response.json({ ok: true, status: current }); } + return Response.json(current); + }; + await mountHook("/change"); expect(container.textContent).toContain("Ready"); - expect(container.textContent).toContain("Orchestrator"); - expect(button("Change mode")).toBeTruthy(); expect(button("Remove")).toBeTruthy(); + await act(async () => { radio("orchestrator").click(); }); + const dialog = await openApply("Change mode"); + expect(dialog.textContent).toContain("orchestrator policy from server"); + expect(requests.some(init => init.method === "PUT")).toBe(false); + await act(async () => { button("Change mode", dialog).click(); await flush(); }); + expect(requests.find(init => init.method === "PUT")?.body).toBe('{"mode":"orchestrator"}'); }); -test("update and partial states use their respective primary actions", async () => { - await mount(status("update-available", "balanced")); - expect(button("Update")).toBeTruthy(); - await act(async () => { root?.unmount(); root = null; }); - await mount(status("partial", "balanced")); - expect(button("Repair")).toBeTruthy(); +for (const state of ["conflict", "unsafe"] as const) { + test(`${state} refuses automatic mutation and projects its distinct reason`, async () => { + const value = makeStatus(state); + value.artifacts.skill.reason = state === "conflict" ? "ownership_conflict" : "unsafe_path"; + let installs = 0; + await mountDirect(value, { install: async () => { installs++; return true; } }); + expect(button("Install").disabled).toBe(true); + expect(container.querySelector('[role="alert"]')?.textContent).toContain(state === "conflict" ? "existing file is not managed" : "could not be safely verified"); + button("Install").click(); + expect(installs).toBe(0); + }); +} + +test("shadowed current install is truthful and never claims Ready", async () => { + const value = makeStatus("current", "balanced"); value.activation = "shadowed"; value.override.state = "active"; + await mountDirect(value); + expect(container.textContent).toContain("Installed, but AGENTS.override.md is active"); + expect(container.textContent).not.toContain("Ready"); }); -test("conflict and unsafe state fail closed with the projected reason", async () => { - const conflict = status("conflict", null); - conflict.artifacts.skill.reason = "ownership_conflict"; - await mount(conflict); - expect(container.textContent).toContain("can’t be changed automatically"); - expect(button("Install").disabled).toBe(true); +test("manual details are collapsed and copy the selected server prompt only after clipboard success", async () => { + const writes: string[] = []; + Object.defineProperty(testWindow.navigator, "clipboard", { configurable: true, value: { writeText: async (text: string) => { writes.push(text); } } }); + await mountDirect(makeStatus()); + const details = container.querySelector("details") as HTMLDetailsElement; + expect(details.open).toBe(false); + await act(async () => { details.querySelector("summary")!.click(); radio("orchestrator").click(); }); + expect(button("Copy setup").textContent).toBe("Copy setup"); + await act(async () => { button("Copy setup").click(); await flush(); }); + expect(writes).toEqual(["orchestrator manual prompt from server"]); + expect(button("Copied")).toBeTruthy(); }); -test("shadowed install is not presented as ready", async () => { - const shadowed = status("current", "balanced"); - shadowed.activation = "shadowed"; - shadowed.override.state = "active"; - await mount(shadowed); - expect(container.textContent).toContain("Installed, but AGENTS.override.md is active"); - expect(container.textContent).not.toContain("Ready"); +test("manual copy failure gives honest unavailable feedback and never claims copied", async () => { + Object.defineProperty(testWindow.navigator, "clipboard", { configurable: true, value: { writeText: async () => { throw new Error("denied"); } } }); + Object.defineProperty(testWindow.document, "execCommand", { configurable: true, value: () => false }); + await mountDirect(makeStatus()); + const details = container.querySelector("details")!; + await act(async () => { details.querySelector("summary")!.click(); button("Copy setup").click(); await flush(); }); + expect(button("Copy unavailable")).toBeTruthy(); + expect(container.textContent).not.toContain("Copied"); }); -test("remove waits for an accessible confirmation before DELETE", async () => { - await mount(status("current", "balanced")); - await act(async () => { button("Remove").click(); }); - const dialog = container.querySelector('[role="alertdialog"]'); - expect(dialog).toBeTruthy(); - expect(removed).toBe(0); - await act(async () => { Array.from(dialog!.querySelectorAll("button")).find(item => item.textContent?.trim() === "Remove")!.click(); }); - expect(removed).toBe(1); +test("Remove sends no DELETE before confirm, retains the failed dialog error, then closes and reminds on success", async () => { + const requests: RequestInit[] = []; + let deletes = 0; let current = makeStatus("current", "balanced"); + globalThis.fetch = async (_url, init) => { + requests.push(init ?? {}); + if (init?.method === "DELETE") { + deletes++; + if (deletes === 1) return Response.json({ error: "locked" }, { status: 500 }); + current = makeStatus(); return Response.json({ ok: true, status: current }); + } + return Response.json(current); + }; + await mountHook("/remove"); + await act(async () => { button("Remove").click(); await flush(); }); + let dialog = container.querySelector('[role="alertdialog"]')!; + expect(requests.some(init => init.method === "DELETE")).toBe(false); + await act(async () => { button("Remove", dialog).click(); await flush(); }); + dialog = container.querySelector('[role="alertdialog"]')!; + expect(dialog.querySelector('[role="alert"]')?.textContent).toContain("request failed"); + expect(requests.find(init => init.method === "DELETE")?.body).toBeUndefined(); + await act(async () => { button("Remove", dialog).click(); await flush(); }); + expect(container.querySelector('[role="alertdialog"]')).toBeNull(); + expect(container.querySelector('[role="status"]')?.textContent).toContain("Start a new Codex task"); }); -test("manual setup stays collapsed and copies only selected server prompt", async () => { - await mount(status()); - const details = container.querySelector("details") as HTMLDetailsElement; - expect(details.open).toBe(false); - expect(container.textContent).toContain("Installer unavailable? Show manual setup"); +test("initial GET failure shows Retry and a successful retry restores truthful status", async () => { + let reads = 0; + globalThis.fetch = async () => ++reads === 1 ? Response.json({ error: "offline" }, { status: 503 }) : Response.json(makeStatus("current", "orchestrator")); + await mountHook("/retry-initial"); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("request failed"); + await act(async () => { button("Retry").click(); await flush(); }); + expect(reads).toBe(2); + expect(container.textContent).toContain("Ready"); + expect(radio("orchestrator").checked).toBe(true); }); -test("busy state disables every setup mutation and exposes a live status", async () => { - await mount(status("current", "balanced"), true); +test("retained-status refresh failure keeps truth, exposes Retry, and clears the error after recovery", async () => { + let reads = 0; const current = makeStatus("current", "balanced"); + globalThis.fetch = async (_url, init) => { + if (init?.method === "PUT") return Response.json({ ok: true, status: current }); + reads++; + if (reads === 2) return Response.json({ error: "refresh failed" }, { status: 503 }); + return Response.json(current); + }; + await mountHook("/retry-retained"); + const dialog = await openApply("Change mode"); + await act(async () => { button("Change mode", dialog).click(); await flush(); }); + expect(container.textContent).toContain("Ready"); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("request failed"); + await act(async () => { button("Retry").click(); await flush(); }); + expect(reads).toBe(3); + expect(container.querySelector('[role="alert"]')).toBeNull(); + expect(container.textContent).toContain("Ready"); +}); + +test("a superseded GET cannot replace the newer status", async () => { + let resolveFirst!: (response: Response) => void; let reads = 0; + globalThis.fetch = async () => { + reads++; + if (reads === 1) return new Promise(resolve => { resolveFirst = resolve; }); + return Response.json(makeStatus("current", "orchestrator")); + }; + function Harness() { + const setup = useCodexDelegationSetup("/race"); + return <>{setup.status?.installedMode ?? "none"}; + } + await render(); + await act(async () => { button("Force reload").click(); await flush(); }); + expect(container.querySelector("[data-state]")?.textContent).toBe("orchestrator"); + await act(async () => { resolveFirst(Response.json(makeStatus("current", "balanced"))); await flush(); }); + expect(container.querySelector("[data-state]")?.textContent).toBe("orchestrator"); +}); + +test("unmount aborts the outstanding GET and suppresses its late result", async () => { + let signal: AbortSignal | undefined; let resolveRead!: (response: Response) => void; + globalThis.fetch = async (_url, init) => { + signal = init?.signal ?? undefined; + return new Promise(resolve => { resolveRead = resolve; }); + }; + function Harness() { const setup = useCodexDelegationSetup("/unmount"); return {setup.loaded ? "loaded" : "pending"}; } + await render(); + await act(async () => { root?.unmount(); root = null; }); + expect(signal?.aborted).toBe(true); + await act(async () => { resolveRead(Response.json(makeStatus("current", "balanced"))); await flush(); }); + expect(container.textContent).toBe(""); +}); + +test("malformed status is rejected as a visible retriable error", async () => { + globalThis.fetch = async () => Response.json({ schemaVersion: 1, state: "current" }); + await mountHook("/malformed"); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("request failed"); + expect(button("Retry")).toBeTruthy(); + expect(container.textContent).not.toContain("Ready"); +}); + +test("in-flight mutation disables every automatic control and announces busy state", async () => { + let resolvePut!: (response: Response) => void; const current = makeStatus("current", "balanced"); + globalThis.fetch = async (_url, init) => init?.method === "PUT" + ? new Promise(resolve => { resolvePut = resolve; }) + : Response.json(current); + await mountHook("/busy"); + const dialog = await openApply("Change mode"); + await act(async () => { button("Change mode", dialog).click(); await flush(); }); + expect(container.querySelector("fieldset")?.hasAttribute("disabled")).toBe(true); expect(button("Preview").disabled).toBe(true); expect(button("Change mode").disabled).toBe(true); expect(button("Remove").disabled).toBe(true); expect(container.querySelector('[aria-live="polite"]')?.textContent).toContain("Working"); + await act(async () => { resolvePut(Response.json({ ok: true, status: current })); await flush(); }); }); -test("preview and remove dialogs restore focus to their triggers", async () => { - await mount(status("current", "balanced")); - const preview = button("Preview"); - preview.focus(); - await act(async () => { preview.click(); }); - await act(async () => { Array.from(container.querySelector('[role="dialog"]')!.querySelectorAll("button")).find(item => item.textContent?.trim() === "Close")!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); - expect(document.activeElement).toBe(preview); - const remove = button("Remove"); - remove.focus(); - await act(async () => { remove.click(); }); - await act(async () => { Array.from(container.querySelector('[role="alertdialog"]')!.querySelectorAll("button")).find(item => item.textContent?.trim() === "Cancel")!.click(); await new Promise(resolve => setTimeout(resolve, 0)); }); - expect(document.activeElement).toBe(remove); +test("preview and remove dialogs take the documented safe initial focus", async () => { + await mountDirect(makeStatus("current", "balanced")); + await openApply("Change mode"); + await act(async () => { await flush(); }); + expect(document.activeElement).toBe(button("Change mode", container.querySelector('[role="dialog"]')!)); + await act(async () => { button("Close").click(); await flush(); button("Remove").click(); await flush(); }); + await act(async () => { await flush(); }); + expect(document.activeElement).toBe(button("Cancel", container.querySelector('[role="alertdialog"]')!)); }); -test("hook sends the exact selected PUT body and re-reads the dedicated resource", async () => { - const requests: Array<{ url: string; init?: RequestInit }> = []; - Object.defineProperty(globalThis, "fetch", { - configurable: true, - value: async (url: string, init?: RequestInit) => { - requests.push({ url, init }); - return Response.json(init?.method === "PUT" ? { ok: true, status: status("current", "orchestrator") } : status()); - }, - }); - function Harness() { - const setup = useCodexDelegationSetup("/hook"); - return <>{setup.loaded ? "loaded" : "loading"}; - } - const { createRoot } = await import("react-dom/client"); - await act(async () => { root = createRoot(container); root.render(); await new Promise(resolve => setTimeout(resolve, 0)); }); - await act(async () => { button("Select orchestrator").click(); button("Hook install").click(); await new Promise(resolve => setTimeout(resolve, 0)); }); - const put = requests.find(request => request.init?.method === "PUT"); - expect(put?.url).toBe("/hook/api/codex-delegation"); - expect(put?.init?.body).toBe(JSON.stringify({ mode: "orchestrator" })); - expect(requests.filter(request => request.init?.method === undefined).length).toBe(2); -}); - -test("retained truthful status exposes retry after a refresh error", async () => { - const value = status("current", "balanced"); - let reloads = 0; - await mount(value); - // Remount with the same truthful status and a controller error, then prove the - // visible retry reaches the supplied refresh boundary. - const { createRoot } = await import("react-dom/client"); - await act(async () => { - root?.unmount(); - root = createRoot(container); - root.render( { reloads++; } }} />); - }); - expect(container.querySelector('[role="alert"]')?.textContent).toContain("request failed"); - await act(async () => { button("Retry").click(); }); - expect(reloads).toBe(1); +test("preview and remove dialogs trap forward and reverse Tab at their boundaries", async () => { + await mountDirect(makeStatus("current", "balanced")); + const preview = await openApply("Change mode"); + const close = button("Close", preview); const confirm = button("Change mode", preview); + confirm.focus(); confirm.dispatchEvent(new testWindow.KeyboardEvent("keydown", { key: "Tab", bubbles: true })); + expect(document.activeElement).toBe(close); + close.dispatchEvent(new testWindow.KeyboardEvent("keydown", { key: "Tab", shiftKey: true, bubbles: true })); + expect(document.activeElement).toBe(confirm); + await act(async () => { close.click(); await flush(); button("Remove").click(); await flush(); }); + const remove = container.querySelector('[role="alertdialog"]')!; + const cancel = button("Cancel", remove); const confirmRemove = button("Remove", remove); + confirmRemove.focus(); confirmRemove.dispatchEvent(new testWindow.KeyboardEvent("keydown", { key: "Tab", bubbles: true })); + expect(document.activeElement).toBe(cancel); +}); + +test("Escape and backdrop close each dialog and restore its actual opener", async () => { + await mountDirect(makeStatus("current", "balanced")); + const previewOpener = button("Preview"); previewOpener.focus(); + await act(async () => { previewOpener.click(); await flush(); }); + await act(async () => { window.dispatchEvent(new testWindow.KeyboardEvent("keydown", { key: "Escape" })); await flush(); }); + expect(container.querySelector('[role="dialog"]')).toBeNull(); + expect(document.activeElement).toBe(previewOpener); + const removeOpener = button("Remove"); removeOpener.focus(); + await act(async () => { removeOpener.click(); await flush(); }); + const backdrop = container.querySelector('[role="alertdialog"]')!.parentElement!; + await act(async () => { backdrop.dispatchEvent(new testWindow.MouseEvent("mousedown", { bubbles: true })); await flush(); }); + expect(container.querySelector('[role="alertdialog"]')).toBeNull(); + expect(document.activeElement).toBe(removeOpener); }); diff --git a/gui/tests/subagents-classic.test.ts b/gui/tests/subagents-classic.test.ts index 21a7591df5..1c8cf8d263 100644 --- a/gui/tests/subagents-classic.test.ts +++ b/gui/tests/subagents-classic.test.ts @@ -1,4 +1,5 @@ import { expect, test } from "bun:test"; +import ts from "typescript"; /** * Subagents ships one command-center layout (configured roster + library + policy). @@ -62,16 +63,81 @@ test("Subagents workspace assets and i18n keys are present", async () => { } }); -test("Subagents places Codex delegation setup after Run Policy with a complete locale family", async () => { +const delegationSetupKeys = [ + "sub.delegationSetup.loading", + "sub.delegationSetup.title", + "sub.delegationSetup.subtitle", + "sub.delegationSetup.statusReady", + "sub.delegationSetup.statusInstalled", + "sub.delegationSetup.statusShadowed", + "sub.delegationSetup.statusNotInstalled", + "sub.delegationSetup.statusUpdate", + "sub.delegationSetup.statusPartial", + "sub.delegationSetup.statusConflict", + "sub.delegationSetup.statusUnsafe", + "sub.delegationSetup.modeLegend", + "sub.delegationSetup.mode.balanced", + "sub.delegationSetup.mode.balancedDescription", + "sub.delegationSetup.mode.orchestrator", + "sub.delegationSetup.mode.orchestratorDescription", + "sub.delegationSetup.liveRoster", + "sub.delegationSetup.skillArtifact", + "sub.delegationSetup.agentsArtifact", + "sub.delegationSetup.preview", + "sub.delegationSetup.install", + "sub.delegationSetup.update", + "sub.delegationSetup.repair", + "sub.delegationSetup.changeMode", + "sub.delegationSetup.remove", + "sub.delegationSetup.removeTitle", + "sub.delegationSetup.removeConfirm", + "sub.delegationSetup.manual", + "sub.delegationSetup.manualHint", + "sub.delegationSetup.copy", + "sub.delegationSetup.copied", + "sub.delegationSetup.copyUnavailable", + "sub.delegationSetup.newTask", + "sub.delegationSetup.working", + "sub.delegationSetup.reasonConflict", + "sub.delegationSetup.reasonUnsafe", + "sub.delegationSetup.error", + "sub.delegationSetup.retry", + "sub.delegationSetup.close", + "sub.delegationSetup.cancel", + "sub.delegationSetup.confirmChangeMode", +] as const; + +test("Subagents places Codex delegation setup after Run Policy", async () => { const workspace = await Bun.file(new URL("../src/components/subagents-workspace/SubagentsWorkspace.tsx", import.meta.url)).text(); - const card = await Bun.file(new URL("../src/components/subagents-workspace/CodexDelegationSetupCard.tsx", import.meta.url)).text(); expect(workspace).toContain('import CodexDelegationSetupCard'); expect(workspace.indexOf("swi-policy")).toBeLessThan(workspace.indexOf("CodexDelegationSetupCard delegationSetup")); - expect(card).not.toContain(">Teach Codex"); +}); + +test("every locale has exact parity with the complete delegation setup key contract", async () => { for (const locale of ["en", "ko", "ja", "de", "ru", "zh"]) { const src = await Bun.file(new URL(`../src/i18n/${locale}.ts`, import.meta.url)).text(); - expect(src).toContain("sub.delegationSetup.title"); - expect(src).toContain("sub.delegationSetup.manual"); - expect(src).toContain("sub.delegationSetup.reasonUnsafe"); + const actual = Array.from(src.matchAll(/^\s*"(sub\.delegationSetup\.[^"]+)"\s*:/gm), match => match[1]).sort(); + expect(actual).toEqual([...delegationSetupKeys].sort()); } }); + +test("delegation card has no hardcoded visible JSX copy", async () => { + const src = await Bun.file(new URL("../src/components/subagents-workspace/CodexDelegationSetupCard.tsx", import.meta.url)).text(); + const sourceFile = ts.createSourceFile("CodexDelegationSetupCard.tsx", src, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX); + const violations: string[] = []; + const visibleAttributes = new Set(["alt", "aria-label", "aria-description", "placeholder", "title"]); + + const visit = (node: ts.Node) => { + if (ts.isJsxText(node) && node.getText(sourceFile).trim()) violations.push(node.getText(sourceFile).trim()); + if (ts.isJsxAttribute(node) && visibleAttributes.has(node.name.getText(sourceFile)) && node.initializer && ts.isStringLiteral(node.initializer)) { + violations.push(node.initializer.text); + } + if (ts.isJsxExpression(node) && node.expression && (ts.isStringLiteral(node.expression) || ts.isNoSubstitutionTemplateLiteral(node.expression))) { + const parentTag = ts.isJsxElement(node.parent) ? node.parent.openingElement.tagName.getText(sourceFile) : ""; + if (parentTag !== "code" && parentTag !== "pre") violations.push(node.expression.text); + } + ts.forEachChild(node, visit); + }; + visit(sourceFile); + expect(violations).toEqual([]); +}); From 0d2940b439bfeba34bfd6532affceda30936d1df Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 15:36:12 -0400 Subject: [PATCH 12/25] test: harden delegation visible-copy contract --- gui/tests/subagents-classic.test.ts | 304 ++++++++++++++++++++++++++-- 1 file changed, 287 insertions(+), 17 deletions(-) diff --git a/gui/tests/subagents-classic.test.ts b/gui/tests/subagents-classic.test.ts index 1c8cf8d263..16a7228e74 100644 --- a/gui/tests/subagents-classic.test.ts +++ b/gui/tests/subagents-classic.test.ts @@ -107,6 +107,292 @@ const delegationSetupKeys = [ "sub.delegationSetup.confirmChangeMode", ] as const; +function findHardcodedVisibleJsxCopy(src: string): string[] { + const sourceFile = ts.createSourceFile("fixture.tsx", src, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX); + const violations: string[] = []; + const nonVisibleAttributes = new Set([ + "aria-busy", "aria-checked", "aria-controls", "aria-current", "aria-describedby", "aria-disabled", + "aria-expanded", "aria-hidden", "aria-labelledby", "aria-live", "aria-modal", "aria-pressed", + "aria-selected", "checked", "className", "defaultChecked", "disabled", "href", "htmlFor", "id", "key", + "multiple", "name", "readOnly", "ref", "rel", "required", "role", "selected", "src", "style", "tabIndex", + "target", "type", + ]); + + const attributeName = (attribute: ts.JsxAttribute) => attribute.name.getText(sourceFile); + const isNonVisibleAttribute = (name: string) => ( + nonVisibleAttributes.has(name) || name.startsWith("data-") || /^on[A-Z]/.test(name) + ); + const unwrap = (expression: ts.Expression): ts.Expression => { + let current = expression; + while ( + ts.isParenthesizedExpression(current) + || ts.isAsExpression(current) + || ts.isTypeAssertionExpression(current) + || ts.isSatisfiesExpression(current) + || ts.isNonNullExpression(current) + ) { + current = current.expression; + } + return current; + }; + const isDelegationTranslationKey = (expression: ts.Expression): boolean => { + const current = unwrap(expression); + if (ts.isStringLiteral(current) || ts.isNoSubstitutionTemplateLiteral(current)) { + return current.text.startsWith("sub.delegationSetup."); + } + if (ts.isTemplateExpression(current)) return current.head.text.startsWith("sub.delegationSetup."); + if (ts.isConditionalExpression(current)) { + return isDelegationTranslationKey(current.whenTrue) && isDelegationTranslationKey(current.whenFalse); + } + // TKey-typed variables and helper results keep non-copy control logic out of + // the JSX while the real source still passes TypeScript's translation-key check. + return ts.isIdentifier(current) || ts.isCallExpression(current) || ts.isPropertyAccessExpression(current); + }; + const isDelegationTranslationCall = (expression: ts.CallExpression) => ( + ts.isIdentifier(expression.expression) + && expression.expression.text === "t" + && expression.arguments.length > 0 + && isDelegationTranslationKey(expression.arguments[0]!) + ); + const isDelegationModeTuple = (expression: ts.Expression) => { + const current = unwrap(expression); + return ts.isArrayLiteralExpression(current) + && current.elements.length === 2 + && ts.isStringLiteral(current.elements[0]!) + && current.elements[0].text === "balanced" + && ts.isStringLiteral(current.elements[1]!) + && current.elements[1].text === "orchestrator"; + }; + const recordLiteral = (node: ts.Node, approvedTechnical: boolean) => { + if (approvedTechnical) return; + const text = ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node) + ? node.text + : node.getText(sourceFile).trim(); + if (text) violations.push(text); + }; + const hasTechnicalCopyApproval = (opening: ts.JsxOpeningLikeElement) => { + const tag = opening.tagName.getText(sourceFile); + if (tag !== "code" && tag !== "pre") return false; + const marker = opening.attributes.properties.find( + property => ts.isJsxAttribute(property) && attributeName(property) === "data-i18n-technical", + ); + return !!marker && ts.isJsxAttribute(marker) && !!marker.initializer + && ts.isStringLiteral(marker.initializer) && marker.initializer.text === "true"; + }; + + let scanExpression: (expression: ts.Expression, approvedTechnical: boolean) => void; + let scanJsxElement: (element: ts.JsxElement, approvedTechnical: boolean) => void; + let scanJsxFragment: (fragment: ts.JsxFragment, approvedTechnical: boolean) => void; + + const scanFunctionBody = (body: ts.ConciseBody, approvedTechnical: boolean) => { + if (!ts.isBlock(body)) { + scanExpression(body, approvedTechnical); + return; + } + const visitReturns = (node: ts.Node) => { + if (node !== body && ts.isFunctionLike(node)) return; + if (ts.isReturnStatement(node) && node.expression) { + scanExpression(node.expression, approvedTechnical); + return; + } + ts.forEachChild(node, visitReturns); + }; + visitReturns(body); + }; + + const scanObjectLiteral = (object: ts.ObjectLiteralExpression, approvedTechnical: boolean) => { + for (const property of object.properties) { + if (ts.isPropertyAssignment(property)) scanExpression(property.initializer, approvedTechnical); + else if (ts.isSpreadAssignment(property)) scanExpression(property.expression, approvedTechnical); + else if (ts.isMethodDeclaration(property) && property.body) scanFunctionBody(property.body, approvedTechnical); + else if (ts.isGetAccessorDeclaration(property) && property.body) scanFunctionBody(property.body, approvedTechnical); + } + }; + + scanExpression = (expression, approvedTechnical) => { + const current = unwrap(expression); + if (ts.isStringLiteral(current) || ts.isNoSubstitutionTemplateLiteral(current)) { + recordLiteral(current, approvedTechnical); + return; + } + if (ts.isTemplateExpression(current) || ts.isTaggedTemplateExpression(current)) { + recordLiteral(current, approvedTechnical); + return; + } + if (ts.isJsxElement(current)) { + scanJsxElement(current, approvedTechnical); + return; + } + if (ts.isJsxSelfClosingElement(current)) { + scanJsxAttributes(current, false); + return; + } + if (ts.isJsxFragment(current)) { + scanJsxFragment(current, approvedTechnical); + return; + } + if (ts.isConditionalExpression(current)) { + scanExpression(current.whenTrue, approvedTechnical); + scanExpression(current.whenFalse, approvedTechnical); + return; + } + if (ts.isBinaryExpression(current)) { + const operator = current.operatorToken.kind; + if (operator === ts.SyntaxKind.AmpersandAmpersandToken || operator === ts.SyntaxKind.CommaToken) { + scanExpression(current.right, approvedTechnical); + } else if ( + operator === ts.SyntaxKind.BarBarToken + || operator === ts.SyntaxKind.QuestionQuestionToken + || operator === ts.SyntaxKind.PlusToken + || operator === ts.SyntaxKind.EqualsToken + || operator === ts.SyntaxKind.PlusEqualsToken + || operator === ts.SyntaxKind.BarBarEqualsToken + || operator === ts.SyntaxKind.AmpersandAmpersandEqualsToken + || operator === ts.SyntaxKind.QuestionQuestionEqualsToken + ) { + scanExpression(current.left, approvedTechnical); + scanExpression(current.right, approvedTechnical); + } + return; + } + if (ts.isArrayLiteralExpression(current)) { + for (const element of current.elements) { + if (ts.isSpreadElement(element)) scanExpression(element.expression, approvedTechnical); + else scanExpression(element, approvedTechnical); + } + return; + } + if (ts.isObjectLiteralExpression(current)) { + scanObjectLiteral(current, approvedTechnical); + return; + } + if (ts.isCallExpression(current)) { + if (isDelegationTranslationCall(current)) return; + const callee = unwrap(current.expression); + if (ts.isPropertyAccessExpression(callee) || ts.isElementAccessExpression(callee)) { + // These exact machine values select translated mode labels; they are + // not rendered copy. Other literal-bearing call receivers still fail. + if (!isDelegationModeTuple(callee.expression)) scanExpression(callee.expression, approvedTechnical); + } else if (!ts.isIdentifier(callee)) { + scanExpression(callee, approvedTechnical); + } + for (const argument of current.arguments) scanExpression(argument, approvedTechnical); + return; + } + if (ts.isNewExpression(current)) { + for (const argument of current.arguments ?? []) scanExpression(argument, approvedTechnical); + return; + } + if (ts.isArrowFunction(current) || ts.isFunctionExpression(current)) { + scanFunctionBody(current.body, approvedTechnical); + return; + } + if (ts.isPropertyAccessExpression(current) || ts.isElementAccessExpression(current)) { + scanExpression(current.expression, approvedTechnical); + return; + } + if (ts.isAwaitExpression(current) || ts.isYieldExpression(current)) { + if (current.expression) scanExpression(current.expression, approvedTechnical); + } + }; + + const scanJsxAttribute = (attribute: ts.JsxAttribute, approvedTechnical: boolean) => { + if (isNonVisibleAttribute(attributeName(attribute)) || !attribute.initializer) return; + if (ts.isStringLiteral(attribute.initializer)) recordLiteral(attribute.initializer, approvedTechnical); + else if (ts.isJsxExpression(attribute.initializer) && attribute.initializer.expression) { + scanExpression(attribute.initializer.expression, approvedTechnical); + } + }; + function scanJsxAttributes(opening: ts.JsxOpeningLikeElement, approvedTechnical: boolean) { + for (const property of opening.attributes.properties) { + if (ts.isJsxAttribute(property)) scanJsxAttribute(property, approvedTechnical); + else scanExpression(property.expression, approvedTechnical); + } + } + const scanJsxChild = (child: ts.JsxChild, approvedTechnical: boolean) => { + if (ts.isJsxText(child)) recordLiteral(child, approvedTechnical); + else if (ts.isJsxExpression(child) && child.expression) scanExpression(child.expression, approvedTechnical); + else if (ts.isJsxElement(child)) scanJsxElement(child, approvedTechnical); + else if (ts.isJsxSelfClosingElement(child)) scanJsxAttributes(child, false); + else if (ts.isJsxFragment(child)) scanJsxFragment(child, approvedTechnical); + }; + scanJsxElement = (element, approvedTechnical) => { + const childTechnicalApproval = approvedTechnical || hasTechnicalCopyApproval(element.openingElement); + scanJsxAttributes(element.openingElement, false); + for (const child of element.children) scanJsxChild(child, childTechnicalApproval); + }; + scanJsxFragment = (fragment, approvedTechnical) => { + for (const child of fragment.children) scanJsxChild(child, approvedTechnical); + }; + + const visitTopLevel = (node: ts.Node) => { + if (ts.isJsxElement(node)) scanJsxElement(node, false); + else if (ts.isJsxSelfClosingElement(node)) scanJsxAttributes(node, false); + else if (ts.isJsxFragment(node)) scanJsxFragment(node, false); + else ts.forEachChild(node, visitTopLevel); + }; + visitTopLevel(sourceFile); + return violations; +} + +test("visible-copy contract rejects direct and recursively wrapped JSX literal mutations", () => { + const fixtures = [ + ["direct JSX text", "const Card = () =>

    Direct visible copy

    ;"], + ["direct string expression", 'const Card = () =>

    {"Direct expression copy"}

    ;'], + ["parenthesized literal", 'const Card = () =>

    {("Parenthesized copy")}

    ;'], + ["asserted literal", 'const Card = () =>

    {("Asserted copy" as string)}

    ;'], + ["conditional literal", 'const Card = () =>

    {enabled ? "Conditional copy" : serverCopy}

    ;'], + ["array-wrapped literal", 'const Card = () =>

    {["Array copy"]}

    ;'], + ["call-wrapped literal", 'const Card = () =>

    {renderCopy("Call copy")}

    ;'], + ] as const; + + const actual = Object.fromEntries(fixtures.map(([name, src]) => [name, findHardcodedVisibleJsxCopy(src)])); + expect(actual).toEqual({ + "direct JSX text": ["Direct visible copy"], + "direct string expression": ["Direct expression copy"], + "parenthesized literal": ["Parenthesized copy"], + "asserted literal": ["Asserted copy"], + "conditional literal": ["Conditional copy"], + "array-wrapped literal": ["Array copy"], + "call-wrapped literal": ["Call copy"], + }); +}); + +test("visible-copy contract rejects unapproved code and pre literal mutations", () => { + const src = 'const Card = () => <>{"Code visible copy"}
    {`Pre visible copy`}
    ;'; + expect(findHardcodedVisibleJsxCopy(src)).toEqual(["Code visible copy", "Pre visible copy"]); +}); + +test("visible-copy contract rejects literals in every literal-bearing visible prop", () => { + const src = `const Card = () => <> + Alt visible copy + {!installed && } {installed && } - {installed && } + {removable && }
    {busy &&

    {t("sub.delegationSetup.working")}

    } {success &&

    {t("sub.delegationSetup.newTask")}

    } diff --git a/gui/tests/codex-delegation-setup.test.tsx b/gui/tests/codex-delegation-setup.test.tsx index a301cb732e..d63976498e 100644 --- a/gui/tests/codex-delegation-setup.test.tsx +++ b/gui/tests/codex-delegation-setup.test.tsx @@ -33,6 +33,18 @@ function makeStatus(state: CodexDelegationStatus["state"] = "not-installed", mod }; } +function makeArtifactStatus( + state: CodexDelegationStatus["state"], + skill: CodexDelegationStatus["artifacts"]["skill"]["state"], + agentsPolicy: CodexDelegationStatus["artifacts"]["agentsPolicy"]["state"], + mode: CodexDelegationMode | null, +): CodexDelegationStatus { + const value = makeStatus(state, mode); + value.artifacts.skill.state = skill; + value.artifacts.agentsPolicy.state = agentsPolicy; + return value; +} + beforeEach(() => { previousGlobals = Object.fromEntries(globals.map(key => [key, Reflect.get(globalThis, key)])) as typeof previousGlobals; testWindow = new Window({ url: "http://localhost/" }); @@ -59,8 +71,12 @@ async function render(node: React.ReactNode) { await act(async () => { root = createRoot(container); root.render({node}); await flush(); }); } +function findButton(label: string, within: ParentNode = container): HTMLButtonElement | null { + return Array.from(within.querySelectorAll("button")).find(item => item.textContent?.trim() === label) ?? null; +} + function button(label: string, within: ParentNode = container): HTMLButtonElement { - const found = Array.from(within.querySelectorAll("button")).find(item => item.textContent?.trim() === label); + const found = findButton(label, within); if (!found) throw new Error(`Missing button: ${label}`); return found; } @@ -245,6 +261,53 @@ test("Remove sends no DELETE before confirm, retains the failed dialog error, th expect(container.querySelector('[role="status"]')?.textContent).toContain("Start a new Codex task"); }); +for (const [name, initial] of [ + ["current", makeArtifactStatus("current", "current", "current", "balanced")], + ["update available", makeArtifactStatus("update-available", "outdated", "current", "balanced")], + ["partial managed skill", makeArtifactStatus("partial", "current", "absent", null)], + ["partial managed policy", makeArtifactStatus("partial", "absent", "outdated", "balanced")], + ["compatibility collision", makeArtifactStatus("conflict", "current", "outdated", "balanced")], +] as const) { + test(`${name} exposes confirmed Remove and sends a bodyless DELETE`, async () => { + const requests: Array<{ url: string; init?: RequestInit }> = []; + let current = initial; + globalThis.fetch = async (url, init) => { + requests.push({ url: String(url), init }); + if (init?.method === "DELETE") { + current = makeStatus(); + return Response.json({ ok: true, status: current }); + } + return Response.json(current); + }; + await mountHook(`/remove-${name.replaceAll(" ", "-")}`); + const remove = findButton("Remove"); + expect(remove).not.toBeNull(); + await act(async () => { remove!.click(); await flush(); }); + const dialog = container.querySelector('[role="alertdialog"]')!; + expect(dialog).toBeTruthy(); + expect(requests.some(request => request.init?.method === "DELETE")).toBe(false); + await act(async () => { button("Remove", dialog).click(); await flush(); }); + const deletion = requests.find(request => request.init?.method === "DELETE")!; + expect(deletion.url).toBe(`/remove-${name.replaceAll(" ", "-")}/api/codex-delegation`); + expect(deletion.init?.body).toBeUndefined(); + }); +} + +for (const [name, value] of [ + ["aggregate state without managed artifacts", makeArtifactStatus("partial", "absent", "absent", null)], + ["foreign skill", makeArtifactStatus("conflict", "foreign", "current", "balanced")], + ["ambiguous agents markers", makeArtifactStatus("conflict", "current", "foreign", "balanced")], + ["aggregate unsafe", makeArtifactStatus("unsafe", "current", "current", "balanced")], + ["unproven one-artifact conflict", makeArtifactStatus("conflict", "current", "absent", null)], +] as const) { + test(`${name} never exposes Remove`, async () => { + let uninstalls = 0; + await mountDirect(value, { uninstall: async () => { uninstalls++; return true; } }); + expect(Array.from(container.querySelectorAll("button")).some(item => item.textContent?.trim() === "Remove")).toBe(false); + expect(uninstalls).toBe(0); + }); +} + test("initial GET failure shows Retry and a successful retry restores truthful status", async () => { let reads = 0; globalThis.fetch = async () => ++reads === 1 ? Response.json({ error: "offline" }, { status: 503 }) : Response.json(makeStatus("current", "orchestrator")); From ff8879c5dcaed8d74a055fd5a8d08991f4b4003a Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 17:56:31 -0400 Subject: [PATCH 21/25] fix: remove partial delegation collisions --- .../subagents-workspace/CodexDelegationSetupCard.tsx | 3 +-- gui/tests/codex-delegation-setup.test.tsx | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx index b89af4076a..e1198eb496 100644 --- a/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx +++ b/gui/src/components/subagents-workspace/CodexDelegationSetupCard.tsx @@ -27,8 +27,7 @@ function isRemovable(status: CodexDelegationStatus): boolean { const artifactStates = [status.artifacts.skill.state, status.artifacts.agentsPolicy.state]; const safelyManaged = artifactStates.every(state => state === "absent" || isOwnedArtifact(state)); if (!safelyManaged || !artifactStates.some(isOwnedArtifact)) return false; - if (status.state === "conflict") return artifactStates.every(isOwnedArtifact); - return status.state === "current" || status.state === "update-available" || status.state === "partial"; + return status.state === "current" || status.state === "update-available" || status.state === "partial" || status.state === "conflict"; } export default function CodexDelegationSetupCard({ delegationSetup }: { delegationSetup: CodexDelegationSetupController }) { diff --git a/gui/tests/codex-delegation-setup.test.tsx b/gui/tests/codex-delegation-setup.test.tsx index d63976498e..c3375c6dab 100644 --- a/gui/tests/codex-delegation-setup.test.tsx +++ b/gui/tests/codex-delegation-setup.test.tsx @@ -267,6 +267,8 @@ for (const [name, initial] of [ ["partial managed skill", makeArtifactStatus("partial", "current", "absent", null)], ["partial managed policy", makeArtifactStatus("partial", "absent", "outdated", "balanced")], ["compatibility collision", makeArtifactStatus("conflict", "current", "outdated", "balanced")], + ["compatibility collision with managed skill", makeArtifactStatus("conflict", "current", "absent", null)], + ["compatibility collision with managed policy", makeArtifactStatus("conflict", "absent", "outdated", "balanced")], ] as const) { test(`${name} exposes confirmed Remove and sends a bodyless DELETE`, async () => { const requests: Array<{ url: string; init?: RequestInit }> = []; @@ -298,7 +300,6 @@ for (const [name, value] of [ ["foreign skill", makeArtifactStatus("conflict", "foreign", "current", "balanced")], ["ambiguous agents markers", makeArtifactStatus("conflict", "current", "foreign", "balanced")], ["aggregate unsafe", makeArtifactStatus("unsafe", "current", "current", "balanced")], - ["unproven one-artifact conflict", makeArtifactStatus("conflict", "current", "absent", null)], ] as const) { test(`${name} never exposes Remove`, async () => { let uninstalls = 0; From 1ef8b7cbf66a6f023a003fc952e04df557b80a83 Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 18:03:06 -0400 Subject: [PATCH 22/25] fix: harden delegation manual instructions --- src/codex/delegation-templates.ts | 52 ++++++++---- tests/codex-delegation-templates.test.ts | 104 +++++++++++++++++------ 2 files changed, 115 insertions(+), 41 deletions(-) diff --git a/src/codex/delegation-templates.ts b/src/codex/delegation-templates.ts index 51ca4ebae0..3f4c06a236 100644 --- a/src/codex/delegation-templates.ts +++ b/src/codex/delegation-templates.ts @@ -14,6 +14,10 @@ export interface CodexDelegationBundle { } const SKILL_URL = new URL("../skills/codexcommander-delegation/SKILL.md", import.meta.url); +const SKILL_PAYLOAD_BEGIN = "<<>>"; +const SKILL_PAYLOAD_END = "<<>>"; +const AGENTS_PAYLOAD_BEGIN = "<<>>"; +const AGENTS_PAYLOAD_END = "<<>>"; function canonicalSkillText(): string { return readFileSync(SKILL_URL, "utf8"); @@ -37,33 +41,51 @@ function agentsBlock(mode: CodexDelegationMode): string { ].join("\n"); } +function framePayload(payload: string, begin: string, end: string): string { + if (payload.includes(begin) || payload.includes(end)) { + throw new Error("canonical delegation payload collides with its manual-copy delimiter"); + } + return `${begin}\n${payload}\n${end}`; +} + export function renderCodexDelegationBundle(mode: CodexDelegationMode): CodexDelegationBundle { const skillText = canonicalSkillText(); const agentsBlockText = agentsBlock(mode); const copyPrompt = [ `Set up CodexCommander delegation in ${mode} mode.`, "", - "Use only these two write targets:", - "- Write the exact skill payload below to `$HOME/.agents/skills/codexcommander-delegation/SKILL.md`.", - "- Add or update only the exact marker-bounded block below in `$CODEX_HOME/AGENTS.md`.", + "Resolve and inspect before any write:", + "- Resolve the platform user home displayed as `$HOME` exactly once as a readable, safe physical directory, then derive the skill target `$HOME/.agents/skills/codexcommander-delegation/SKILL.md`; never blindly interpolate an empty home variable.", + "- Resolve the Codex home exactly once. Use a configured `CODEX_HOME` only when its trimmed value is non-empty and resolves to a readable, safe physical directory. When it is unset or empty, use `$HOME/.codex`. Refuse an unreadable, non-directory, or unsafe configured root instead of falling back, and never blindly interpolate an empty shell variable.", + "- From that validated Codex home derive the policy target displayed as `$CODEX_HOME/AGENTS.md`, compatibility collision path `skills/codexcommander-delegation`, read-only override path `AGENTS.override.md`, and protected configuration path `config.toml`; `$CODEX_HOME/AGENTS.md` is a symbolic display path, never a shell string to interpolate before resolution.", + "", + "Before writing, safely inspect the targets and override, preview both proposed artifact changes, and obtain my explicit approval. Make no write before approval; approval never substitutes for the safety requirements below.", "", - "Before writing, inspect both targets, preview both proposed changes and obtain my explicit approval. Do not make any changes until I approve.", + "Ownership and content rules:", + "- Replace the target skill only when its frontmatter has exactly one `name: codexcommander-delegation`, one `metadata:` mapping containing exactly `managed-by: codexcommander` and `managed-version: \"1\"`; otherwise treat it as foreign and refuse.", + "- Refuse when the compatibility collision path exists.", + "- In the policy target, accept only an absent marker pair or one exact full-line begin marker followed by one exact full-line end marker. Refuse duplicate, orphaned, reversed, malformed, substring, or otherwise ambiguous markers.", + "- For one existing exact pair, replace the inclusive managed region from the first byte of the begin-marker line through the last byte of the end-marker line with the supplied marker-inclusive block. For an absent pair, append the block with only the minimum line separator: none after an existing LF/CRLF, otherwise one detected EOL (CRLF if the file contains CRLF, LF otherwise). Normalize only the inserted block to that EOL and preserve every prior and unrelated byte.", "", - "Safety rules:", - "- Replace an existing target skill only when its frontmatter has the exact ownership identity `name: codexcommander-delegation`, `managed-by: codexcommander`, and `managed-version: \"1\"`; otherwise treat it as foreign ownership and refuse the write.", - "- Also inspect `$CODEX_HOME/skills/codexcommander-delegation` for a same-name skill-name collision. If one exists, refuse the write.", - "- In `$CODEX_HOME/AGENTS.md`, replace only bytes inside one exact full-line begin/end marker pair. Treat duplicate, orphaned, reversed, malformed, or substring markers as an ambiguous marker state and refuse the write. Preserve every unrelated byte in `$CODEX_HOME/AGENTS.md`.", - "- If either target has an unsafe path or filesystem state, refuse to write rather than overwrite conflicting content.", - "- Do not edit `$CODEX_HOME/AGENTS.override.md`, `$CODEX_HOME/config.toml`, or `subagentDeveloperInstructions`.", - "- Do not copy the current roster or hardcode model IDs, effort levels, tool namespaces, or slot counts.", + "Override activation rules:", + "- Inspect `AGENTS.override.md` read-only and never edit it. Absent or zero-byte means a fresh Codex task can load the global policy; any non-zero-byte file, including whitespace-only content, means the artifacts are structurally installed but the policy is shadowed; unreadable or unsafe means activation is unknown. Never guarantee activation.", "", - "After the approved writes, report the paths changed and tell me to start a new Codex task so the policy is guaranteed to load.", + "Fail-closed filesystem rules:", + "- Refuse any symlink, junction, or reparse substitution in either validated root, any parent component, or a leaf; a present leaf must be a regular single-link file, never a hardlink or nonregular file.", + "- Read with bounded, no-follow inspection and fatal UTF-8 decoding: at most 256 KiB for each skill and 1 MiB for each AGENTS file. Refuse unreadable, oversized, invalid-UTF-8, or changing inputs.", + "- Bind publication to the exact inspected parent and preimage: revalidate parent identity plus leaf identity and bytes immediately before publishing; create an exclusive regular single-link temporary file in that verified parent; write and sync the exact desired bytes; publish atomically relative to the verified parent; then verify the postimage identity and bytes.", + "- Refuse without writing if available filesystem primitives cannot establish no-follow parent/leaf checks, exact parent and preimage revalidation, exclusive temporary creation, parent-bound atomic publication, and postimage verification.", + "- Do not edit the override, `config.toml`, `subagentDeveloperInstructions`, or any path other than the two artifact targets. Do not copy a roster or persist model/provider IDs, effort values, tool namespaces, or slot counts.", + "", + "Payload framing rules: each exact payload starts after the LF following its BEGIN delimiter and ends before the single wrapper LF immediately preceding its END delimiter. The wrapper LF and all delimiter bytes are not part of either artifact. Preserve payload bytes between those boundaries exactly, including whether the payload itself has a terminal newline; never write a delimiter.", "", "Canonical skill payload:", - skillText, + framePayload(skillText, SKILL_PAYLOAD_BEGIN, SKILL_PAYLOAD_END), + "", + "Canonical marker-inclusive AGENTS.md block payload:", + framePayload(agentsBlockText, AGENTS_PAYLOAD_BEGIN, AGENTS_PAYLOAD_END), "", - "Canonical AGENTS.md block:", - agentsBlockText, + "After approved writes, report the paths changed and the override-derived activation state, and advise me to start a new Codex task. Say a fresh task can load the policy only for an absent or zero-byte override; report every non-zero-byte override as shadowed and an unreadable or unsafe override as activation unknown. Never claim guaranteed activation.", ].join("\n"); return { mode, skillText, agentsBlockText, copyPrompt }; diff --git a/tests/codex-delegation-templates.test.ts b/tests/codex-delegation-templates.test.ts index 19b40b15d2..33bae32db7 100644 --- a/tests/codex-delegation-templates.test.ts +++ b/tests/codex-delegation-templates.test.ts @@ -11,6 +11,23 @@ import { upsertDelegationAgentsBlock, } from "../src/codex/delegation-agents-block"; +const SKILL_PAYLOAD_BEGIN = "<<>>"; +const SKILL_PAYLOAD_END = "<<>>"; +const AGENTS_PAYLOAD_BEGIN = "<<>>"; +const AGENTS_PAYLOAD_END = "<<>>"; + +function extractPayload(prompt: string, begin: string, end: string): string { + const beginToken = `${begin}\n`; + const endToken = `\n${end}`; + const start = prompt.indexOf(beginToken); + const finish = prompt.indexOf(endToken, start + beginToken.length); + if (start === -1 || finish === -1 || prompt.indexOf(beginToken, start + beginToken.length) !== -1 + || prompt.indexOf(endToken, finish + endToken.length) !== -1) { + throw new Error(`expected exactly one ordered ${begin}/${end} payload pair`); + } + return prompt.slice(start + beginToken.length, finish); +} + describe("Codex delegation templates", () => { test("balanced is deterministic and carries no roster ids", () => { const first = renderCodexDelegationBundle("balanced"); @@ -35,43 +52,78 @@ describe("Codex delegation templates", () => { }); test.each(["balanced", "orchestrator"] as const)( - "%s manual setup prompt is self-contained and fail-closed", + "%s manual setup prompt has an exact fail-closed wrapper and byte-extractable payloads", (mode) => { const bundle = renderCodexDelegationBundle(mode); + const skillBegin = bundle.copyPrompt.indexOf(SKILL_PAYLOAD_BEGIN); + const skillEnd = bundle.copyPrompt.indexOf(SKILL_PAYLOAD_END); + const agentsBegin = bundle.copyPrompt.indexOf(AGENTS_PAYLOAD_BEGIN); + const agentsEnd = bundle.copyPrompt.indexOf(AGENTS_PAYLOAD_END); - expect(bundle.copyPrompt).toContain(`Set up CodexCommander delegation in ${mode} mode.`); - expect(bundle.copyPrompt).toContain("`$HOME/.agents/skills/codexcommander-delegation/SKILL.md`"); - expect(bundle.copyPrompt).toContain("`$CODEX_HOME/AGENTS.md`"); - expect(bundle.copyPrompt).toContain("`$CODEX_HOME/skills/codexcommander-delegation`"); - expect(bundle.copyPrompt).toContain("preview both proposed changes and obtain my explicit approval"); - expect(bundle.copyPrompt).toContain("Do not make any changes until I approve."); - expect(bundle.copyPrompt).toContain("Preserve every unrelated byte in `$CODEX_HOME/AGENTS.md`."); - expect(bundle.copyPrompt).toContain( - "Replace an existing target skill only when its frontmatter has the exact ownership identity `name: codexcommander-delegation`, `managed-by: codexcommander`, and `managed-version: \"1\"`; otherwise treat it as foreign ownership and refuse the write.", + expect([skillBegin, skillEnd, agentsBegin, agentsEnd].every((index) => index >= 0)).toBe(true); + expect(skillBegin).toBeLessThan(skillEnd); + expect(skillEnd).toBeLessThan(agentsBegin); + expect(agentsBegin).toBeLessThan(agentsEnd); + expect(bundle.copyPrompt.slice(0, skillBegin)).toBe( + `Set up CodexCommander delegation in ${mode} mode.\n\n` + + "Resolve and inspect before any write:\n" + + "- Resolve the platform user home displayed as `$HOME` exactly once as a readable, safe physical directory, then derive the skill target `$HOME/.agents/skills/codexcommander-delegation/SKILL.md`; never blindly interpolate an empty home variable.\n" + + "- Resolve the Codex home exactly once. Use a configured `CODEX_HOME` only when its trimmed value is non-empty and resolves to a readable, safe physical directory. When it is unset or empty, use `$HOME/.codex`. Refuse an unreadable, non-directory, or unsafe configured root instead of falling back, and never blindly interpolate an empty shell variable.\n" + + "- From that validated Codex home derive the policy target displayed as `$CODEX_HOME/AGENTS.md`, compatibility collision path `skills/codexcommander-delegation`, read-only override path `AGENTS.override.md`, and protected configuration path `config.toml`; `$CODEX_HOME/AGENTS.md` is a symbolic display path, never a shell string to interpolate before resolution.\n\n" + + "Before writing, safely inspect the targets and override, preview both proposed artifact changes, and obtain my explicit approval. Make no write before approval; approval never substitutes for the safety requirements below.\n\n" + + "Ownership and content rules:\n" + + "- Replace the target skill only when its frontmatter has exactly one `name: codexcommander-delegation`, one `metadata:` mapping containing exactly `managed-by: codexcommander` and `managed-version: \"1\"`; otherwise treat it as foreign and refuse.\n" + + "- Refuse when the compatibility collision path exists.\n" + + "- In the policy target, accept only an absent marker pair or one exact full-line begin marker followed by one exact full-line end marker. Refuse duplicate, orphaned, reversed, malformed, substring, or otherwise ambiguous markers.\n" + + "- For one existing exact pair, replace the inclusive managed region from the first byte of the begin-marker line through the last byte of the end-marker line with the supplied marker-inclusive block. For an absent pair, append the block with only the minimum line separator: none after an existing LF/CRLF, otherwise one detected EOL (CRLF if the file contains CRLF, LF otherwise). Normalize only the inserted block to that EOL and preserve every prior and unrelated byte.\n\n" + + "Override activation rules:\n" + + "- Inspect `AGENTS.override.md` read-only and never edit it. Absent or zero-byte means a fresh Codex task can load the global policy; any non-zero-byte file, including whitespace-only content, means the artifacts are structurally installed but the policy is shadowed; unreadable or unsafe means activation is unknown. Never guarantee activation.\n\n" + + "Fail-closed filesystem rules:\n" + + "- Refuse any symlink, junction, or reparse substitution in either validated root, any parent component, or a leaf; a present leaf must be a regular single-link file, never a hardlink or nonregular file.\n" + + "- Read with bounded, no-follow inspection and fatal UTF-8 decoding: at most 256 KiB for each skill and 1 MiB for each AGENTS file. Refuse unreadable, oversized, invalid-UTF-8, or changing inputs.\n" + + "- Bind publication to the exact inspected parent and preimage: revalidate parent identity plus leaf identity and bytes immediately before publishing; create an exclusive regular single-link temporary file in that verified parent; write and sync the exact desired bytes; publish atomically relative to the verified parent; then verify the postimage identity and bytes.\n" + + "- Refuse without writing if available filesystem primitives cannot establish no-follow parent/leaf checks, exact parent and preimage revalidation, exclusive temporary creation, parent-bound atomic publication, and postimage verification.\n" + + "- Do not edit the override, `config.toml`, `subagentDeveloperInstructions`, or any path other than the two artifact targets. Do not copy a roster or persist model/provider IDs, effort values, tool namespaces, or slot counts.\n\n" + + "Payload framing rules: each exact payload starts after the LF following its BEGIN delimiter and ends before the single wrapper LF immediately preceding its END delimiter. The wrapper LF and all delimiter bytes are not part of either artifact. Preserve payload bytes between those boundaries exactly, including whether the payload itself has a terminal newline; never write a delimiter.\n\n" + + "Canonical skill payload:\n", ); - expect(bundle.copyPrompt).toContain( - "Also inspect `$CODEX_HOME/skills/codexcommander-delegation` for a same-name skill-name collision. If one exists, refuse the write.", + expect(bundle.copyPrompt.slice(skillEnd + SKILL_PAYLOAD_END.length, agentsBegin)).toBe( + "\n\nCanonical marker-inclusive AGENTS.md block payload:\n", ); - expect(bundle.copyPrompt).toContain("exact full-line begin/end marker pair"); - expect(bundle.copyPrompt).toContain("ambiguous marker state and refuse the write"); - expect(bundle.copyPrompt).toContain("unsafe path or filesystem state"); - expect(bundle.copyPrompt).toContain("refuse to write rather than overwrite conflicting content"); - expect(bundle.copyPrompt).toContain( - "Do not edit `$CODEX_HOME/AGENTS.override.md`, `$CODEX_HOME/config.toml`, or `subagentDeveloperInstructions`.", - ); - expect(bundle.copyPrompt).toContain("tell me to start a new Codex task"); - expect(bundle.copyPrompt.split(bundle.skillText)).toHaveLength(2); - expect(bundle.copyPrompt.split(bundle.agentsBlockText)).toHaveLength(2); - expect(bundle.copyPrompt).toContain( - "Do not copy the current roster or hardcode model IDs, effort levels, tool namespaces, or slot counts.", + expect(bundle.copyPrompt.slice(agentsEnd + AGENTS_PAYLOAD_END.length)).toBe( + "\n\nAfter approved writes, report the paths changed and the override-derived activation state, and advise me to start a new Codex task. Say a fresh task can load the policy only for an absent or zero-byte override; report every non-zero-byte override as shadowed and an unreadable or unsafe override as activation unknown. Never claim guaranteed activation.", ); - for (const frozenData of ["gpt-5.6", "kimi/", "xai/", "grok-4.6", "functions.collaboration", "ccx_collaboration"]) { - expect(bundle.copyPrompt).not.toContain(frozenData); + expect(extractPayload(bundle.copyPrompt, SKILL_PAYLOAD_BEGIN, SKILL_PAYLOAD_END)).toBe(bundle.skillText); + expect(extractPayload(bundle.copyPrompt, AGENTS_PAYLOAD_BEGIN, AGENTS_PAYLOAD_END)).toBe(bundle.agentsBlockText); + for (const delimiter of [SKILL_PAYLOAD_BEGIN, SKILL_PAYLOAD_END, AGENTS_PAYLOAD_BEGIN, AGENTS_PAYLOAD_END]) { + expect(bundle.skillText).not.toContain(delimiter); + expect(bundle.agentsBlockText).not.toContain(delimiter); } + expect(bundle.copyPrompt).not.toMatch(/guaranteed to load/i); + expect(bundle.copyPrompt).not.toMatch(/(?:follow|allow) (?:a )?(?:symbolic )?link/i); + expect(bundle.copyPrompt).not.toMatch(/approval (?:is|makes|renders).{0,30}safe/i); + expect(bundle.copyPrompt).not.toMatch( + /\b(?:may|can|should|must)\s+(?:make\s+)?(?:a\s+)?write.{0,40}\bbefore approval\b/i, + ); + expect(bundle.copyPrompt).not.toMatch( + /\b(?:may|can|should|must)\s+(?:edit|modify|write|remove|replace).{0,40}\bAGENTS\.override\.md\b/i, + ); }, ); + test("canonical artifacts carry no concrete roster, model, effort, tool-namespace, or slot data", () => { + const bundle = renderCodexDelegationBundle("balanced"); + const artifacts = `${bundle.skillText}\n${bundle.agentsBlockText}`; + + expect(artifacts).not.toMatch( + /\b(?:gpt|claude|gemini|grok|deepseek|llama|mistral|qwen|kimi|xai|anthropic|openai)[-_/.:][a-z0-9]/i, + ); + expect(artifacts).not.toMatch(/\b(?:functions|tools|mcp|ccx_collaboration)[.:_]{1,2}[a-z][\w.-]*/i); + expect(artifacts).not.toMatch(/\b(?:low|medium|high|xhigh|max|ultra)\s+(?:reasoning\s+)?effort\b/i); + expect(artifacts).not.toMatch(/\b\d+\s+(?:concurrency\s+)?slots?\b/i); + }); + test("managed skill ownership is carried by SKILL.md itself", () => { const skill = renderCodexDelegationBundle("balanced").skillText; expect(isCodexCommanderManagedSkill(skill)).toBe(true); From d2d8323f4294b89cae89ce081a3cdbc8746acb98 Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 18:04:44 -0400 Subject: [PATCH 23/25] fix: harden delegation filesystem transactions --- src/codex/delegation-installer.ts | 26 +++++-- tests/codex-delegation-installer.test.ts | 99 ++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 6 deletions(-) diff --git a/src/codex/delegation-installer.ts b/src/codex/delegation-installer.ts index 82edb0488d..9d72973deb 100644 --- a/src/codex/delegation-installer.ts +++ b/src/codex/delegation-installer.ts @@ -74,6 +74,8 @@ export interface CodexDelegationInstallerDeps { userHome?: string; codexHome?: string; beforePublish?: (artifact: "skill" | "agents") => void; + /** @internal Deterministic seam for post-publication verification tests. */ + afterPublish?: (artifact: "skill" | "agents", operation: "write" | "remove") => void; } const SKILL_LIMIT = 256 * 1024; @@ -466,12 +468,16 @@ function safeWrite( artifact: "skill" | "agents", deps: CodexDelegationInstallerDeps, ): FileSnapshotPresent { + const limit = artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT; + const bytes = Buffer.from(text, "utf8"); + if (bytes.length > limit) { + throw new DelegationFsError("too_large", "delegation output exceeds its read bound"); + } const root = rootForPath(paths, path); ensureSafeParent(root, dirname(path)); const parentBefore = lstatSync(dirname(path), { bigint: true }); - const currentBefore = readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT); + const currentBefore = readSnapshot(root, path, limit); if (!snapshotsEqual(expected, currentBefore)) throw new DelegationFsError("changed_during_mutation", "delegation preimage changed before preparation"); - const bytes = Buffer.from(text, "utf8"); const mode = expected.kind === "file" ? Number(expected.stat.mode & 0o777n) : 0o600; const tempPath = join(dirname(path), `.${basename(path)}.ccx.${process.pid}.${++tempSequence}.tmp`); let descriptor: number | null = null; @@ -500,7 +506,7 @@ function safeWrite( assertSafeExistingDirectories(root, dirname(path)); const parentNow = lstatSync(dirname(path), { bigint: true }); if (!sameDirectoryIdentity(parentPrepared, parentNow)) throw new DelegationFsError("changed_during_mutation", "delegation parent changed before publication"); - const current = readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT); + const current = readSnapshot(root, path, limit); if (!snapshotsEqual(expected, current)) throw new DelegationFsError("changed_during_mutation", "delegation preimage changed before publication"); const temp = readSnapshot(root, tempPath, bytes.length); if (temp.kind !== "file" || temp.stat.dev !== tempIdentity.dev || temp.stat.ino !== tempIdentity.ino || !temp.bytes.equals(bytes)) { @@ -508,7 +514,8 @@ function safeWrite( } renameAtomicFile(tempPath, path); published = true; - const after = readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT); + deps.afterPublish?.(artifact, "write"); + const after = readSnapshot(root, path, limit); if (after.kind !== "file" || after.stat.dev !== tempIdentity.dev || after.stat.ino !== tempIdentity.ino || !after.bytes.equals(bytes)) { throw new DelegationFsError("partial_write", "delegation postimage verification failed", true); } @@ -518,8 +525,11 @@ function safeWrite( } catch { /* not all platforms permit directory fsync */ } return after; } catch (error) { + if (published) { + throw new DelegationFsError("partial_write", "delegation post-publication verification failed", true, { cause: error }); + } if (error instanceof DelegationFsError) throw error; - throw new DelegationFsError(published ? "partial_write" : "write_failed", "delegation write failed", published, { cause: error }); + throw new DelegationFsError("write_failed", "delegation write failed", false, { cause: error }); } finally { if (descriptor !== null) try { closeSync(descriptor); } catch { /* primary error wins */ } safeTempCleanup(tempPath, tempIdentity); @@ -547,13 +557,17 @@ function safeRemove( if (!snapshotsEqual(expected, current)) throw new DelegationFsError("changed_during_mutation", "delegation preimage changed before removal"); unlinkSync(path); published = true; + deps.afterPublish?.(artifact, "remove"); if (readSnapshot(root, path, artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT).kind !== "absent") { throw new DelegationFsError("partial_write", "delegation removal verification failed", true); } return ABSENT; } catch (error) { + if (published) { + throw new DelegationFsError("partial_write", "delegation post-removal verification failed", true, { cause: error }); + } if (error instanceof DelegationFsError) throw error; - throw new DelegationFsError(published ? "partial_write" : "write_failed", "delegation removal failed", published, { cause: error }); + throw new DelegationFsError("write_failed", "delegation removal failed", false, { cause: error }); } } diff --git a/tests/codex-delegation-installer.test.ts b/tests/codex-delegation-installer.test.ts index cadec36c54..c317b5ea29 100644 --- a/tests/codex-delegation-installer.test.ts +++ b/tests/codex-delegation-installer.test.ts @@ -335,6 +335,27 @@ describe("Codex delegation installer", () => { expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, large.deps)).toMatchObject({ reason: "too_large" }); }); + test("AGENTS output crossing the read bound refuses before publication and compensates the skill", () => { + const fx = fixture(); + const agentsBefore = Buffer.alloc(1024 * 1024 - 1, 0x78); + write(fx.agentsPath, agentsBefore); + + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + + expect(outcome).toMatchObject({ + ok: false, + changed: false, + reason: "too_large", + status: { + state: "not-installed", + artifacts: { skill: { state: "absent" }, agentsPolicy: { state: "absent" } }, + }, + }); + expect(readFileSync(fx.agentsPath)).toEqual(agentsBefore); + expect(existsSync(fx.skillPath)).toBe(false); + expect(readdirSync(fx.codexHome)).toEqual(["AGENTS.md"]); + }); + test("changed preimage before publish refuses", () => { const fx = fixture(); write(fx.agentsPath, "user preface\n"); @@ -404,6 +425,84 @@ describe("Codex delegation installer", () => { expect(readFileSync(fx.skillPath, "utf8")).toBe("concurrent replacement"); }); + test("a DelegationFsError after rename reports a published partial write", () => { + const fx = fixture(); + const deps = { + ...fx.deps, + afterPublish: (artifact: "skill" | "agents", operation: "write" | "remove") => { + if (artifact === "skill" && operation === "write") { + writeFileSync(fx.skillPath, Buffer.alloc(256 * 1024 + 1, 0x78)); + } + }, + }; + + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, deps); + + expect(outcome).toMatchObject({ ok: false, changed: true, reason: "partial_write" }); + expect(readFileSync(fx.skillPath).byteLength).toBe(256 * 1024 + 1); + expect(existsSync(fx.agentsPath)).toBe(false); + }); + + test("an arbitrary error after rename reports a published partial write", () => { + const fx = fixture(); + const deps = { + ...fx.deps, + afterPublish: (artifact: "skill" | "agents", operation: "write" | "remove") => { + if (artifact === "skill" && operation === "write") throw new Error("post-rename verification failure"); + }, + }; + + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, deps); + + expect(outcome).toMatchObject({ + ok: false, + changed: true, + reason: "partial_write", + status: { state: "partial", artifacts: { skill: { state: "current" }, agentsPolicy: { state: "absent" } } }, + }); + expect(readFileSync(fx.skillPath, "utf8")).toBe(renderCodexDelegationBundle("balanced").skillText); + expect(existsSync(fx.agentsPath)).toBe(false); + }); + + test("a DelegationFsError after unlink reports a published partial write", () => { + const fx = fixture(); + mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + const deps = { + ...fx.deps, + afterPublish: (artifact: "skill" | "agents", operation: "write" | "remove") => { + if (artifact === "skill" && operation === "remove") mkdirSync(fx.skillPath); + }, + }; + + const outcome = mutateCodexDelegation({ action: "uninstall" }, deps); + + expect(outcome).toMatchObject({ ok: false, changed: true, reason: "partial_write" }); + expect(lstatSync(fx.skillPath).isDirectory()).toBe(true); + expect(readFileSync(fx.agentsPath, "utf8")).toBe(""); + }); + + test("an arbitrary error after unlink reports a published partial write", () => { + const fx = fixture(); + mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + const deps = { + ...fx.deps, + afterPublish: (artifact: "skill" | "agents", operation: "write" | "remove") => { + if (artifact === "skill" && operation === "remove") throw new Error("post-unlink verification failure"); + }, + }; + + const outcome = mutateCodexDelegation({ action: "uninstall" }, deps); + + expect(outcome).toMatchObject({ + ok: false, + changed: true, + reason: "partial_write", + status: { state: "not-installed", artifacts: { skill: { state: "absent" }, agentsPolicy: { state: "absent" } } }, + }); + expect(existsSync(fx.skillPath)).toBe(false); + expect(readFileSync(fx.agentsPath, "utf8")).toBe(""); + }); + test("active AGENTS.override.md reports shadowed without changing override bytes", () => { const fx = fixture(); const override = Buffer.from("# local override\r\nDo not delegate.\r\n"); From c43a5a33fc32748d57b1e9a5aef2de05b2238395 Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 18:16:28 -0400 Subject: [PATCH 24/25] fix: complete delegation fallback safety --- src/codex/delegation-templates.ts | 27 ++++++++--- tests/codex-delegation-templates.test.ts | 60 ++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 9 deletions(-) diff --git a/src/codex/delegation-templates.ts b/src/codex/delegation-templates.ts index 3f4c06a236..638e3d457c 100644 --- a/src/codex/delegation-templates.ts +++ b/src/codex/delegation-templates.ts @@ -18,6 +18,12 @@ const SKILL_PAYLOAD_BEGIN = "<< payload.includes(delimiter))) { + throw new Error("canonical delegation payload collides with a manual-copy delimiter"); + } } +} + +function framePayload(payload: string, begin: string, end: string): string { return `${begin}\n${payload}\n${end}`; } export function renderCodexDelegationBundle(mode: CodexDelegationMode): CodexDelegationBundle { const skillText = canonicalSkillText(); const agentsBlockText = agentsBlock(mode); + assertManualPayloadFrameSafety(skillText, agentsBlockText); const copyPrompt = [ `Set up CodexCommander delegation in ${mode} mode.`, "", "Resolve and inspect before any write:", "- Resolve the platform user home displayed as `$HOME` exactly once as a readable, safe physical directory, then derive the skill target `$HOME/.agents/skills/codexcommander-delegation/SKILL.md`; never blindly interpolate an empty home variable.", - "- Resolve the Codex home exactly once. Use a configured `CODEX_HOME` only when its trimmed value is non-empty and resolves to a readable, safe physical directory. When it is unset or empty, use `$HOME/.codex`. Refuse an unreadable, non-directory, or unsafe configured root instead of falling back, and never blindly interpolate an empty shell variable.", + "- Resolve the Codex home exactly once. A configured `CODEX_HOME` whose trimmed value is non-empty is authoritative and must resolve to a readable, safe physical directory; refuse an unreadable, non-directory, or unsafe explicit root instead of falling back. When `CODEX_HOME` is unset or empty, use the same effective platform default discovery as CodexCommander: start with `$HOME/.codex`; if that Linux/default home contains `config.toml`, select it. On supported WSL when it does not, honor the `[automount] root` from `/etc/wsl.conf` (default `/mnt`), discover readable physical Windows-profile `.codex` homes containing `config.toml`, prefer the candidate matching `USERPROFILE`, otherwise select a sole candidate only, and fall back to the Linux default when discovery is absent or ambiguous. Validate the selected root and never blindly interpolate an empty shell variable.", "- From that validated Codex home derive the policy target displayed as `$CODEX_HOME/AGENTS.md`, compatibility collision path `skills/codexcommander-delegation`, read-only override path `AGENTS.override.md`, and protected configuration path `config.toml`; `$CODEX_HOME/AGENTS.md` is a symbolic display path, never a shell string to interpolate before resolution.", "", "Before writing, safely inspect the targets and override, preview both proposed artifact changes, and obtain my explicit approval. Make no write before approval; approval never substitutes for the safety requirements below.", @@ -72,10 +85,12 @@ export function renderCodexDelegationBundle(mode: CodexDelegationMode): CodexDel "", "Fail-closed filesystem rules:", "- Refuse any symlink, junction, or reparse substitution in either validated root, any parent component, or a leaf; a present leaf must be a regular single-link file, never a hardlink or nonregular file.", + "- If an exact target parent is missing beneath its validated physical root, inspect each existing component and create one missing descendant at a time with exclusive, non-recursive directory creation using mode `0700` on POSIX (or the platform-equivalent user-only mode). Immediately lstat, realpath, and identity-check every created or concurrently appeared component; require a physical directory at the expected path with no symlink, junction, or reparse substitution, record its identity, and refuse any failed or changing check.", "- Read with bounded, no-follow inspection and fatal UTF-8 decoding: at most 256 KiB for each skill and 1 MiB for each AGENTS file. Refuse unreadable, oversized, invalid-UTF-8, or changing inputs.", - "- Bind publication to the exact inspected parent and preimage: revalidate parent identity plus leaf identity and bytes immediately before publishing; create an exclusive regular single-link temporary file in that verified parent; write and sync the exact desired bytes; publish atomically relative to the verified parent; then verify the postimage identity and bytes.", + "- Bind publication to the exact inspected parent and preimage: revalidate parent identity plus leaf identity and bytes immediately before publishing. For each artifact that changes, create one exclusive same-parent temporary file per artifact as the sole transient-path exception; create it as a regular single-link file at mode `0600`, bind its recorded device/file identity, preserve an existing target's mode when supported, write and sync the exact desired bytes, revalidate it, publish atomically relative to the verified parent, then verify the postimage identity and bytes.", + "- On every success or failure, clean up only that exact temporary path when its recorded identity still matches a regular single-link file; never remove a changed, replaced, or unknown path.", "- Refuse without writing if available filesystem primitives cannot establish no-follow parent/leaf checks, exact parent and preimage revalidation, exclusive temporary creation, parent-bound atomic publication, and postimage verification.", - "- Do not edit the override, `config.toml`, `subagentDeveloperInstructions`, or any path other than the two artifact targets. Do not copy a roster or persist model/provider IDs, effort values, tool namespaces, or slot counts.", + "- Only the two target artifacts may persistently change. The minimal missing physical parent directories needed to create those exact artifacts are the only authorized scaffolding; do not create, edit, remove, or leave any unrelated persistent file or directory. Do not edit the override, `config.toml`, or `subagentDeveloperInstructions`. Do not copy a roster or persist model/provider IDs, effort values, tool namespaces, or slot counts.", "", "Payload framing rules: each exact payload starts after the LF following its BEGIN delimiter and ends before the single wrapper LF immediately preceding its END delimiter. The wrapper LF and all delimiter bytes are not part of either artifact. Preserve payload bytes between those boundaries exactly, including whether the payload itself has a terminal newline; never write a delimiter.", "", diff --git a/tests/codex-delegation-templates.test.ts b/tests/codex-delegation-templates.test.ts index 33bae32db7..96fe16d083 100644 --- a/tests/codex-delegation-templates.test.ts +++ b/tests/codex-delegation-templates.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import * as delegationTemplates from "../src/codex/delegation-templates"; import { DELEGATION_BEGIN_MARKER, DELEGATION_END_MARKER, @@ -15,6 +16,12 @@ const SKILL_PAYLOAD_BEGIN = "<< { expect(bundle.agentsBlockText).toContain("synthesis"); }); + test.each(["balanced", "orchestrator"] as const)( + "%s manual setup follows platform default discovery instead of equating an unset CODEX_HOME with the Unix default", + (mode) => { + const prompt = renderCodexDelegationBundle(mode).copyPrompt; + + expect(prompt).toContain("same effective platform default discovery as CodexCommander"); + expect(prompt).toContain("supported WSL"); + expect(prompt).toContain("Windows-profile `.codex` home"); + expect(prompt).toContain("`USERPROFILE`"); + expect(prompt).not.toContain("When it is unset or empty, use `$HOME/.codex`."); + }, + ); + + test.each(["balanced", "orchestrator"] as const)( + "%s manual setup authorizes a safe fresh parent and exactly one identity-bound temp without unrelated persistent writes", + (mode) => { + const prompt = renderCodexDelegationBundle(mode).copyPrompt; + + expect(prompt).toContain("one missing descendant at a time"); + expect(prompt).toContain("exclusive, non-recursive directory creation"); + expect(prompt).toContain("Immediately lstat, realpath, and identity-check"); + expect(prompt).toContain("mode `0700` on POSIX"); + expect(prompt).toContain("one exclusive same-parent temporary file per artifact"); + expect(prompt).toContain("sole transient-path exception"); + expect(prompt).toContain("clean up only that exact temporary path when its recorded identity still matches"); + expect(prompt).toContain("Only the two target artifacts may persistently change"); + expect(prompt).not.toContain("any path other than the two artifact targets"); + }, + ); + + test.each( + MANUAL_PAYLOAD_DELIMITERS.flatMap((delimiter) => [ + [`skill payload containing ${delimiter}`, `${delimiter}\ncanonical skill`, "canonical policy"], + [`AGENTS payload containing ${delimiter}`, "canonical skill", `${delimiter}\ncanonical policy`], + ] as const), + )("rejects a cross-frame delimiter collision in the %s", (_case, skillText, agentsBlockText) => { + const seam = ( + delegationTemplates as unknown as { + assertManualPayloadFrameSafety?: (skill: string, agents: string) => void; + } + ).assertManualPayloadFrameSafety; + + expect(() => seam?.(skillText, agentsBlockText)).toThrow(/collides with a manual-copy delimiter/); + }); + test.each(["balanced", "orchestrator"] as const)( "%s manual setup prompt has an exact fail-closed wrapper and byte-extractable payloads", (mode) => { @@ -68,7 +120,7 @@ describe("Codex delegation templates", () => { `Set up CodexCommander delegation in ${mode} mode.\n\n` + "Resolve and inspect before any write:\n" + "- Resolve the platform user home displayed as `$HOME` exactly once as a readable, safe physical directory, then derive the skill target `$HOME/.agents/skills/codexcommander-delegation/SKILL.md`; never blindly interpolate an empty home variable.\n" - + "- Resolve the Codex home exactly once. Use a configured `CODEX_HOME` only when its trimmed value is non-empty and resolves to a readable, safe physical directory. When it is unset or empty, use `$HOME/.codex`. Refuse an unreadable, non-directory, or unsafe configured root instead of falling back, and never blindly interpolate an empty shell variable.\n" + + "- Resolve the Codex home exactly once. A configured `CODEX_HOME` whose trimmed value is non-empty is authoritative and must resolve to a readable, safe physical directory; refuse an unreadable, non-directory, or unsafe explicit root instead of falling back. When `CODEX_HOME` is unset or empty, use the same effective platform default discovery as CodexCommander: start with `$HOME/.codex`; if that Linux/default home contains `config.toml`, select it. On supported WSL when it does not, honor the `[automount] root` from `/etc/wsl.conf` (default `/mnt`), discover readable physical Windows-profile `.codex` homes containing `config.toml`, prefer the candidate matching `USERPROFILE`, otherwise select a sole candidate only, and fall back to the Linux default when discovery is absent or ambiguous. Validate the selected root and never blindly interpolate an empty shell variable.\n" + "- From that validated Codex home derive the policy target displayed as `$CODEX_HOME/AGENTS.md`, compatibility collision path `skills/codexcommander-delegation`, read-only override path `AGENTS.override.md`, and protected configuration path `config.toml`; `$CODEX_HOME/AGENTS.md` is a symbolic display path, never a shell string to interpolate before resolution.\n\n" + "Before writing, safely inspect the targets and override, preview both proposed artifact changes, and obtain my explicit approval. Make no write before approval; approval never substitutes for the safety requirements below.\n\n" + "Ownership and content rules:\n" @@ -80,10 +132,12 @@ describe("Codex delegation templates", () => { + "- Inspect `AGENTS.override.md` read-only and never edit it. Absent or zero-byte means a fresh Codex task can load the global policy; any non-zero-byte file, including whitespace-only content, means the artifacts are structurally installed but the policy is shadowed; unreadable or unsafe means activation is unknown. Never guarantee activation.\n\n" + "Fail-closed filesystem rules:\n" + "- Refuse any symlink, junction, or reparse substitution in either validated root, any parent component, or a leaf; a present leaf must be a regular single-link file, never a hardlink or nonregular file.\n" + + "- If an exact target parent is missing beneath its validated physical root, inspect each existing component and create one missing descendant at a time with exclusive, non-recursive directory creation using mode `0700` on POSIX (or the platform-equivalent user-only mode). Immediately lstat, realpath, and identity-check every created or concurrently appeared component; require a physical directory at the expected path with no symlink, junction, or reparse substitution, record its identity, and refuse any failed or changing check.\n" + "- Read with bounded, no-follow inspection and fatal UTF-8 decoding: at most 256 KiB for each skill and 1 MiB for each AGENTS file. Refuse unreadable, oversized, invalid-UTF-8, or changing inputs.\n" - + "- Bind publication to the exact inspected parent and preimage: revalidate parent identity plus leaf identity and bytes immediately before publishing; create an exclusive regular single-link temporary file in that verified parent; write and sync the exact desired bytes; publish atomically relative to the verified parent; then verify the postimage identity and bytes.\n" + + "- Bind publication to the exact inspected parent and preimage: revalidate parent identity plus leaf identity and bytes immediately before publishing. For each artifact that changes, create one exclusive same-parent temporary file per artifact as the sole transient-path exception; create it as a regular single-link file at mode `0600`, bind its recorded device/file identity, preserve an existing target's mode when supported, write and sync the exact desired bytes, revalidate it, publish atomically relative to the verified parent, then verify the postimage identity and bytes.\n" + + "- On every success or failure, clean up only that exact temporary path when its recorded identity still matches a regular single-link file; never remove a changed, replaced, or unknown path.\n" + "- Refuse without writing if available filesystem primitives cannot establish no-follow parent/leaf checks, exact parent and preimage revalidation, exclusive temporary creation, parent-bound atomic publication, and postimage verification.\n" - + "- Do not edit the override, `config.toml`, `subagentDeveloperInstructions`, or any path other than the two artifact targets. Do not copy a roster or persist model/provider IDs, effort values, tool namespaces, or slot counts.\n\n" + + "- Only the two target artifacts may persistently change. The minimal missing physical parent directories needed to create those exact artifacts are the only authorized scaffolding; do not create, edit, remove, or leave any unrelated persistent file or directory. Do not edit the override, `config.toml`, or `subagentDeveloperInstructions`. Do not copy a roster or persist model/provider IDs, effort values, tool namespaces, or slot counts.\n\n" + "Payload framing rules: each exact payload starts after the LF following its BEGIN delimiter and ends before the single wrapper LF immediately preceding its END delimiter. The wrapper LF and all delimiter bytes are not part of either artifact. Preserve payload bytes between those boundaries exactly, including whether the payload itself has a terminal newline; never write a delimiter.\n\n" + "Canonical skill payload:\n", ); From 3456caab8b949a6c4df9b95e2d99f874ac8f7534 Mon Sep 17 00:00:00 2001 From: pavelhov Date: Mon, 24 Aug 2026 18:20:29 -0400 Subject: [PATCH 25/25] fix: restore delegation preimages exactly --- src/codex/delegation-installer.ts | 28 +++++---- tests/codex-delegation-installer.test.ts | 79 ++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 16 deletions(-) diff --git a/src/codex/delegation-installer.ts b/src/codex/delegation-installer.ts index 9d72973deb..42133862e8 100644 --- a/src/codex/delegation-installer.ts +++ b/src/codex/delegation-installer.ts @@ -464,12 +464,14 @@ function safeWrite( paths: Paths, path: string, expected: FileSnapshot, - text: string, + desired: string | Buffer, artifact: "skill" | "agents", deps: CodexDelegationInstallerDeps, ): FileSnapshotPresent { const limit = artifact === "skill" ? SKILL_LIMIT : AGENTS_LIMIT; - const bytes = Buffer.from(text, "utf8"); + const bytes = typeof desired === "string" + ? Buffer.from(desired, "utf8") + : Buffer.from(desired); if (bytes.length > limit) { throw new DelegationFsError("too_large", "delegation output exceeds its read bound"); } @@ -591,7 +593,7 @@ function compensate(context: InspectionContext, applied: AppliedMutation, deps: if (applied.before.kind === "absent") { if (applied.after.kind === "file") safeRemove(context.paths, applied.path, applied.after, applied.artifact, deps); } else { - safeWrite(context.paths, applied.path, applied.after, applied.before.text, applied.artifact, deps); + safeWrite(context.paths, applied.path, applied.after, applied.before.bytes, applied.artifact, deps); } } @@ -616,8 +618,12 @@ function removeEmptySkillDir(paths: Paths, expected: BigIntStats): void { } } -function failureStatus(deps: CodexDelegationInstallerDeps, fallback: CodexDelegationStatus): CodexDelegationStatus { - try { return buildInspection(deps).status; } catch { return fallback; } +function failureStatus(deps: CodexDelegationInstallerDeps): CodexDelegationStatus { + try { + return buildInspection(deps).status; + } catch (error) { + return unsafeStatus(classifyIoError(error).reason); + } } export function mutateCodexDelegation( @@ -628,13 +634,11 @@ export function mutateCodexDelegation( return { ok: false, changed: false, reason: "mutation_busy", status: inspectCodexDelegation(deps) }; } mutationInProgress = true; - let initialStatus = unsafeStatus("unsafe_path"); try { const paths = resolvePaths(deps); assertNotRealHomeUnderTest(paths.skillPath); assertNotRealHomeUnderTest(paths.agentsPath); const context = buildInspection({ ...deps, userHome: paths.userHome, codexHome: paths.codexHome }); - initialStatus = context.status; if (context.status.artifacts.skill.state === "foreign") { return { ok: false, changed: false, reason: "foreign_skill", status: context.status }; } @@ -685,17 +689,17 @@ export function mutateCodexDelegation( ? error : new DelegationFsError("write_failed", "delegation mutation failed", false, { cause: error }); if (classified.published) { - return { ok: false, changed: true, reason: "partial_write", status: failureStatus(deps, initialStatus) }; + return { ok: false, changed: true, reason: "partial_write", status: failureStatus(deps) }; } if (first !== null && index > 0) { try { compensate(context, first, deps); - return { ok: false, changed: false, reason: classified.reason, status: failureStatus(deps, initialStatus) }; + return { ok: false, changed: false, reason: classified.reason, status: failureStatus(deps) }; } catch { - return { ok: false, changed: true, reason: "partial_write", status: failureStatus(deps, initialStatus) }; + return { ok: false, changed: true, reason: "partial_write", status: failureStatus(deps) }; } } - return { ok: false, changed: false, reason: classified.reason, status: failureStatus(deps, initialStatus) }; + return { ok: false, changed: false, reason: classified.reason, status: failureStatus(deps) }; } } @@ -705,7 +709,7 @@ export function mutateCodexDelegation( return { ok: true, changed, status }; } catch (error) { const classified = classifyIoError(error); - return { ok: false, changed: classified.published, reason: classified.reason, status: failureStatus(deps, initialStatus) }; + return { ok: false, changed: classified.published, reason: classified.reason, status: failureStatus(deps) }; } finally { mutationInProgress = false; } diff --git a/tests/codex-delegation-installer.test.ts b/tests/codex-delegation-installer.test.ts index c317b5ea29..199779d0bc 100644 --- a/tests/codex-delegation-installer.test.ts +++ b/tests/codex-delegation-installer.test.ts @@ -335,6 +335,34 @@ describe("Codex delegation installer", () => { expect(mutateCodexDelegation({ action: "install", mode: "balanced" }, large.deps)).toMatchObject({ reason: "too_large" }); }); + test("AGENTS output exactly at the read bound is accepted", () => { + const fx = fixture(); + const agentsBlock = renderCodexDelegationBundle("balanced").agentsBlockText; + const agentsBefore = "x".repeat(1024 * 1024 - Buffer.byteLength(agentsBlock, "utf8") - 1); + write(fx.agentsPath, agentsBefore); + + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + + expect(outcome).toMatchObject({ ok: true, changed: true, status: { state: "current" } }); + expect(readFileSync(fx.agentsPath).byteLength).toBe(1024 * 1024); + expect(readFileSync(fx.agentsPath, "utf8")).toBe(`${agentsBefore}\n${agentsBlock}`); + }); + + test("multibyte AGENTS output crossing the byte bound refuses without residue", () => { + const fx = fixture(); + const agentsBefore = `${"é".repeat((1024 * 1024 - 1) / 2)}x`; + write(fx.agentsPath, agentsBefore); + const exactBefore = readFileSync(fx.agentsPath); + + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + + expect(agentsBefore.length).toBeLessThan(1024 * 1024); + expect(exactBefore.byteLength).toBe(1024 * 1024 - 1); + expect(outcome).toMatchObject({ ok: false, changed: false, reason: "too_large" }); + expect(readFileSync(fx.agentsPath)).toEqual(exactBefore); + expect(existsSync(fx.skillPath)).toBe(false); + }); + test("AGENTS output crossing the read bound refuses before publication and compensates the skill", () => { const fx = fixture(); const agentsBefore = Buffer.alloc(1024 * 1024 - 1, 0x78); @@ -356,6 +384,27 @@ describe("Codex delegation installer", () => { expect(readdirSync(fx.codexHome)).toEqual(["AGENTS.md"]); }); + test("oversized-derived AGENTS refusal restores exact managed skill bytes and mode", () => { + const fx = fixture(); + const skillBefore = Buffer.concat([ + Buffer.from([0xef, 0xbb, 0xbf]), + Buffer.from(renderCodexDelegationBundle("balanced").skillText, "utf8"), + ]); + const agentsBefore = Buffer.alloc(1024 * 1024 - 1, 0x78); + write(fx.skillPath, skillBefore); + chmodSync(fx.skillPath, 0o640); + write(fx.agentsPath, agentsBefore); + const skillModeBefore = lstatSync(fx.skillPath).mode & 0o777; + if (process.platform !== "win32") expect(skillModeBefore).toBe(0o640); + + const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); + + expect(outcome).toMatchObject({ ok: false, changed: false, reason: "too_large" }); + expect(readFileSync(fx.skillPath)).toEqual(skillBefore); + expect(lstatSync(fx.skillPath).mode & 0o777).toBe(skillModeBefore); + expect(readFileSync(fx.agentsPath)).toEqual(agentsBefore); + }); + test("changed preimage before publish refuses", () => { const fx = fixture(); write(fx.agentsPath, "user preface\n"); @@ -425,7 +474,7 @@ describe("Codex delegation installer", () => { expect(readFileSync(fx.skillPath, "utf8")).toBe("concurrent replacement"); }); - test("a DelegationFsError after rename reports a published partial write", () => { + test("an oversized post-rename skill reports a published partial write with unsafe status", () => { const fx = fixture(); const deps = { ...fx.deps, @@ -438,7 +487,18 @@ describe("Codex delegation installer", () => { const outcome = mutateCodexDelegation({ action: "install", mode: "balanced" }, deps); - expect(outcome).toMatchObject({ ok: false, changed: true, reason: "partial_write" }); + expect(outcome).toMatchObject({ + ok: false, + changed: true, + reason: "partial_write", + status: { + state: "unsafe", + artifacts: { + skill: { state: "unsafe", reason: "too_large" }, + agentsPolicy: { state: "unsafe", reason: "too_large" }, + }, + }, + }); expect(readFileSync(fx.skillPath).byteLength).toBe(256 * 1024 + 1); expect(existsSync(fx.agentsPath)).toBe(false); }); @@ -464,7 +524,7 @@ describe("Codex delegation installer", () => { expect(existsSync(fx.agentsPath)).toBe(false); }); - test("a DelegationFsError after unlink reports a published partial write", () => { + test("a directory recreated after unlink reports a published partial write with unsafe status", () => { const fx = fixture(); mutateCodexDelegation({ action: "install", mode: "balanced" }, fx.deps); const deps = { @@ -476,7 +536,18 @@ describe("Codex delegation installer", () => { const outcome = mutateCodexDelegation({ action: "uninstall" }, deps); - expect(outcome).toMatchObject({ ok: false, changed: true, reason: "partial_write" }); + expect(outcome).toMatchObject({ + ok: false, + changed: true, + reason: "partial_write", + status: { + state: "unsafe", + artifacts: { + skill: { state: "unsafe", reason: "unsafe_path" }, + agentsPolicy: { state: "unsafe", reason: "unsafe_path" }, + }, + }, + }); expect(lstatSync(fx.skillPath).isDirectory()).toBe(true); expect(readFileSync(fx.agentsPath, "utf8")).toBe(""); });