refactor(skill): slim chief-engineer doctrine - #4
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f83320c7f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - Use Claude only with repository/data authorization and only when it did not | ||
| author the affected code, design, or contract. |
There was a problem hiding this comment.
When a Claude executor has already authored part of a change and needs a follow-up implementation, test, or debugging dispatch, this unconditional rule makes Claude ineligible for any further use on the affected code. That conflicts with the executor roles immediately above and broadens the prior review/challenge independence requirement; the README still scopes authorship independence specifically to Claude reviews (README.md lines 231-240). Restrict this condition to independent review/challenge lanes so authorized Claude workers can continue their assigned work.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7f437c2: authorization still applies to every Claude lane, while authorship independence and Grok fallback are now explicitly scoped to review/challenge. A fresh targeted Terra-high read-only re-verification returned clean.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f437c212b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Enforce read-only roles with a real sandbox. `scout` and `reviewer` always use | ||
| the adapter, never a Desktop native child that inherits the parent sandbox. |
There was a problem hiding this comment.
On a normal installation, install.sh still copies every agents/*.toml, including ce-scout.toml and ce-reviewer.toml, into the discoverable native-agent directory. An operator can therefore select these shipped agents directly even though this change says native scout/reviewer children may inherit the parent sandbox and must never be used, bypassing the adapter boundary introduced to prevent writes. Remove these definitions from installation or otherwise prevent native invocation of the adapter-only roles.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6b54a09: native scout/reviewer definitions are deleted; install.sh now copies an explicit mechanic/worker/senior allowlist, fails closed on stale adapter-only definitions without deleting user files, and has fresh/stale installation regression tests. Full local suite: 22 passed. Focused Terra review returned clean. Residual user/manual reintroduction is outside this package’s authority and is documented rather than adding a daemon/runtime control plane.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b54a09866
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| The bundled defaults use the GPT-5.6 family available to the original setup. | ||
| Edit both the skill table and `agents/` TOML files if your Codex account exposes | ||
| different model IDs. | ||
| Edit the skill table and matching write-capable `agents/` TOML files if your | ||
| Codex account exposes different model IDs. |
There was a problem hiding this comment.
When an account exposes different model IDs and uses the now-mandatory adapter path for scout or reviewer, the suggested edits have no effect on the dispatched model: ce-dispatch.sh still hardcodes the primary and fallback model IDs in its role routing. Because this change also removes the native configurations for those roles, following these instructions leaves the adapter requesting unavailable models; include the adapter's model pins (and the installed copy) in the customization steps.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1e4e5f4. The customization guidance now requires updating the skill table, write-capable native TOMLs, and ce-dispatch.sh primary/fallback role pins, including equivalent installed copies because the installer is non-overwriting.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e4e5f4162
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - `plutil -extract KEYPATH FMT FILE` writes back to `FILE` when `-o` is omitted. | ||
| For inspection use `plutil -p` or pass `-o -`. |
There was a problem hiding this comment.
For current plutil, this command is read-only when -o is omitted: plutil --help states that -extract “outputs the value” and that “by default, the output is to stdout unless -o is specified”; an extraction also leaves the input file hash unchanged. This warning therefore reverses the CLI semantics and directs users away from a safe machine-readable inspection command toward plutil -p, whose help explicitly says its format is not stable. Correct the warning so operators are not taught an inaccurate corruption mechanism.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 38864fa and mirrored to the installed runtime skill. Verified on the current host: plutil -help states -extract defaults to stdout unless -o is specified, and a before/after SHA-256 probe confirmed the input plist was unchanged. The operations reference now says exactly that and reserves -p for unstable human-readable output.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
public package (the original local target was 341 lines)
references/operations.mdtopology, and rollover criteria
the adapter's real read-only sandbox; align README with that rule
allows only mechanic/worker/senior and fails closed on stale adapter-only
definitions without deleting user files
targeted re-verification; pure wording/comment/doc-text fixes with no
executable semantics close through deterministic proof-surface checks and
the GitHub bot on the new head
rule
Measured density
Always-loaded
SKILL.md:Public-package section delta:
The installable doctrine plus operations reference is 309 lines; the gain is
that only the 225-line doctrine is loaded by default.
Rule-removal audit
restated the same architect/not-execution-pool boundary.
scar and presentation guidance already lives outside this skill.
persistent workstreams”; normal approval and verification remain global.
existing/user-approved
chief-staterule and the approved-topology rule.rule plus the explicit lost-facts/contradiction/rereading signals.
explicit human gate and brief-bound approval record remain.
references/worker-brief.md; the doctrine retains the sufficiency and stopconditions.
fingerprinting, ignored-evidence details, and the
plutiltrap: moved toreferences/operations.mdand adapter--help.allowlist mechanics: moved to
references/operations.md; authorization,authorship independence, effort routing, and fallback order remain doctrine.
references/operations.md; the blocking pre-wave rule remains in §8.consolidated into one imperative home per rule.
No correctness gate was removed.
Considered cuts retained
a native scout inherited write access and modified seven LaunchAgent plists
retained as the authority boundary
role-integrity boundaries
final head-bound GitHub review: retained because each closes a distinct risk
2026-07-27 audit found 67% of round-2+ findings were reachable but unreported
in round one
context and task/session explosion
Verification
bash -n install.sh skill/chief-engineer/scripts/ce-dispatch.shshellcheck install.sh skill/chief-engineer/scripts/ce-dispatch.shruff check skill/chief-engineer/scripts/ce-token-report.py testsruff format --check skill/chief-engineer/scripts/ce-token-report.py testspython3 -m unittest -v(22 passed)CODEX_HOMEplutil -extracthelp plus before/after input hash probeThe installer sandbox challenge also tested stale/runtime reintroduction.
Package-controlled fresh and partial install paths are closed. This repository
cannot prevent a user or another tool from later adding arbitrary native agent
definitions to Codex runtime; adding a daemon/runtime control plane would
exceed package authority. The installer therefore detects stale exact
definitions, fails before writes, and requires explicit non-destructive
migration.
Anti-drift
This PR does not alter Claude-side role semantics or rollover policy. The
real-sandbox rule is a previously approved Codex-substrate catch-up, and the
wording-only repair exception is already present in the Claude twin, so no
additional Claude-side edit is required.