Repository navigation
refactor(codex): generate native role files for skill personas - #8384
Conversation
codexRoles turns the personas discoverPersonas registers, plus the built-in default and explore roles, into agents/<name>.toml files with name, description and developer_instructions (fullsend-ai#6970). A reserved, duplicate, description-less or body-less persona is skipped with a warning; model, tools, permissionMode, background and maxTurns are reported and never rendered. piAgentDef keeps the raw frontmatter; piPersona gains Description, Body and Frontmatter. Nothing calls this yet. Signed-off-by: Dharit Shah <dhshah@redhat.com>
For an agent whose tools include Agent, with harness security on and a parent model Codex resolves to collaboration V1 (its entry in the pinned CLI's bundled catalog says v1 or carries no version; codex debug models --bundled, read at Bootstrap), Bootstrap creates $CODEX_HOME/agents/, uploads one regular file per role and records each digest in the runner-held set, so the spawn guard admits the registered roles and denies a planted or edited file. Every other agent, and every run with security off, gets today's files. config.toml keeps [agents] enabled = false until the dispatch note lands (fullsend-ai#6970). Signed-off-by: Dharit Shah <dhshah@redhat.com>
PR Summary by QodoGenerate guarded Codex role files for skill personas
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
|
🤖 Review · Commit: |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Site previewPreview: https://857287f4-site.fullsend-ai.workers.dev Commit: |
parsePiAgent assigned def.Frontmatter twice on consecutive lines. Signed-off-by: Dharit Shah <dhshah@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 1:52 AM UTC · Completed 2:03 AM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $2.81 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Risk Assessment: moderate (2/5) DetailsModerate risk: large change size and active churn on runtime bootstrap files are offset by zero protected or security-sensitive paths, default-disabled feature gating, and alignment with linked issue #6970. Previous runRisk Assessment: moderate (2/5) DetailsModerate risk: large change size and active churn on runtime bootstrap files are offset by zero protected or security-sensitive paths, default-disabled feature gating, and alignment with linked issue #6970. Previous run (2)Risk Assessment: moderate (2/5) DetailsModerate risk: large change size and active churn on runtime bootstrap files are offset by zero protected or security-sensitive paths, default-disabled feature gating, and alignment with linked issue #6970. |
|
Looks good to me Previous runLooks good to me Previous run (2)ReviewFindingsMedium
Low
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
requires-manual-review
The toml comment and the TOML test read as if a role file never carries a model line. ADR 0126's per-persona model arrives with the PR of fullsend-ai#6970 that makes delegation reachable; default and explore never carry one. The expected persona paths in TestCodexRoles_DiscoverySkipIsAWarning are joined with filepath, as discoverPersonas builds them. Signed-off-by: Dharit Shah <dhshah@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 2:10 AM UTC · Completed 2:18 AM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.92 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ready-for-merge
dhshah13
added
the
ok-to-test
A native child runs with its role's developer_instructions in place of the parent's, so nothing of codexNoSubagentNote reached it, including the chunked-read rule for exec truncation (fullsend-ai#7831), and only the built-in roles stated the child contract. Every role, persona or built-in, now opens with codexChildNote: pi's CHILD_SYSTEM_NOTE word for word, then codexChunkedReadNote, the paragraph factored out of codexNoSubagentNote. The parent's note and config.toml are byte-equal. The built-ins drop their own contract clause. Signed-off-by: Dharit Shah <dhshah@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 2:24 PM UTC · Completed 2:32 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.83 |
Superseded by updated review
Summary
For an agent whose tools include
Agent, with harness security on and a parent model Codexresolves to collaboration V1, Bootstrap writes one TOML role file per skill persona, plus the
built-in
defaultandexploreroles, to$CODEX_HOME/agents/and records their digests in therunner-held set PR 1's spawn guard checks. Every other agent, and every run with security off, gets
the same files as today.
Inert until PR 6:
config.tomlkeeps[agents] enabled = false, so Codex's multi-agent tools stayoff and no child loads a role file;
config.toml,hooks.jsonand the launch command are byte-equalto
main(goldens).Related Issue
Part of #6970. Third of the series that replaces the closed #7809 after ADR 0126 (#7827); builds on
#8122 and #8213.
Changes
internal/runtime/codex_personas.go: the gate (codexDelegates,codexParentResolvesV1on thepinned CLI's bundled catalog), the catalog type and parser, the role list (
codexRoles: skillpersonas plus
defaultandexplore, sorted) and the role TOML (name,description,developer_instructions; nomodelline in this PR: ADR 0126's per-persona model arriveswith PR 6 of the codex runtime: run the review/retro sub-agent rosters through codex's native spawn_agent roles #6970 order, where delegation becomes reachable;
defaultandexplorenevercarry one). A child runs with its role's
developer_instructionsin place of the parent's, soevery role opens with
codexChildNote: pi's child contract and the chunked-read rule.internal/runtime/pi_agent.go,pi_personas.go:piAgentDefkeeps the raw frontmatter andpiPersonacarries it withDescriptionandBody, the only pi change.internal/runtime/codex_bootstrap.go: the catalog preflight (codex debug models --bundled,offline, like
codex --version); the gate is decided beforeconfig.tomlis rendered; when itpasses,
agents/is created, one regular file per role is uploaded andRoleFilesrecords eachdigest. The chunked-read paragraph of
codexNoSubagentNoteis factored out ascodexChunkedReadNote, shared with the role files;config.tomlstays byte-equal (golden).internal/runtime/codex_personas_test.go,codex_bootstrap_test.go: role rules (reserved,duplicate, description-less and body-less personas skipped with a warning;
model,tools,permissionMode,background,maxTurnsreported and never rendered), the catalog, the gateon and off (V2, unlisted and run-level-V2 parents off; a parent whose entry carries no version
on), the three failure paths, and a planted or edited role file denying the spawn through the
installed hook adapter.
internal/runtime/testdata/codex/models-bundled-0.159.3.json: the pinned CLI's catalog reducedto
slugandmulti_agent_version; regenerated on a CLI bump, never edited.internal/runtime/codex_golden_test.go,testdata/codex/golden/roles/*.toml,launch-delegating.txt: four golden cases; the delegating launch differs fromlaunch-hooks.txtin the
FULLSEND_CODEX_SPAWN_DIGESTSvalue only, which a test pins.docs/contributing/runtime-implementation.mdwas consulted and updated: the layout tree, therole-file row of the guard-anchor table, the "not wired" bullet, a re-check row for the catalog,
the child note next to the chunked-read rule.
The gate is ADR 0126's Decision as written: a parent whose entry in the bundled catalog selects
v1, or carries no version, delegates; a V2 or unlisted parent runs in one context, with onestderr line saying why.
Interpretations, for a maintainer to correct: a reserved (
enabled,explorer,workeronCodex), duplicate, description-less or body-less persona is a warning, never a Bootstrap error; a
tools:list naming onlyTaskdelegates, asTaskalready maps tospawn_agent; the parentmodel is the one
Runlaunches with, and one that does not translate to an OpenAI id is treatedas unlisted (
Runreports it as today); hidden catalog entries count, socodex-auto-reviewresolves to V1 as a parent.
Testing
make lintpasses (staged first)TestCodexDelegates,TestDiscoverPersonas_CarriesDescriptionAndBody,TestCodexRole_*,TestCodexRoles_*,TestCodexDroppedFrontmatterKeys,TestCodexCatalog,TestCodexParentResolvesV1,TestCodexPreflightCatalog,TestCodexRolesDir,TestCodexRuntimeBootstrap_WritesRoleFilesForADelegatingAgent,TestCodexRuntimeBootstrap_NoRoleFilesWhenTheGateFails,TestCodexRuntimeBootstrap_ReportsRoleFileFailures,TestCodexBootstrap_PlantedRoleFileDeniesSpawn,TestCodexGolden_DelegatingLaunchDiffersOnlyInSpawnDigestsand fourTestCodexGoldencases; theexisting pi and claude tests are unedited.
go test -race ./internal/runtime/andmake go-testgreen;go vetclean; docs build.origin/mainat29695351; Codex pinCODEX_VERSION=0.159.3.go tool cover -func): every new function 100% (codexDelegates,codexRoles,codexRole.toml,codexDroppedFrontmatterKeys,codexCatalog.has,codexCatalog.resolvesV1,parseCodexCatalog,codexParentResolvesV1,codexRolesDir) exceptcodexPreflightCatalog90%;Bootstrap87%; touchedparsePiAgent100%,discoverPersonas87%.Checklist
🤖 Generated with Claude Code