afbab942a6
* docs(skill): simplify again after the code review, and demand a trigger for defensive branches The full loop ran simplify (step 5) before Codex's code review (step 6) and never again, so nothing walked back what the review added. Adversarial review is asked to find what could go wrong, so its output is a list of arguments for more code and it is never asked whether the branch it wants is reachable — accepting objections only ratchets the diff upward. Step 6 now ends by re-running step 5's checks over the hunks the review touched. The other half was earlier than the review. The standing rule already said complexity is bought only by a measurement, but it said it about performance, so a branch added for robustness did not visibly fall under it. It now covers hypothetical hosts as well as hypothetical performance: name the input that reaches a defensive branch and the image it happens on, or delete it and let the case fail visibly. Step 3's attack-your-own-plan list asks the same question before any code exists, which is where it is cheapest to answer. The case study in references/simplify.md is #3013: 25 lines of code at review, 10 merged. A pure-bash fallback written at implement time for images shipping with-contenv but not s6-dumpenv — reasoned from the two binaries living in different s6 packages, never demonstrated on a real image, and defending a case that would have degraded to the pre-fix behaviour anyway — plus the helper function and second reset that existed only to serve it. Deleting the fallback deleted all of it. The review's own objections were correct and cost two tokens on an existing line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skill): fold recent-issue lessons into hassio-addon-workflow Distilled from the calibre-web trusted-ips saga (#3004/#3009/#3010) and the seerr builder revert (#2993/#2997): - traps.md: 'merged is not on master' (builder revert-on-failure); new section on writing into an app's own config (user-editable fields, prefer boot-constant values, dual-stack mapped ranges) - simplify.md: case study — stateful merge machinery (+34 lines, closed) vs trusting the static supervisor range (net -6 lines, shipped) - SKILL.md: post-merge survival check in step 9; a one-line feedback loop in step 10 so follow-up PRs feed lessons back into references/ Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mbqutqkh7yTj4EnhWKFBQx * docs(skill): address Codex review — auth blast radius, fetch before post-merge check - traps.md: trusting a whole range for an auth header is an impersonation trade-off needing the maintainer's explicit call, not a neutral simplification (Codex P1) - SKILL.md: fetch origin master before the post-merge survival check, the tracking ref is stale otherwise (Codex P2) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mbqutqkh7yTj4EnhWKFBQx * docs(skill): fetch before the traps.md post-merge check too (Copilot review) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mbqutqkh7yTj4EnhWKFBQx * docs(skill): name the post-merge check explicitly — tree, not ancestry CodeRabbit's merge-risk note on #3015: the post-merge step said to confirm the commit 'survived', which reads as an ancestry check. A revert leaves the commit in history and undoes its tree, so --contains reports success on exactly the case the step exists to catch. Names the diff check instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mbqutqkh7yTj4EnhWKFBQx * docs(skill): scope the post-merge check to your paths, not the whole tree CodeRabbit was right that traps.md's 'your commit's tree is still what origin/master holds' is invalid on a moving master — unrelated commits break whole-tree equality. Scoped to the touched paths, matching SKILL.md. Its other half, an ancestry check with merge-base --is-ancestor, does not hold here: the repo squash-merges, so a merged PR head is never an ancestor. Verified on #3010 — --is-ancestor reports NOT an ancestor while its fix is live on master, i.e. a false failure on exactly the case the step must pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mbqutqkh7yTj4EnhWKFBQx --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2.8 KiB
Codex review — invocation and prompt guidance
Used for step 3 (plan review) and step 6 (code review), full loop only. Codex is a genuinely different model reading the files itself; on this workload it has repeatedly been worth the minutes. Delegate the invocation to a subagent (see SKILL.md's subagent-delegation note) so its output doesn't land verbatim in your context — have the subagent return only Codex's objections and your assessment of each.
Invocation
Use the CLI, not the MCP tool, for prompts of this size. The codex MCP tool timed out twice
on ~4 KB prompts (2026-08-03); the CLI with the same content succeeded. The MCP tool is still fine
for short questions.
--sandbox read-only lets Codex read files but blocks writes and command execution, and
approval_policy=never means it will not be prompted for permission to run anything either — so
paste every number into the prompt rather than expecting Codex to gather it. - < feeds the
prompt file on stdin. Run it in the background so you are not blocked for the several minutes it
takes (& here, or your harness's background-task mechanism):
codex exec --model gpt-5.6-sol --sandbox read-only --skip-git-repo-check \
-c approval_policy='"never"' - < prompt.md > codex_out.txt 2>&1 &
Writing the prompt
- Plan review (step 3): include the files to read, your measurements with numbers, the proposed changes, and explicit instructions to challenge you. Ask direct questions ("is this really add-on-fixable?", "give the precise flag set") rather than "review this".
- Code review (step 6): point it at
git diff origin/master...HEADplus the reasoning behind each hunk. Ask specifically what breaks: upgrade paths, hosts unlike this one, users who configured things by hand. Ask directly whether a simpler mechanism would achieve the same thing — an outside reader spots one-level-too-deep framing far more easily than the person who just built it. - Codex's sandbox often cannot run local commands and falls back to reading GitHub, so paste the evidence in rather than assuming it will find it.
Codex agrees with confident premises. It has confirmed a wrong conclusion stated too confidently, and separately caught a genuine methodology error in the same review. Treat its confirmations with the same scepticism as its objections — especially about the build.
Its objections ratchet complexity upward. An adversarial reviewer is asked to find what could go wrong, so its output is a list of arguments for more code; it is never asked whether the branch it wants is reachable. Separate "this is wrong" from "this is undefended" before you write anything: the first is a bug and you fix it, the second is a claim about some host, and it needs the same demonstration you would demand of a measurement. That is what step 6's second simplify pass is for.