From c96a6c0332255d4cab891d11211d0d11c3f79fd7 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 20 Oct 2026 15:47:53 -0700 Subject: [PATCH 1/4] fix(cli): a public Codex seat gets its declared skills as AGENTS.md (0.1.118) The Codex adapter never read `environment.skills`, so every self-serve Codex seat (all public) ran without the Commonly skill buildDefaultEnvironment declares for it: the Codex half of the gap #2354 closed for Claude. `codex exec` has no skill loader a public seat can use, but it reads $CODEX_HOME/AGENTS.md as the user's instructions, and a public seat's CODEX_HOME is one this CLI owns (preparePublicCodexHome). Measured on codex-cli 0.162.1 with the public flags (`--ignore-user-config` skips config.toml only): a marker in that file came back on a new thread, and a changed marker came back on `exec resume`, so existing seats pick it up on their next turn. - codex.js: public spawns write the declared `skills.claude` SKILL.md bodies (ADR-008's SKILL.md list; Codex reads the same format) to the public home's AGENTS.md, 0600, staged then renamed, capped at 32 KiB; removed when nothing is declared. A private seat's home is the operator's and is never written. - skill-prompt.js: the reader #2354 wrote inside claude.js, moved out so both adapters render a skill the same way; Claude's output is unchanged. environment.js's mountSkills uses the shared path resolver. Walk on the real binary through the adapter: with the bundled skill declared, the seat quoted "# Being a good Commonly member" (3.5 s); with none declared, "NONE". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG --- cli/__tests__/adapters.codex.skills.test.mjs | 148 +++++++++++++++++++ cli/package-lock.json | 4 +- cli/package.json | 2 +- cli/src/lib/adapters/claude.js | 37 +---- cli/src/lib/adapters/codex.js | 39 +++++ cli/src/lib/environment.js | 6 +- cli/src/lib/skill-prompt.js | 67 +++++++++ 7 files changed, 262 insertions(+), 41 deletions(-) create mode 100644 cli/__tests__/adapters.codex.skills.test.mjs create mode 100644 cli/src/lib/skill-prompt.js diff --git a/cli/__tests__/adapters.codex.skills.test.mjs b/cli/__tests__/adapters.codex.skills.test.mjs new file mode 100644 index 0000000000..427c8c40b7 --- /dev/null +++ b/cli/__tests__/adapters.codex.skills.test.mjs @@ -0,0 +1,148 @@ +/** + * A public Codex seat gets its declared skills as $CODEX_HOME/AGENTS.md. + * + * Found 2026-10-20: this adapter never read `environment.skills`, so every + * outsider's Codex seat ran without the house rules buildDefaultEnvironment + * declares for it. The Claude half of the same gap shipped in #2354. Codex + * reads $CODEX_HOME/AGENTS.md under the public flags, on a new thread and on + * `exec resume` (measured on codex-cli 0.162.1); a public seat's home is one + * the CLI owns, so the declared SKILL.md bodies are written there. + */ + +import { jest } from '@jest/globals'; +import { EventEmitter } from 'events'; +import { + mkdir, + mkdtemp, + readFile, + rm, + stat, + writeFile, +} from 'fs/promises'; +import { tmpdir } from 'os'; +import { join } from 'path'; + +await jest.unstable_mockModule('child_process', () => ({ + spawnSync: jest.fn(), + spawn: jest.fn(), +})); + +const codex = (await import('../src/lib/adapters/codex.js')).default; + +// The fake codex reads AGENTS.md while it "runs", which is when the real one +// would: the file must be in place before the child starts. +const makeSpawnImpl = () => { + const calls = []; + const impl = (cmd, args, opts) => { + const home = opts.env.CODEX_HOME; + const seen = readFile(join(home, 'AGENTS.md'), 'utf8').catch(() => null); + calls.push({ args, opts, seen }); + const proc = new EventEmitter(); + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.kill = jest.fn(); + setTimeout(async () => { + await seen; + proc.stdout.emit('data', Buffer.from('{"type":"thread.started","thread_id":"sid-skill"}\n')); + const outIdx = args.indexOf('-o'); + await writeFile(args[outIdx + 1], 'ok', 'utf8'); + proc.emit('close', 0); + }, 0); + return proc; + }; + return { impl, calls }; +}; + +const HOUSE_RULES = '---\nname: commonly\ndescription: How to behave in a pod.\n---\n\n# Being a good Commonly member\n\nOrient before you post.\n'; + +describe('codex adapter — a public seat gets its declared skills as AGENTS.md', () => { + let root; + let operatorHome; + let publicHome; + let workspace; + let skillSrc; + + beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'cli-codex-skill-')); + operatorHome = join(root, 'operator-codex'); + publicHome = join(root, 'public-codex'); + workspace = join(root, 'workspace'); + skillSrc = join(root, 'skills', 'commonly'); + await Promise.all([operatorHome, publicHome, workspace, skillSrc].map((dir) => mkdir(dir, { recursive: true }))); + await writeFile(join(skillSrc, 'SKILL.md'), HOUSE_RULES, 'utf8'); + await writeFile(join(operatorHome, 'AGENTS.md'), '# The operator\'s private instructions\n', 'utf8'); + }); + afterEach(async () => { await rm(root, { recursive: true, force: true }); }); + + const spawnSeat = (impl, environment, extra = {}) => codex.spawn('hi', { + sessionId: null, + cwd: workspace, + agentName: 'skill-test-agent', + environment, + env: { ...process.env, CODEX_HOME: operatorHome }, + _publicCodexHome: publicHome, + _spawnImpl: impl, + ...extra, + }); + + test('the skill body is in the public home\'s AGENTS.md before codex starts, without frontmatter', async () => { + const { impl, calls } = makeSpawnImpl(); + await spawnSeat(impl, { sandbox: { trust: 'public' }, skills: { claude: [skillSrc] } }); + + expect(calls).toHaveLength(1); + expect(calls[0].opts.env.CODEX_HOME).toBe(publicHome); + const seen = await calls[0].seen; + expect(seen).toContain('## Skill: commonly'); + expect(seen).toContain('# Being a good Commonly member\n\nOrient before you post.'); + expect(seen).not.toContain('description: How to behave'); + expect(seen).not.toContain('private instructions'); + }); + + test('the file is private to the operator', async () => { + const { impl } = makeSpawnImpl(); + await spawnSeat(impl, { sandbox: { trust: 'public' }, skills: { claude: [skillSrc] } }); + // eslint-disable-next-line no-bitwise + expect((await stat(join(publicHome, 'AGENTS.md'))).mode & 0o777).toBe(0o600); + }); + + test('a relative skill path is taken from the workspace', async () => { + const local = join(workspace, 'house'); + await mkdir(local); + await writeFile(join(local, 'SKILL.md'), '# Local rules\n\nBe brief.\n', 'utf8'); + const { impl, calls } = makeSpawnImpl(); + await spawnSeat(impl, { sandbox: { trust: 'public' }, skills: { claude: ['./house'] } }); + + expect(await calls[0].seen).toContain('## Skill: house\n\n# Local rules'); + }); + + test('a skill dropped from the declaration stops applying on the next spawn', async () => { + const first = makeSpawnImpl(); + await spawnSeat(first.impl, { sandbox: { trust: 'public' }, skills: { claude: [skillSrc] } }); + expect(await first.calls[0].seen).toContain('## Skill: commonly'); + + const second = makeSpawnImpl(); + await spawnSeat(second.impl, { sandbox: { trust: 'public' } }); + expect(await second.calls[0].seen).toBeNull(); + }); + + test('a declared skill with no SKILL.md is skipped, not fatal', async () => { + await rm(join(skillSrc, 'SKILL.md')); + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const { impl, calls } = makeSpawnImpl(); + await spawnSeat(impl, { sandbox: { trust: 'public' }, skills: { claude: [skillSrc] } }); + + expect(calls).toHaveLength(1); + expect(await calls[0].seen).toBeNull(); + expect(warn).toHaveBeenCalledWith(`[codex] public seat: no SKILL.md to inline at ${skillSrc}`); + warn.mockRestore(); + }); + + test('a private seat runs on the operator\'s own home, which is never written', async () => { + const { impl, calls } = makeSpawnImpl(); + await spawnSeat(impl, { skills: { claude: [skillSrc] } }); + + expect(calls[0].opts.env.CODEX_HOME).toBe(operatorHome); + expect(await readFile(join(operatorHome, 'AGENTS.md'), 'utf8')).toBe('# The operator\'s private instructions\n'); + await expect(stat(join(publicHome, 'AGENTS.md'))).rejects.toMatchObject({ code: 'ENOENT' }); + }); +}); diff --git a/cli/package-lock.json b/cli/package-lock.json index c9fc4d5369..fee9b4f5c7 100644 --- a/cli/package-lock.json +++ b/cli/package-lock.json @@ -1,12 +1,12 @@ { "name": "@commonlyai/cli", - "version": "0.1.117", + "version": "0.1.118", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@commonlyai/cli", - "version": "0.1.117", + "version": "0.1.118", "license": "Apache-2.0", "dependencies": { "commander": "^12.0.0" diff --git a/cli/package.json b/cli/package.json index f27c1dc898..ee5c1ff0d7 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@commonlyai/cli", - "version": "0.1.117", + "version": "0.1.118", "license": "Apache-2.0", "description": "The Commonly CLI — connect agents, manage pods, iterate fast", "type": "module", diff --git a/cli/src/lib/adapters/claude.js b/cli/src/lib/adapters/claude.js index b349155fd7..1184fbdb18 100644 --- a/cli/src/lib/adapters/claude.js +++ b/cli/src/lib/adapters/claude.js @@ -79,6 +79,7 @@ import { prepareMcpSpawn, PUBLIC_MCP_UNAVAILABLE_CODE, publicMcpUnavailableError, selectedMcpVersionDir, } from '../mcp-home.js'; import { buildMemoryPreamble } from '../memory-bridge.js'; +import { skillsAsText } from '../skill-prompt.js'; import { adapterFailure, spawnCredentials } from '../upstream-refusal.js'; // See codex.js for the rationale on bumping the default + env override. @@ -274,41 +275,9 @@ const buildPublicClaudePolicyArgs = (mcpToolPatterns) => { // be admitted through Seatbelt. Linux caps one argv string at 128 KiB, so the // total is capped well under it. const PUBLIC_SKILL_PROMPT_MAX_BYTES = 96 * 1024; -const SKILL_FRONTMATTER_RE = /^---\r?\n([\s\S]*?)\r?\n---[ \t]*\r?\n?/; - const publicSkillPromptArgs = async (skillSources) => { - const sections = []; - let total = 0; - for (const source of skillSources || []) { - let markdown; - try { - // eslint-disable-next-line no-await-in-loop - markdown = await readFile(join(source, 'SKILL.md'), 'utf8'); - } catch { - // eslint-disable-next-line no-console - console.warn(`[claude] public seat: no SKILL.md to inline at ${source}`); - continue; - } - const frontmatter = SKILL_FRONTMATTER_RE.exec(markdown); - const name = (frontmatter && /^name:\s*(.+)$/m.exec(frontmatter[1])?.[1].trim()) - || source.split(/[\\/]/).filter(Boolean).pop(); - const section = `## Skill: ${name}\n\n${markdown.slice(frontmatter ? frontmatter[0].length : 0).trim()}`; - const bytes = Buffer.byteLength(section, 'utf8'); - if (total + bytes > PUBLIC_SKILL_PROMPT_MAX_BYTES) { - // eslint-disable-next-line no-console - console.warn(`[claude] public seat: skill ${name} not inlined (over ${PUBLIC_SKILL_PROMPT_MAX_BYTES} bytes in total)`); - continue; - } - total += bytes; - sections.push(section); - } - if (sections.length === 0) return []; - return [ - '--append-system-prompt', - 'These skills are part of how you work here. Skills are not loaded as files in this seat, ' - + 'so each one is given in full below. Follow them as you would a loaded skill.\n\n' - + sections.join('\n\n'), - ]; + const text = await skillsAsText(skillSources, { maxBytes: PUBLIC_SKILL_PROMPT_MAX_BYTES, label: 'claude' }); + return text ? ['--append-system-prompt', text] : []; }; const runClaude = ({ cmd, args, cwd, env, timeoutMs, credentials = [], spawnImpl = childSpawn }) => new Promise((resolve, reject) => { diff --git a/cli/src/lib/adapters/codex.js b/cli/src/lib/adapters/codex.js index 57878390a3..4043f098ba 100644 --- a/cli/src/lib/adapters/codex.js +++ b/cli/src/lib/adapters/codex.js @@ -47,9 +47,11 @@ import { mkdtemp, readFile, readlink, + rename, rm, symlink, unlink, + writeFile, } from 'fs/promises'; import { homedir, tmpdir } from 'os'; import { @@ -62,6 +64,7 @@ import { supportedGitHooks } from '../commit-attribution.js'; import { deliverSeatCredential, withholdRuntimeCredential } from '../mcp-credential-delivery.js'; import { prepareMcpSpawn, publicMcpUnavailableError, selectedMcpVersionDir } from '../mcp-home.js'; import { buildMemoryPreamble } from '../memory-bridge.js'; +import { resolveSkillSource, skillsAsText } from '../skill-prompt.js'; import { isLegacySandboxTrust, normalizeSandboxTrust } from '../environment.js'; import { adapterFailure, spawnCredentials } from '../upstream-refusal.js'; @@ -308,6 +311,41 @@ const preparePublicCodexHome = async (ctx) => { return publicHome; }; +// A public seat ran with none of its declared skills: this adapter never read +// `environment.skills`, so every outsider's Codex worked without the house +// rules buildDefaultEnvironment declares for it (found 2026-10-20 beside the +// Claude half of the same gap, #2354). `codex exec` has no skill loader a +// public seat can use, but it reads $CODEX_HOME/AGENTS.md as the user's own +// instructions, and the public home is one this CLI owns. Measured on +// codex-cli 0.162.1 with this adapter's public flags (`--ignore-user-config` +// skips config.toml only): a marker in that file came back verbatim on a new +// thread, and a changed marker came back on `exec resume` of the same thread, +// so a seat picks the skills up on its next turn either way. +// +// `skills.claude` is ADR-008's SKILL.md list ("drivers copy or symlink as +// appropriate"); Codex reads the same format, so no new key. Rewritten each +// spawn (staged, then renamed, so a concurrent read never sees half a file), +// and removed when nothing is declared so a dropped skill stops applying. +// The cap is Codex's default `project_doc_max_bytes` for AGENTS.md files. +const PUBLIC_CODEX_INSTRUCTIONS_MAX_BYTES = 32 * 1024; + +const writePublicCodexInstructions = async (publicHome, ctx) => { + const declared = Array.isArray(ctx.environment?.skills?.claude) ? ctx.environment.skills.claude : []; + const baseDir = ctx.cwd || process.cwd(); + const text = await skillsAsText( + declared.map((source) => resolveSkillSource(source, baseDir)), + { maxBytes: PUBLIC_CODEX_INSTRUCTIONS_MAX_BYTES, label: 'codex' }, + ); + const target = join(publicHome, 'AGENTS.md'); + if (!text) { + await rm(target, { force: true }); + return; + } + const staged = `${target}.${process.pid}.tmp`; + await writeFile(staged, `${text}\n`, { encoding: 'utf8', mode: 0o600 }); + await rename(staged, target); +}; + const publicPermissionProfileFlags = ( mode, commitAttributionEnv = null, @@ -653,6 +691,7 @@ export default { } if (publicSandboxMode !== null) { childEnv.CODEX_HOME = await preparePublicCodexHome(ctx); + await writePublicCodexInstructions(childEnv.CODEX_HOME, ctx); } const { threadId } = await runCodex({ diff --git a/cli/src/lib/environment.js b/cli/src/lib/environment.js index 5add93e5b3..e8865b43db 100644 --- a/cli/src/lib/environment.js +++ b/cli/src/lib/environment.js @@ -20,6 +20,7 @@ import { import { existsSync } from 'fs'; import { isAbsolute, join, resolve as pathResolve, basename } from 'path'; import { homedir } from 'os'; +import { resolveSkillSource } from './skill-prompt.js'; // ── Schema — keep the allow-list narrow; ADR-008 §invariants #1+#2 ────────── // @@ -524,10 +525,7 @@ export const mountSkills = async (spec, workspacePath, envFileDir = null) => { await mkdir(skillsDir, { recursive: true }); for (const rawSource of sources) { - const source = expandHome(rawSource); - const absSource = isAbsolute(source) - ? source - : pathResolve(envFileDir || workspacePath, source); + const absSource = resolveSkillSource(rawSource, envFileDir || workspacePath); if (!existsSync(absSource)) { conflicted.push({ path: absSource, reason: 'missing-source' }); continue; diff --git a/cli/src/lib/skill-prompt.js b/cli/src/lib/skill-prompt.js new file mode 100644 index 0000000000..377c841915 --- /dev/null +++ b/cli/src/lib/skill-prompt.js @@ -0,0 +1,67 @@ +// A public seat cannot load its declared skills as files, so each adapter +// delivers the SKILL.md bodies as instructions text instead: +// - claude: skills are disabled outright (`--disable-slash-commands`, +// `--setting-sources ''`), so the text rides as `--append-system-prompt`; +// - codex: `exec` has no skill loader the seat can use, and a public seat's +// CODEX_HOME is one this CLI owns, so the text is that home's AGENTS.md. +// One reader for both, so the two runtimes cannot drift on what a skill says. +import { readFile } from 'fs/promises'; +import { isAbsolute, join, resolve as pathResolve } from 'path'; +import { homedir } from 'os'; + +const SKILL_FRONTMATTER_RE = /^---\r?\n([\s\S]*?)\r?\n---[ \t]*\r?\n?/; + +export const SKILLS_AS_TEXT_PREAMBLE = 'These skills are part of how you work here. Skills are not loaded as files in this seat, ' + + 'so each one is given in full below. Follow them as you would a loaded skill.'; + +/** + * A declared skill path as the environment spec means it: `~/` expanded, + * relative paths taken from `baseDir`. The same resolution mountSkills applies. + */ +export const resolveSkillSource = (rawSource, baseDir) => { + let source = rawSource; + if (source === '~') source = homedir(); + else if (typeof source === 'string' && source.startsWith('~/')) source = join(homedir(), source.slice(2)); + return isAbsolute(source) ? source : pathResolve(baseDir, source); +}; + +/** + * Each source's SKILL.md as `## Skill: ` plus its body without the + * frontmatter, in declared order, stopping short of `maxBytes` in total. A + * source with no SKILL.md, or one that would cross the cap, is skipped with a + * warning naming the runtime (`label`) rather than failing the spawn. + */ +export const readSkillSections = async (skillSources, { maxBytes, label }) => { + const sections = []; + let total = 0; + for (const source of skillSources || []) { + let markdown; + try { + // eslint-disable-next-line no-await-in-loop + markdown = await readFile(join(source, 'SKILL.md'), 'utf8'); + } catch { + // eslint-disable-next-line no-console + console.warn(`[${label}] public seat: no SKILL.md to inline at ${source}`); + continue; + } + const frontmatter = SKILL_FRONTMATTER_RE.exec(markdown); + const name = (frontmatter && /^name:\s*(.+)$/m.exec(frontmatter[1])?.[1].trim()) + || source.split(/[\\/]/).filter(Boolean).pop(); + const section = `## Skill: ${name}\n\n${markdown.slice(frontmatter ? frontmatter[0].length : 0).trim()}`; + const bytes = Buffer.byteLength(section, 'utf8'); + if (total + bytes > maxBytes) { + // eslint-disable-next-line no-console + console.warn(`[${label}] public seat: skill ${name} not inlined (over ${maxBytes} bytes in total)`); + continue; + } + total += bytes; + sections.push(section); + } + return sections; +}; + +/** The preamble and every section, or null when no skill could be read. */ +export const skillsAsText = async (skillSources, options) => { + const sections = await readSkillSections(skillSources, options); + return sections.length ? `${SKILLS_AS_TEXT_PREAMBLE}\n\n${sections.join('\n\n')}` : null; +}; From 76c24158a303e19f5f19e13696e2d773d82c22b2 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 20 Oct 2026 15:49:58 -0700 Subject: [PATCH 2/4] docs(skill): close every handoff with an @mention C14 call rehearsal, 2026-10-20: Claude handed Codex a change and promised to review it; Codex posted the result without @mentioning Claude, so Claude never woke and the review never came. The kernel now cues the first handoff (#2367); the skill states the rule for every runtime that loads it. Edited in docs/agents/skills/commonly/SKILL.md (prepublishOnly copies it over the packaged one) and mirrored; skill-sync pins it beside the #702 guardrail. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG --- cli/__tests__/skill-sync.test.mjs | 8 ++++++++ cli/skills/commonly/SKILL.md | 6 ++++++ docs/agents/skills/commonly/SKILL.md | 6 ++++++ 3 files changed, 20 insertions(+) diff --git a/cli/__tests__/skill-sync.test.mjs b/cli/__tests__/skill-sync.test.mjs index 10da1ad41a..0a66b24cf9 100644 --- a/cli/__tests__/skill-sync.test.mjs +++ b/cli/__tests__/skill-sync.test.mjs @@ -46,4 +46,12 @@ describe('commonly skill copies stay in sync', () => { const body = fs.readFileSync(DOCS, 'utf8'); expect(body).toContain('Post as yourself, never as your operator'); }); + + // C14 call rehearsal, 2026-10-20: Claude handed Codex a change and promised a + // review; Codex posted the result without @mentioning Claude, so the review + // never came. The kernel cues the first handoff (#2367); this is the rule. + test('the handoff rule is present: close every handoff with an @mention', () => { + const body = fs.readFileSync(DOCS, 'utf8'); + expect(body).toContain('Close every handoff with an @mention'); + }); }); diff --git a/cli/skills/commonly/SKILL.md b/cli/skills/commonly/SKILL.md index 6ae6443dd2..aae7e0ac2b 100644 --- a/cli/skills/commonly/SKILL.md +++ b/cli/skills/commonly/SKILL.md @@ -135,6 +135,12 @@ How to reach out: not for offloading work you can do yourself. If you can do the thing, do it; a capable peer should pick up stalled work, not queue it behind a note. +**Close every handoff with an @mention.** An agent wakes only when it's +@mentioned. When you hand a peer work you'll check, ask it to @mention you when +it's done. When you finish work someone handed you, post the result and +@mention them. Without that mention the result sits unread, and the review you +promised never happens. + **Post intentionally — updates yes, echoes no.** More than one message per turn is *good* when each carries something new: "On it — pulling the numbers." … then, after the work … "Done — here's what I found: …". What's noise is *restating* what diff --git a/docs/agents/skills/commonly/SKILL.md b/docs/agents/skills/commonly/SKILL.md index 6ae6443dd2..aae7e0ac2b 100644 --- a/docs/agents/skills/commonly/SKILL.md +++ b/docs/agents/skills/commonly/SKILL.md @@ -135,6 +135,12 @@ How to reach out: not for offloading work you can do yourself. If you can do the thing, do it; a capable peer should pick up stalled work, not queue it behind a note. +**Close every handoff with an @mention.** An agent wakes only when it's +@mentioned. When you hand a peer work you'll check, ask it to @mention you when +it's done. When you finish work someone handed you, post the result and +@mention them. Without that mention the result sits unread, and the review you +promised never happens. + **Post intentionally — updates yes, echoes no.** More than one message per turn is *good* when each carries something new: "On it — pulling the numbers." … then, after the work … "Done — here's what I found: …". What's noise is *restating* what From 2f69e419081ca72677ead0b1ad2dc4bee08c1f30 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:01:19 -0700 Subject: [PATCH 3/4] test(cli): pin the public Codex skill cap at Codex's own 32 KiB Vera's C5 on #2368: raising PUBLIC_CODEX_INSTRUCTIONS_MAX_BYTES to 1 GiB failed no arm, and the 32 KiB figure was asserted without a source. codex-cli 0.162.1 ships `project_doc_max_bytes = 32768` in its default config (read from the shipped binary); the comment now says so. The new arm inlines a section of exactly 32768 bytes and nothing after it, so a cap one byte lower or any larger cap fails. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015eTFc9yqm2vKZiUyvLuBT4 --- cli/__tests__/adapters.codex.skills.test.mjs | 22 ++++++++++++++++++++ cli/src/lib/adapters/codex.js | 5 ++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/cli/__tests__/adapters.codex.skills.test.mjs b/cli/__tests__/adapters.codex.skills.test.mjs index 427c8c40b7..0a22f6612b 100644 --- a/cli/__tests__/adapters.codex.skills.test.mjs +++ b/cli/__tests__/adapters.codex.skills.test.mjs @@ -137,6 +137,28 @@ describe('codex adapter — a public seat gets its declared skills as AGENTS.md' warn.mockRestore(); }); + test('the cap is Codex\'s own 32 KiB: a section of exactly 32768 bytes is inlined, and nothing after it', async () => { + // Vera's C5 on #2368: raising this cap to 1 GiB failed no arm, and Codex + // truncates AGENTS.md past `project_doc_max_bytes` (32768 at 0.162.1), so a + // larger cap would let a skill be cut with no warning from either side. + const header = '## Skill: big\n\n'; + const big = join(root, 'skills', 'big'); + const after = join(root, 'skills', 'after'); + await mkdir(big, { recursive: true }); + await mkdir(after, { recursive: true }); + await writeFile(join(big, 'SKILL.md'), `---\nname: big\n---\n${'x'.repeat(32768 - header.length)}\n`, 'utf8'); + await writeFile(join(after, 'SKILL.md'), '---\nname: after\n---\nOne more line.\n', 'utf8'); + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const { impl, calls } = makeSpawnImpl(); + await spawnSeat(impl, { sandbox: { trust: 'public' }, skills: { claude: [big, after] } }); + + const seen = await calls[0].seen; + expect(seen).toContain(`${header}${'x'.repeat(32768 - header.length)}`); + expect(seen).not.toContain('## Skill: after'); + expect(warn).toHaveBeenCalledWith('[codex] public seat: skill after not inlined (over 32768 bytes in total)'); + warn.mockRestore(); + }); + test('a private seat runs on the operator\'s own home, which is never written', async () => { const { impl, calls } = makeSpawnImpl(); await spawnSeat(impl, { skills: { claude: [skillSrc] } }); diff --git a/cli/src/lib/adapters/codex.js b/cli/src/lib/adapters/codex.js index 4043f098ba..e02b6d0bba 100644 --- a/cli/src/lib/adapters/codex.js +++ b/cli/src/lib/adapters/codex.js @@ -326,7 +326,10 @@ const preparePublicCodexHome = async (ctx) => { // appropriate"); Codex reads the same format, so no new key. Rewritten each // spawn (staged, then renamed, so a concurrent read never sees half a file), // and removed when nothing is declared so a dropped skill stops applying. -// The cap is Codex's default `project_doc_max_bytes` for AGENTS.md files. +// The cap is Codex's default `project_doc_max_bytes` for AGENTS.md files: +// codex-cli 0.162.1 ships `project_doc_max_bytes = 32768` in its default +// config (read from the shipped binary, 2026-10-21). Codex truncates past it, +// so a skill that would cross it is left out whole rather than cut mid-text. const PUBLIC_CODEX_INSTRUCTIONS_MAX_BYTES = 32 * 1024; const writePublicCodexInstructions = async (publicHome, ctx) => { From 6d065459537f3c7b0f5c886dab214382849e3ecc Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:35:06 -0700 Subject: [PATCH 4/4] fix(cli): the skill cap bounds the AGENTS.md Codex reads, not the sum of sections; say where Codex's own limit applies Vera's HOLD on #2368 at 2f69e419. The cap counted section bytes only, so the preamble, the separators and the final newline came after the budget was spent, and a file the arm certified ran 168 bytes past 32768. readSkillSections now budgets the text skillsAsText returns, with an optional trailer, and the Codex writer passes its final newline as that trailer. Claude's text is unchanged; its 96 KiB budget now counts the preamble too. The comment is corrected from Codex's source at rust-v0.162.1. The global $CODEX_HOME/AGENTS.md is read whole (codex-home/src/instructions/mod.rs) and sits outside project_doc_max_bytes, which budgets only discovered project docs (core/src/agents_md.rs). So 32 KiB is our own ceiling, Codex does not truncate this file, and it never shares a budget with a workspace AGENTS.md. Two arms measure the file one byte either side of 32768. Six mutations each fail at least one: section-only budget, trailer uncounted, trailer counted but not written, cap +1, cap -1, cap 1 GiB. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015eTFc9yqm2vKZiUyvLuBT4 --- cli/__tests__/adapters.codex.skills.test.mjs | 40 ++++++++++++++++---- cli/src/lib/adapters/codex.js | 20 +++++++--- cli/src/lib/skill-prompt.js | 22 +++++++---- 3 files changed, 60 insertions(+), 22 deletions(-) diff --git a/cli/__tests__/adapters.codex.skills.test.mjs b/cli/__tests__/adapters.codex.skills.test.mjs index 0a22f6612b..8f661f97d7 100644 --- a/cli/__tests__/adapters.codex.skills.test.mjs +++ b/cli/__tests__/adapters.codex.skills.test.mjs @@ -28,6 +28,7 @@ await jest.unstable_mockModule('child_process', () => ({ })); const codex = (await import('../src/lib/adapters/codex.js')).default; +const { SKILLS_AS_TEXT_PREAMBLE } = await import('../src/lib/skill-prompt.js'); // The fake codex reads AGENTS.md while it "runs", which is when the real one // would: the file must be in place before the child starts. @@ -137,28 +138,51 @@ describe('codex adapter — a public seat gets its declared skills as AGENTS.md' warn.mockRestore(); }); - test('the cap is Codex\'s own 32 KiB: a section of exactly 32768 bytes is inlined, and nothing after it', async () => { - // Vera's C5 on #2368: raising this cap to 1 GiB failed no arm, and Codex - // truncates AGENTS.md past `project_doc_max_bytes` (32768 at 0.162.1), so a - // larger cap would let a skill be cut with no warning from either side. - const header = '## Skill: big\n\n'; + // Vera on #2368: raising the cap to 1 GiB failed no arm (C5), and the cap + // first counted only the sections, so a file that met it ran 168 bytes past + // it once the preamble, separators and final newline were added. The cap is + // our own ceiling on the file as written (Codex does not cap this file; see + // the comment on PUBLIC_CODEX_INSTRUCTIONS_MAX_BYTES), so these arms measure + // the file itself, one byte either side of 32768. + const capFixture = async (bodyBytes) => { const big = join(root, 'skills', 'big'); const after = join(root, 'skills', 'after'); await mkdir(big, { recursive: true }); await mkdir(after, { recursive: true }); - await writeFile(join(big, 'SKILL.md'), `---\nname: big\n---\n${'x'.repeat(32768 - header.length)}\n`, 'utf8'); + await writeFile(join(big, 'SKILL.md'), `---\nname: big\n---\n${'x'.repeat(bodyBytes)}\n`, 'utf8'); await writeFile(join(after, 'SKILL.md'), '---\nname: after\n---\nOne more line.\n', 'utf8'); + return [big, after]; + }; + // preamble + "\n\n" + "## Skill: big\n\n" + body + "\n" is the whole file. + const bodyForFileOf = (fileBytes) => fileBytes + - Buffer.byteLength(SKILLS_AS_TEXT_PREAMBLE, 'utf8') - 2 - Buffer.byteLength('## Skill: big\n\n', 'utf8') - 1; + + test('a skill that brings AGENTS.md to exactly 32768 bytes is inlined, and the file is 32768 bytes', async () => { const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); const { impl, calls } = makeSpawnImpl(); - await spawnSeat(impl, { sandbox: { trust: 'public' }, skills: { claude: [big, after] } }); + await spawnSeat(impl, { sandbox: { trust: 'public' }, skills: { claude: await capFixture(bodyForFileOf(32768)) } }); const seen = await calls[0].seen; - expect(seen).toContain(`${header}${'x'.repeat(32768 - header.length)}`); + expect(seen).toContain('## Skill: big'); expect(seen).not.toContain('## Skill: after'); + expect(Buffer.byteLength(seen, 'utf8')).toBe(32768); expect(warn).toHaveBeenCalledWith('[codex] public seat: skill after not inlined (over 32768 bytes in total)'); warn.mockRestore(); }); + test('one byte more and that skill is left out whole, while a later one that fits still goes in', async () => { + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const { impl, calls } = makeSpawnImpl(); + await spawnSeat(impl, { sandbox: { trust: 'public' }, skills: { claude: await capFixture(bodyForFileOf(32768) + 1) } }); + + const seen = await calls[0].seen; + expect(seen).not.toContain('## Skill: big'); + expect(seen).toContain('## Skill: after'); + expect(Buffer.byteLength(seen, 'utf8')).toBeLessThanOrEqual(32768); + expect(warn).toHaveBeenCalledWith('[codex] public seat: skill big not inlined (over 32768 bytes in total)'); + warn.mockRestore(); + }); + test('a private seat runs on the operator\'s own home, which is never written', async () => { const { impl, calls } = makeSpawnImpl(); await spawnSeat(impl, { skills: { claude: [skillSrc] } }); diff --git a/cli/src/lib/adapters/codex.js b/cli/src/lib/adapters/codex.js index e02b6d0bba..3872375701 100644 --- a/cli/src/lib/adapters/codex.js +++ b/cli/src/lib/adapters/codex.js @@ -326,10 +326,18 @@ const preparePublicCodexHome = async (ctx) => { // appropriate"); Codex reads the same format, so no new key. Rewritten each // spawn (staged, then renamed, so a concurrent read never sees half a file), // and removed when nothing is declared so a dropped skill stops applying. -// The cap is Codex's default `project_doc_max_bytes` for AGENTS.md files: -// codex-cli 0.162.1 ships `project_doc_max_bytes = 32768` in its default -// config (read from the shipped binary, 2026-10-21). Codex truncates past it, -// so a skill that would cross it is left out whole rather than cut mid-text. +// The cap is our own, set at Codex's `project_doc_max_bytes` (32768 in +// codex-cli 0.162.1's shipped default config, read from the binary 2026-10-21). +// It is not a limit Codex applies to this file. At rust-v0.162.1 the global +// `$CODEX_HOME/AGENTS.md` is read whole and trimmed +// (codex-home/src/instructions/mod.rs) and passed in as user instructions, +// outside the project-doc budget, which counts only the project AGENTS.md files +// discovered from the working directory (core/src/agents_md.rs, +// load_project_instructions). So the skills never share a budget with a +// workspace's own AGENTS.md, and nothing past the cap would be cut. The cap is +// what keeps the injected text to the size Codex allows a project doc. It +// bounds the file as written, final newline included, and a skill that would +// cross it is left out whole. const PUBLIC_CODEX_INSTRUCTIONS_MAX_BYTES = 32 * 1024; const writePublicCodexInstructions = async (publicHome, ctx) => { @@ -337,7 +345,7 @@ const writePublicCodexInstructions = async (publicHome, ctx) => { const baseDir = ctx.cwd || process.cwd(); const text = await skillsAsText( declared.map((source) => resolveSkillSource(source, baseDir)), - { maxBytes: PUBLIC_CODEX_INSTRUCTIONS_MAX_BYTES, label: 'codex' }, + { maxBytes: PUBLIC_CODEX_INSTRUCTIONS_MAX_BYTES, label: 'codex', trailer: '\n' }, ); const target = join(publicHome, 'AGENTS.md'); if (!text) { @@ -345,7 +353,7 @@ const writePublicCodexInstructions = async (publicHome, ctx) => { return; } const staged = `${target}.${process.pid}.tmp`; - await writeFile(staged, `${text}\n`, { encoding: 'utf8', mode: 0o600 }); + await writeFile(staged, text, { encoding: 'utf8', mode: 0o600 }); await rename(staged, target); }; diff --git a/cli/src/lib/skill-prompt.js b/cli/src/lib/skill-prompt.js index 377c841915..54879b5b72 100644 --- a/cli/src/lib/skill-prompt.js +++ b/cli/src/lib/skill-prompt.js @@ -25,15 +25,19 @@ export const resolveSkillSource = (rawSource, baseDir) => { return isAbsolute(source) ? source : pathResolve(baseDir, source); }; +const SECTION_SEPARATOR = '\n\n'; + /** * Each source's SKILL.md as `## Skill: ` plus its body without the - * frontmatter, in declared order, stopping short of `maxBytes` in total. A - * source with no SKILL.md, or one that would cross the cap, is skipped with a - * warning naming the runtime (`label`) rather than failing the spawn. + * frontmatter, in declared order. `maxBytes` bounds the whole text + * skillsAsText returns (preamble, separators and `trailer` included), so a cap + * is the size of what the runtime reads. A source with no SKILL.md, or one that + * would cross the cap, is skipped with a warning naming the runtime (`label`) + * rather than failing the spawn. */ -export const readSkillSections = async (skillSources, { maxBytes, label }) => { +export const readSkillSections = async (skillSources, { maxBytes, label, trailer = '' }) => { const sections = []; - let total = 0; + let total = Buffer.byteLength(SKILLS_AS_TEXT_PREAMBLE, 'utf8') + Buffer.byteLength(trailer, 'utf8'); for (const source of skillSources || []) { let markdown; try { @@ -48,7 +52,7 @@ export const readSkillSections = async (skillSources, { maxBytes, label }) => { const name = (frontmatter && /^name:\s*(.+)$/m.exec(frontmatter[1])?.[1].trim()) || source.split(/[\\/]/).filter(Boolean).pop(); const section = `## Skill: ${name}\n\n${markdown.slice(frontmatter ? frontmatter[0].length : 0).trim()}`; - const bytes = Buffer.byteLength(section, 'utf8'); + const bytes = Buffer.byteLength(SECTION_SEPARATOR, 'utf8') + Buffer.byteLength(section, 'utf8'); if (total + bytes > maxBytes) { // eslint-disable-next-line no-console console.warn(`[${label}] public seat: skill ${name} not inlined (over ${maxBytes} bytes in total)`); @@ -60,8 +64,10 @@ export const readSkillSections = async (skillSources, { maxBytes, label }) => { return sections; }; -/** The preamble and every section, or null when no skill could be read. */ +/** The preamble, every section and `trailer`, or null when no skill could be read. */ export const skillsAsText = async (skillSources, options) => { const sections = await readSkillSections(skillSources, options); - return sections.length ? `${SKILLS_AS_TEXT_PREAMBLE}\n\n${sections.join('\n\n')}` : null; + return sections.length + ? `${SKILLS_AS_TEXT_PREAMBLE}${SECTION_SEPARATOR}${sections.join(SECTION_SEPARATOR)}${options.trailer || ''}` + : null; };