feat(default)!: plan on opus, implement on sonnet, across a handoff by pedromvgomes · Pull Request #86 · pedromvgomes/agentic-toolkit

pedromvgomes · 2026-09-09T06:09:21Z

Planning and implementing want different models, but a session cannot change its own
model and a plan that stays where it was written drags every token of planning context
through every task. This is two sessions with a /clear between them, and a document
that survives it.

/plan-feature runs on opus and reads no code: understanding questions go to
memory-explorer, location questions to Explore pinned to haiku, and a survey needing
synthesis counts as an understanding question however it is phrased. The draft is
reviewed by plan-reviewer on fable, which returns findings and cannot edit — a reviewer
able to edit returns an improved plan, and an improved plan is indistinguishable from an
approved one. Planning ends at a handoff.

After the user's /clear, a SessionStart hook on startup|clear points the session at
the waiting handoff. implement-handoff coordinates from the main session and lands one
task per task-implementer, never two at once, reading each diff and running the
verification command itself rather than acting on the report. review-implementation
then loops panel-code-review until a pass finds no RED or AMBER, and open-pr
documents, stages what the work taught into the memory store, opens the pull request and
puts it through a second, posted review on a different panel.

Trust

A git exclude has no effect on an already-tracked file, so hiding handoff/ through
info/exclude never made a committed handoff impossible. A branch that commits
handoff/x.md would otherwise drive every session started or cleared on that checkout,
through subagents holding Write and Bash. Both entry points now check the one property a
branch cannot forge from inside itself, and refuse a tracked handoff rather than reading
its tasks to judge whether they look reasonable.

task-implementer is denied the subagent tool under both spellings. Nesting is allowed
three layers deep, so "one implementer at a time" had to be a fact about its tool set
rather than a sentence asking it to behave.

Retirements

continuation-session wrote a second handoff format, in a second location, discovered a
second way — a document in $TMPDIR is invisible to the hook, so the two resumed
differently and nothing said which you were holding. Its clean-and-pushed check moves to
write-handoff; its candidates/ staging moves to open-pr, at the one point every path
reaches. ADR 0013 records that decision together with the alternative it overturns.

docs/plans/ goes with it, following the precedent of retiring a plan by first extracting
what it held. Two ADRs shared the number 0005; the local-review one becomes 0012.

Breaking

The default stack now sets the session's default model to sonnet. Settings merge shallow
last-wins with no diagnostic naming the key that was taken, so a consumer whose default
was something else will find it changed on their next sync. It is what makes the split
hold across the /clear. A consumer keeps its own by setting model in a settings
definition of its own, which its entry manifest applies last. Nothing in the flow depends
on the value — this buys cost, not correctness.

continuation-session is removed. A consumer's next render prunes it.

Checks

make check green at every commit. Reviewed in four passes of the loop this branch adds,
converging clean; the one RED it found is the handoff-trust fix above.

Then run on itself to open this pull request, which is what surfaced the last commit:
wrap-session-reviewer was taking its merge-base from the local default branch, stale by
eleven commits in a layout where nobody checks that branch out, and the git instruction
did not say which directory "the root .envrc" meant.

Two ADRs shared the number 0005, so a comment citing "ADR 0005" resolved to either the memory-anchoring decision or the local-review one depending on which file the reader opened first. The two source comments that cite it by number both mean the local-review decision. The memory-anchoring ADR keeps 0005: it is cited by path from CONTEXT.md, the curator prompt and a note under .agents/memory/notes/, and notes have exactly one writer.

A Handoff carries a Predecessor rather than a gate: Approval already lists "gate" as a word to avoid, and the two are different enough that one word for both loses the distinction — a predecessor is ordering any session can read from GitHub, and approval is a control no model may reach.

…will not converge review-implementation drives panel-code-review over the working tree and applies what it finds, up to five passes. It owns the loop alone: the roster, the prompts and the severity ladder stay in the review manifest and the binary, so there is one answer to each rather than a copy that drifts. Four exits, and only one of them is clean. A run that reached no verdict reports zero defects exactly as a clean run does, so the loop checks that first. A pass whose surviving fingerprints match the previous pass has stopped making progress and stops rather than spending the rest of the budget to learn it again. Neither stalled nor capped may open a pull request. Both mean the branch still carries the defects the loop reported. The flow ships as its own stack because the sonnet default it will carry is load-bearing for the implementing half, and settings merge shallow last-wins with no override diagnostic — so putting it in the default stack would change the model of every session in every repo that consumes it.

open-pr updates the documentation for the modules a branch touched, stages what the work taught into the memory store, pushes, opens the pull request under the repo's git rules, and puts it through a posted panel review. Ordering carries the weight: everything that changes the branch happens before the pull request exists. Documentation written after gh pr create lands outside the change it describes, and findings staged afterwards belong to a session that has already ended. The staging retires with continuation-session and lands here rather than in the handoff writer, because a plan of a single slice never writes a second handoff — so a writer holding the step would silently skip it in the commonest case. This is the one point every path reaches. wrap-session keeps its own entry point. It asks which scope to review because a session being wrapped might not be a branch being shipped; here the scope is the branch's commits and there is nothing to ask. panel-code-review declares the fix path it routes to. It sends a pull request's findings to pr-review-resolver, so a stack carrying one without the other offers a route that does not exist.

write-handoff owns the document a fresh session reads to continue the work. It carries a goal, the current state and the next steps; a plan's tasks, slices and predecessor are optional, so a handoff written by hand halfway through something carries none of them and is still one. A format demanding them would serve planning only, and abandon the case that needs a handoff most. The resume location is no longer a decision. The next session continues in the same worktree, and what the handoff records instead is the pull request it may not start before — absent when none is open, which is the ordinary shape when a handoff exists only to buy a clean context. handoff/ is excluded through the common dir's info/exclude rather than the consumer's .gitignore: the folder is a fact about how somebody works, not about the project, and the common dir makes one write cover every worktree of a bare repo. Handing off goes through the hook alone. A continuation prompt to paste as well would be a second way in, and the two would drift.

…hat starts cold The hook fires on startup and clear — the two ways a session begins with no memory of the one that wrote the handoff — and injects an instruction naming the skill that consumes it. It globs handoff/ at depth one, so a consumed handoff under handoff/done/ is invisible without parsing or a state field. It cannot run a skill or change a model; it only says what is waiting. implement-handoff coordinates from the main session. Nesting is allowed three layers deep, so nothing about the platform stops it delegating the role — but its model is the only one in the flow that outlives a single call, and a delegated coordinator would hold every implementer's transcript in the context this exists to keep clear. Work behind a predecessor waits for that pull request to merge. Reading the gate is automatic; moving the branch onto the updated base asks first, because it is the one step that changes where the user is standing. One implementer per task, never two: they share a working tree, and the second reads one the first is still changing. The coordinator reads the diff and runs the verification command itself — the report is a claim, and acting on the claim gives up the only thing the arrangement buys. task-implementer is denied the Agent tool, so one implementer cannot start another. It does not commit; a task that commits itself removes the review it exists to be given. The handoff is consumed only on a run that reached a pull request. Everything about picking work back up depends on that file staying where it is.

…t the handoff /plan-feature raises itself to opus and spends none of it reading. Understanding questions go to memory-explorer, which consults the store before exploring; location questions go to Explore pinned to haiku, whose default is dearer. A survey needing synthesis is an understanding question however it is phrased, so it routes to the explorer rather than to a listing agent. The draft is reviewed by plan-reviewer on fable — a model that did not write it — which returns findings and cannot edit. A reviewer able to edit returns an improved plan, and an improved plan is indistinguishable from an approved one. Planning ends at the handoff. The command's model applies while it runs and no longer, so the session that comes back after the user's /clear is on the settings default, which the flow's own fragment sets to sonnet. That fragment ships here rather than in the default stack because settings merge shallow last-wins with no override diagnostic, and it would otherwise change the model of every session in every repo consuming the default stack.

…cision it held write-handoff is the one handoff writer. continuation-session wrote a second format, in a second location, discovered a second way — a document in $TMPDIR is invisible to the session-start hook, so the two resumed differently and nothing told you which you were holding. Its parts are carried rather than dropped. The clean-and-pushed precondition is write-handoff's section 2. The resume decision it made is gone because the question changed: the worktree is now a constant and what varies is the pull request a handoff waits on. The candidates/ staging moves to open-pr, recorded in ADR 0013 along with the alternative that was rejected before and again now. docs/plans/ goes with it. adapter-cli-update.md describes work whose every slice is merged, verified against the tree rather than its own header. repo-memory.md held the rejection ADR 0013 overturns, which is why that decision is written down before the file carrying it is deleted. The v0.7.0 release notes still name the skill. They record what shipped in that release, and editing them would make the release notes lie. BREAKING CHANGE: the continuation-session skill is removed. A consumer's next render prunes it, since the adapter diffs against .agtk-manifest.json and drops what it previously tracked.

Covers what each stage spends and why, the /clear the user performs by hand, how a plan spanning several pull requests gates each slice on the one before it, and how each skill is used on its own. The stacks bullet named a stack the repo does not have and omitted four it does.

A git exclude has no effect on an already-tracked file, so hiding handoff/ through info/exclude never made a committed handoff impossible — only unlikely. A branch that commits handoff/x.md drove every session started or cleared on that checkout: implement-handoff built its task list from that document, dispatched subagents holding Write, Edit and Bash, and ran the command it named. Both entry points now check the one thing a branch cannot forge from inside itself. The hook advertises only untracked handoffs; implement-handoff refuses a tracked one outright rather than reading its tasks to see whether they look reasonable. A committed handoff is reported rather than skipped quietly, because its presence is worth knowing. The subagent tool is named under both spellings wherever it is granted or denied. agtk validates neither list against a tool vocabulary, so an unrecognised name is ignored rather than rejected — which left task-implementer's guard inert under one spelling and plan-feature unable to delegate under the other. The two pre-existing definitions that dispatch subagents named only the older spelling and had the same latent break. The review loop reads the whole branch once and then only what changed. Pass 1 is the branch's only independent reading, since the coordinator that accepted each task's diff is the model that wrote the acceptance; every later pass has had the rest read one pass ago, and re-reading it costs the first pass again for an answer already given.

The flow moves out of its own stack and into the one consumers actually get: /plan-feature, plan-reviewer, task-implementer, the four skills and the session-start hook. Each skill is listed in its own right rather than hidden behind the command that chains them, because each is usable alone — a handoff written by hand, a review loop over any branch, shipping any finished branch. Folding it into the default stack also removes the seven override diagnostics a consumer of both stacks was getting. Listing shared definitions twice was the price of a stack that worked alone, and there is no second stack now. BREAKING CHANGE: the default stack sets the session's default model to sonnet. Settings merge shallow last-wins with no diagnostic naming the key that was taken, so a consumer whose default was something else will find it changed on their next sync. It is what makes the flow's split hold across the /clear the user performs: a command's model applies while it runs and no longer, so the session that comes back lands on this default. A consumer keeps its own by setting `model` in a settings definition of its own, which its entry manifest applies last. Nothing in the flow depends on the value — /plan-feature raises itself to opus from any default, and the implementing half is correct on any of them, only dearer.

The rationale in the definition described a setting nobody received without asking for it. It ships in the default stack, so the file an operator reads to judge whether adopting that stack changes their session model has to say that it does, name the merge rule that makes it silent, and point at the same rule read backwards as the way out.

Three notes the work paid for: a hook's fail_closed is parsed and documented and reaches no rendered settings file, so every hook is fail-open whatever it declares; render deletes previously-tracked paths it no longer produces, so dropping a definition needs no cleanup step; and two settings definitions setting one key merge shallow last-wins with no diagnostic naming the key that was taken. The sentinel note is re-stamped still-true — it governs agtk's own CLI, not the exit code of a rendered hook's handler, which is the confusion the fail_closed note is next to.

Names the build and check commands, the layout of the catalog and the CLI, and points at CONTEXT.md for the glossary — whose terms are precise and whose _Avoid_ lists are bans, so a wrong synonym reads as a misunderstanding rather than a style choice. Root CLAUDE.md stays untouched: agtk renders it and gitignores it, seeding the import from this file, so hand-writing one would fight the toolkit's own render.

…rc where it is Running the flow on itself surfaced both of these. wrap-session-reviewer took its merge-base from the local default branch, which the instruction offered as the first option. A local default branch is only as current as the last time somebody checked it out, and in a bare-repo worktree layout nobody ever does — so a review of a 32-file branch reported on 155 files, most of them work that landed upstream long before it. The git instruction called .envrc's location "the root" without saying which. It is the directory holding .bare/, not the worktree's parent: a branch name carrying a slash nests the worktree one level deeper per segment, so a relative path finds nothing and any fixed depth is wrong as soon as a branch is named differently. Staged alongside them: a curate run can rule on every candidate and still fail to delete the files, so a non-empty backlog is not evidence that curation has not run.

agtk-code-review

Review by `agtk` — panel `deep-codex`

This review did not reach a verdict: no reviewer answered, so nothing looked at this change

Could not answer (6)

correctness-codex#1: correctness-codex#1 was never started: provider unavailable: codex: exec: "codex": executable file not found in $PATH
correctness-codex#2: correctness-codex#2 was never started: provider unavailable: codex: exec: "codex": executable file not found in $PATH
security-codex#1: security-codex#1 was never started: provider unavailable: codex: exec: "codex": executable file not found in $PATH
security-codex#2: security-codex#2 was never started: provider unavailable: codex: exec: "codex": executable file not found in $PATH
performance-codex#1: performance-codex#1 was never started: provider unavailable: codex: exec: "codex": executable file not found in $PATH
performance-codex#2: performance-codex#2 was never started: provider unavailable: codex: exec: "codex": executable file not found in $PATH

This review is partial: what these would have found is unknown, not absent.

Absent from the reviewed copy (2): a symlink, a submodule or a file too large to read is not code this review looked at.

_{panel deep-codex, 6 runs, 6 could not answer, 1 convention docs read · range main...e6b791a48704041e738378294d5ed9b245f51322 · manifest built-in default}

github-actions · 2026-09-09T06:12:30Z

bulwark

✅ scan — no findings
✅ coverage — go: 81.4% (baseline 81.3%); go patch: 84.7% (160/189 new lines; baseline 81.3%)

📦 Full bulwark output — complete scan and coverage logs.

agtk-code-review

Review by `agtk` — panel `deep-codex`

This review did not reach a verdict: no reviewer answered, so nothing looked at this change

Could not answer (6)

correctness-codex#1: correctness-codex#1 ran and did not answer: codex ended the turn: {
"type": "error",
"error": {
"type": "invalid_request_error",
"code": "invalid_json_schema",
"message": "Invalid schema for response_format 'codex_output_schema': In context=('properties', 'findings', 'items'), 'required' is required to be supplied and to be an array including every key in properties. Missing 'suggestion'.",
"param": "text.format.schema"
…
correctness-codex#2: correctness-codex#2 ran and did not answer: codex ended the turn: {
"type": "error",
"error": {
"type": "invalid_request_error",
"code": "invalid_json_schema",
"message": "Invalid schema for response_format 'codex_output_schema': In context=('properties', 'findings', 'items'), 'required' is required to be supplied and to be an array including every key in properties. Missing 'suggestion'.",
"param": "text.format.schema"
…
security-codex#1: security-codex#1 ran and did not answer: codex ended the turn: {
"type": "error",
"error": {
"type": "invalid_request_error",
"code": "invalid_json_schema",
"message": "Invalid schema for response_format 'codex_output_schema': In context=('properties', 'findings', 'items'), 'required' is required to be supplied and to be an array including every key in properties. Missing 'suggestion'.",
"param": "text.format.schema"
…
security-codex#2: security-codex#2 ran and did not answer: codex ended the turn: {
"type": "error",
"error": {
"type": "invalid_request_error",
"code": "invalid_json_schema",
"message": "Invalid schema for response_format 'codex_output_schema': In context=('properties', 'findings', 'items'), 'required' is required to be supplied and to be an array including every key in properties. Missing 'suggestion'.",
"param": "text.format.schema"
…
performance-codex#1: performance-codex#1 ran and did not answer: codex ended the turn: {
"type": "error",
"error": {
"type": "invalid_request_error",
"code": "invalid_json_schema",
"message": "Invalid schema for response_format 'codex_output_schema': In context=('properties', 'findings', 'items'), 'required' is required to be supplied and to be an array including every key in properties. Missing 'suggestion'.",
"param": "text.format.schema"
…
performance-codex#2: performance-codex#2 ran and did not answer: codex ended the turn: {
"type": "error",
"error": {
"type": "invalid_request_error",
"code": "invalid_json_schema",
"message": "Invalid schema for response_format 'codex_output_schema': In context=('properties', 'findings', 'items'), 'required' is required to be supplied and to be an array including every key in properties. Missing 'suggestion'.",
"param": "text.format.schema"
…

This review is partial: what these would have found is unknown, not absent.

Absent from the reviewed copy (2): a symlink, a submodule or a file too large to read is not code this review looked at.

_{panel deep-codex, 6 runs, 6 could not answer, 1 convention docs read · range main...e6b791a48704041e738378294d5ed9b245f51322 · manifest built-in default}

…dated Step 2 staged new findings and said nothing about the ones already there. A candidate staged before the branch was written was staged against code that has since moved, and a branch that renames or deletes what one names leaves a finding that still reads as true. The curator cannot catch it. It is fed the candidates and the matching slice of the index and not the code, so a candidate naming a deleted symbol reads exactly like one naming a symbol that is still there, and promotion writes it into notes/ where a wrong claim is most expensive. The skill now says to rewrite or delete such a candidate and report which, and that leaving it is a choice rather than the neutral option. notes/ is untouched either way, so ADR 0003 stands.

agtk-code-review

Review by `agtk` — panel `deep-codex`

3 RED, 2 AMBER. 5 posted as inline comments, 0 against a whole file.

What's good

The coordinator checks each implementer’s diff and reruns verification before committing.
The review loop distinguishes a failed review from a clean result and caps repeated attempts.
Render tests check that the flow’s dependencies and handoff template reach consumers.
Memory staging preserves the curator’s exclusive authority to author notes.

Absent from the reviewed copy (2): a symlink, a submodule or a file too large to read is not code this review looked at.

_{panel deep-codex, 12 runs, 1 dropped by a validator, 1 convention docs read · range main...5058f88a2d4f6d5c96910f786aa9f2e6146069c7 · manifest built-in default}

A handoff directs subagents holding Write, Edit and Bash, so the document chooses this session's tasks, file boundaries and commands. What stood between that and a branch author was `git ls-files --error-unmatch`, written twice — as shell in the session-start hook and as prose in implement-handoff. Tracked-ness alone is not the test. Git tracks paths, so committing a `handoff -> real` symlink alongside `real/task.md` leaves the path `handoff/task.md` untracked while its content is entirely branch-authored, and both checks read it as locally written. internal/handoff answers the question once. It Lstats the handoff directory before its entries, refuses a symlink rather than resolving it, measures containment against the resolved directory, and names every refusal — a document withheld in silence and a directory holding nothing produce the same empty list, and only one means there is no work waiting. Only exit 1 from ls-files means untracked; git declining to answer is read as tracked. The cost of that being wrong is a handoff somebody re-creates, against a branch choosing what a session runs. agtk handoff list exposes it so both entry points can stop implementing it.

…d on The session-start hook and implement-handoff each carried their own copy of `git ls-files --error-unmatch`, and both read a committed `handoff ->` symlink as a locally written document. One check, written twice, wrong in both. Both now call `agtk handoff list` and act only on what it returns. The hook fails closed and says so: a missing or failing agtk withholds the handoff rather than offering it unchecked, because a check that did not run is not a check that passed. Refusals are reported instead of discarded — the notice the hook gave for tracked files now covers every reason a document is held back. Outside a repository the hook stays silent. There is no handoff/ there and nothing to check, which is not a failed check. The render tests asserted the mechanism by its literal git command. They now assert that each entry point defers and that the hook fails closed; which handoffs may be acted on is internal/handoff's to decide and its tests to prove.

Checking out the base branch before creating the new one fails wherever that branch already has a worktree of its own: fatal: 'main' is already used by worktree at '.../main' That is the layout this repository itself uses, so the step that starts the next slice could not run here at all — and the branch creation that follows it never ran either. The fast-forward merge of the local base bought nothing the remote ref does not already give. Branching from origin/<base> after the fetch is the updated base, without touching a branch somebody else has checked out.

--auto-fix reports what it found and then fixes it, so the fingerprints a pass reports describe the code as it stood before that pass's own fixes landed. Comparing two such sets could match while the second pass's fixes had already corrected the code, and the loop stopped as failed on the strength of code no reviewer ever read. A stall now needs the fingerprints to repeat and the pass to have changed nothing. The head and the worktree are both checked: a fix left uncommitted moves no head, and reading that as nothing having happened is the same mistake one level down. The alternative — always spending a verification pass after fixes — buys the same guarantee and pays for it on every genuine stall, which is what the loop's own economy argues against.

A handoff carries a goal, the current state and the next steps, and a task-free one is read as a single routine task whose file boundary is what its next steps describe. The template a writer follows had no such section, so following it with Tasks omitted produced a handoff with a goal and nothing to act on — the ad-hoc case the optional half exists to serve. A render test now asserts the section is there and says what it stands in for. Nothing guarded the template's contents before, which is how a required section went missing without a failure anywhere.

Agent.Model's documentation listed inherit, sonnet, opus, haiku and a full model id. `fable` was missing, and plan-reviewer runs on it deliberately — the plan is reviewed by a model that did not write it, which a render test already pins. Nothing validates the field: the adapter writes whatever it is given straight into the rendered frontmatter. A list that reads like an enum and is not one invites the conclusion that a value absent from it is a defect, which is the reading this documentation produced. It now names fable and says the field is unvalidated. SCHEMA.md is generated from the tag and regenerated here. Nothing checks it is current, so a tag edited without it goes stale silently.

ValidateAnchorPath confines an anchor by reading the pattern, so it rejects `..` and absolute paths and cannot see where a path actually resolves. A symlink inside the project spells out as contained and points anywhere, and both hashing paths called os.Stat — which follows it. The blob of a file outside the repository was recorded into a note that is then committed. Both now Lstat, so a symlink is not a regular file and is skipped, which is the treatment a review root already gives one. A glob is the easier way in of the two because the pattern never names the link. Lint follows the same rule. Reporting a symlinked anchor as merely unstamped would send the reader to a command that skips it, and the loop never goes green.

…caped name Three holes in the change that was meant to close one, found by the panel reading it back. A committed submodule at handoff/ was handed over. Its files are real regular files in a real directory and the outer index carries only the gitlink, so ls-files reports every path inside as untracked — exit 1, the status that means "not in the index" — and each branch-authored document read as locally written. Reproduced end to end. A nested repository is now refused, as a review root already refuses a gitlink. Refused paths went to stderr unescaped, and the session-start hook pipes that into a fresh session's context. A refusal is by construction about a branch-authored path, so a filename carrying newlines wrote its own lines into the context of the session that then dispatches subagents holding Write, Edit and Bash. Both streams now quote, as reviewrun does with a path it refuses to write. The containment check could never fire: os.ReadDir yields base names, which carry no separator, so the joined path's parent always equalled the resolved directory. It read as the guarantee while testing nothing. The directory is resolved once instead, and the dead branch is gone rather than left to be trusted by the next reader. In memory, Lstat confined only the anchor's final component. The kernel resolves every directory above it, so `internal/x -> ~/.ssh` with the anchor `internal/x/id_rsa` named a real regular file and hashed a key into a note that is then committed; a glob enumerated the whole linked directory. The path is resolved and required to stay under the project root.

…ested audit called HashFile directly, which is an os.ReadFile and resolves every path component, so an anchor under a linked directory was read from outside the repository and its blob reported. Stamp refusing that anchor leaves the recorded blob empty, so audit re-read and re-reported the outside file on every run rather than once. It is reported as invalid, the kind whose definition already covers a path that escapes the project. The newline test asserted fmt.Sprintf("%q") inside its own body, exercising the standard library rather than the command: reverting either %q to %s left the suite green and silently reopened the path by which a branch-authored filename writes its own lines into a session's startup context. It runs `handoff list` now, and fails when the quoting is removed. The lint hint for a path resolving out of the project had no case — the table that covers the others writes files and cannot make a link — and neither did audit's refusal. Both have one. EvalSymlinks returns "" with its error, so assigning back to dir left the failure naming an empty path instead of the directory that would not resolve.

The containment guard tested existence with Lstat, which succeeds on a symlink whose target is gone — and EvalSymlinks then fails on the same path, so contained() said no and the anchor was reported as resolving outside the project. That is the collapse the guard's own comment forbids, and it is the kind an agent may act on by dropping the anchor. Existence is tested through the link. Following it establishes only that something is there; contained() still decides whether it may be read.

agtk-code-review

Review by `agtk` — panel `deep-codex`

1 RED, 2 AMBER. 2 posted as inline comments, 1 against a whole file.

What's good

Handoff eligibility is centralized in a deterministic command, with explicit refusals for tracked files, symlinks, and nested repositories.
Anchor confinement checks resolve parent directories, with regression coverage for escapes and missing targets.
Stack-render tests verify that the feature flow's definitions and required companions reach consumers.

Ran and reported nothing: security-codex#2, performance-codex#1, performance-codex#2

Absent from the reviewed copy (2): a symlink, a submodule or a file too large to read is not code this review looked at.

_{panel deep-codex, 11 runs, 1 dropped by a validator, 1 convention docs read · range main...f46ec866c1a6c6703941a58afc00a1303ab545c5 · manifest built-in default}

…f directory On a case-insensitive filesystem — macOS by default, which is where this is developed — a committed `Handoff/` answers to the path `handoff/`, while git's index is case-sensitive and holds `Handoff/task.md`. Asking about `handoff/task.md` found no entry, so the document read as untracked and a branch-authored handoff was advertised as local work. Reproduced. The directory's name is established by reading the parent rather than by joining a constant, because os.Lstat answers the filesystem's question and not this one: it resolves `handoff` to `Handoff` and reports success. The spelling is what git's index is keyed by. Audit refuses a leaf symlink as stamping does. Replacing an anchored file with an in-project link to identical content audited as fresh while `anchor` refused to stamp it, so a note read as holding against a file the store will not record. Ordered after the target-exists test, so a dangling link stays missing rather than becoming invalid. FEATURE-FLOW.md documented the resume sequence that checks out the base branch, which fails wherever that branch has its own worktree and contradicts the skill that performs it.

agtk-code-review

Review by `agtk` — panel `deep-codex`

1 AMBER. 1 posted as inline comments, 0 against a whole file.

What's good

Handoff eligibility is centralized in a tested command that refuses tracked files, symlinks and nested repositories.
Anchor checks now resolve containment across the whole path, with regression coverage for symlink escapes and missing files.

Ran and reported nothing: security-codex#1, security-codex#2

Absent from the reviewed copy (2): a symlink, a submodule or a file too large to read is not code this review looked at.

Already on this pull request (1)

Not posted again. A finding is withheld only when the code it quotes is byte-identical to one an existing thread quotes, so a fix that changed the code is a new finding rather than a suppressed one.

definitions/skills/implement-handoff/SKILL.md:69-73 — correctness · a thread quoting this exact code was resolved

_{panel deep-codex, 10 runs, 2 dropped by a validator, 1 already on the pull request, 1 convention docs read · range main...8711b4d78a8b5ea952018b3b2f9c409064357f27 · manifest built-in default}

implement-handoff creates the next slice's branch once the predecessor merges, and reads both the branch and its base off the handoff. The template required neither, so a handoff could carry a predecessor, read as complete, and leave the next session with nothing to create the branch from. Both are required wherever a predecessor is present. A render test asserts it, alongside the one covering next steps: the template is the shape a writer follows, and nothing else checks that what it prescribes is enough to resume.

agtk-code-review

Review by `agtk` — panel `deep-codex`

No findings survived the panel.

Could not answer (5)

correctness-codex#1: correctness-codex#1 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Sep 10th, 2026 2:21 AM.
correctness-codex#2: correctness-codex#2 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Sep 10th, 2026 2:21 AM.
security-codex#1: security-codex#1 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Sep 10th, 2026 2:21 AM.
security-codex#2: security-codex#2 ran and did not answer: I’ll trace the handoff trust checks and anchor confinement, comparing them with the repository’s existing security controls.
performance-codex#1: performance-codex#1 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Sep 10th, 2026 2:21 AM.

This review is partial: what these would have found is unknown, not absent.

Ran and reported nothing: performance-codex#2

Absent from the reviewed copy (2): a symlink, a submodule or a file too large to read is not code this review looked at.

_{panel deep-codex, 7 runs, 5 could not answer, 1 convention docs read · range main...ba3908aa295976cbbfe8585516f49d95f7d7d3d5 · manifest built-in default}

G204 fires on any subprocess taking a variable argument. This one is a fixed argv whose `--` ends the options, so the single variable is read as a pathspec and never as a flag; it is a name os.ReadDir returned from the handoff directory, and no shell is involved. Annotated with the reason, as every other git call in this repository is. gosec reports no issues across the tree.

…ecline Patch coverage fell below the baseline: the json output, the refusal of a handoff path that is not a directory, and the refusal to run outside a repository had no tests, and each is a contract rather than an internal detail. The json form is what a caller parses, so its shape is covered directly: an accepted document and a refusal are present and distinguishable, and both arrays stay arrays when there is nothing, so a caller that iterates need not special-case null.

agtk-code-review

Review by `agtk` — panel `deep-codex`

2 RED. 2 posted as inline comments, 0 against a whole file.

What's good

Handoff trust checks are centralized in one command used by both entry points, with explicit refusal reporting.
Memory stamping and auditing now check resolved path containment and cover symlink escapes with regression tests.
The implementation flow requires the coordinator to inspect each task’s diff and rerun verification before committing.

Ran and reported nothing: correctness-codex#1, correctness-codex#2, security-codex#2, performance-codex#2

Absent from the reviewed copy (2): a symlink, a submodule or a file too large to read is not code this review looked at.

_{panel deep-codex, 8 runs, 1 convention docs read · range main...bb72b1aae833f583ed6ad15497b960a050850358 · manifest built-in default}

Refusing a case-aliased directory left the same alias open one level down. git's index is case-sensitive and a filesystem need not be, so a checkout of `handoff/Task.md` where `task.md` already exists writes the existing file and leaves the on-disk name lowercase. Asking git about the on-disk spelling then found no entry, and branch-authored content was offered as local work. Reproduced through git plumbing, which is what such a checkout leaves behind. git is asked once for what it tracks under the directory, keyed by lower-cased name, rather than once per file for an exact path. Both sides of the comparison fold, and each direction has its own case in the test — dropping either fold fails exactly one of them. Failing to answer refuses every candidate rather than being decided file by file, which is what the removed per-file helper did.

agtk-code-review

Review by `agtk` — panel `deep-codex`

1 RED. 1 posted as inline comments, 0 against a whole file.

What's good

Centralizes handoff eligibility checks in one package shared by the hook and the implementation skill.
Adds regression coverage for symlinks, nested repositories, filename case aliases, and quoted CLI output.
Checks resolved anchor containment before hashing and distinguishes missing anchors from invalid paths.

Ran and reported nothing: performance-codex#1, performance-codex#2

Absent from the reviewed copy (2): a symlink, a submodule or a file too large to read is not code this review looked at.

_{panel deep-codex, 11 runs, 1 convention docs read · range main...2ef1595305fcefbd1ea226a07762ea8193db4ee8 · manifest built-in default}

A pathspec is matched case-sensitively, so `git ls-files -- handoff` returned nothing against an index holding `Handoff/task.md` while the working tree had `handoff/`. The tracked set came back empty and the whole directory read as untracked: branch-authored content offered as local work. Reproduced. The index is read without a pathspec and both components are folded. The test table gains the directory case alongside the two filename ones, and restoring the pathspec fails exactly that row.

agtk-code-review

Review by `agtk` — panel `deep-codex`

This review did not reach a verdict: no reviewer answered, so nothing looked at this change

Could not answer (6)

correctness-codex#1: correctness-codex#1 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at 12:45 PM.
correctness-codex#2: correctness-codex#2 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at 12:45 PM.
security-codex#1: security-codex#1 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at 12:45 PM.
security-codex#2: security-codex#2 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at 12:45 PM.
performance-codex#1: performance-codex#1 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at 12:45 PM.
performance-codex#2: performance-codex#2 ran and did not answer: codex ended the turn: You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at 12:45 PM.

This review is partial: what these would have found is unknown, not absent.

Absent from the reviewed copy (2): a symlink, a submodule or a file too large to read is not code this review looked at.

_{panel deep-codex, 6 runs, 6 could not answer, 1 convention docs read · range main...0b73fc5d34f40fd051cf5e9490c986531bcca402 · manifest built-in default}

…g paths Folding case on both components still left the composed/decomposed alias open: a filesystem may store a name decomposed while the index holds it composed, the two are one file, and the comparison came out unequal — so the branch's content was handed over as locally written. Reproduced with a committed NFC name against an NFD directory entry. Fixing that fold in turn was the wrong move. Comparing paths here means reimplementing git's own comparison, which has more folds than it looks: a case-sensitive index over a filesystem that need not be, a case-sensitive pathspec, and core.precomposeunicode. Each one missed is the same defect, and it had already been found three times. git is asked instead. `git ls-files -o` names what is untracked, and anything it does not name is refused. --exclude-standard is deliberately absent: handoff/ is excluded through info/exclude, so the ignore rules would hide every legitimate handoff — ignored and untracked is the state a handoff lives in. A name git spells differently from the directory listing is refused rather than guessed at. The table covers all four aliases, and each fails only its own row.

… into feature/implement-feature-skill

…ey exist Two faults in delegating the comparison to git. The query had no pathspec, so it walked the whole worktree with ignore rules disabled and enumerated every ignored path in the repository — on every session start, since the hook runs this. It is confined to the handoff subtree. The pathspec is matched against worktree paths, spelled as the directory listing spells them, so it does not reintroduce the index-side folding this delegates. The case tests asserted a folding filesystem's behaviour everywhere and broke CI. Where names do not fold, `Task.md` and `task.md` are two different files and the untracked one is genuinely local: offering it is correct, and there is no alias to close. Each row now names the pair that has to fold and skips when it does not — derived probing got the directory row wrong, because its two filenames are identical and only the directory differs. The composed and decomposed names are written as escapes. As literal characters an editor that normalised this file would make them equal and the row vacuous, which is the failure it exists to catch. Verified on a case-sensitive volume as well as this one: the rows skip there and the suite passes on both.

pedromvgomes added 15 commits September 9, 2026 05:00

agtk-code-review Bot reviewed Sep 9, 2026

View reviewed changes

pedromvgomes mentioned this pull request Sep 9, 2026

fix(code-review): codex reviewers can answer, and a no-verdict review stops suppressing #87

Merged

agtk-code-review Bot reviewed Sep 9, 2026

View reviewed changes

pedromvgomes added 9 commits September 9, 2026 20:30

docs(adr): a handoff is trusted structurally, not by prose

3206bf0

pedromvgomes added 2 commits September 9, 2026 21:08

agtk-code-review Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread internal/handoff/handoff.go Outdated

Comment thread docs/FEATURE-FLOW.md Outdated

agtk-code-review Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread internal/memory/audit.go

agtk-code-review Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread definitions/skills/write-handoff/references/handoff-template.md

agtk-code-review Bot reviewed Sep 9, 2026

View reviewed changes

pedromvgomes added 2 commits September 10, 2026 07:32

agtk-code-review Bot reviewed Sep 10, 2026

View reviewed changes

Comment thread definitions/commands/plan-feature.md

Comment thread internal/handoff/handoff.go

agtk-code-review Bot reviewed Sep 10, 2026

View reviewed changes

Comment thread internal/handoff/handoff.go Outdated

agtk-code-review Bot reviewed Sep 10, 2026

View reviewed changes

pedromvgomes added 4 commits September 10, 2026 11:21

Merge branch 'main' into feature/implement-feature-skill

fce9be7

Merge remote-tracking branch 'origin/feature/implement-feature-skill'…

997080e

… into feature/implement-feature-skill

pedromvgomes merged commit 435010c into main Sep 10, 2026
10 checks passed

pedromvgomes deleted the feature/implement-feature-skill branch September 10, 2026 10:03

Conversation

pedromvgomes commented Sep 9, 2026

Trust

Retirements

Breaking

Checks

Uh oh!

agtk-code-review Bot left a comment

Choose a reason for hiding this comment

Review by agtk — panel deep-codex

Could not answer (6)

Uh oh!

github-actions Bot commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

bulwark

Uh oh!

agtk-code-review Bot left a comment

Choose a reason for hiding this comment

Review by agtk — panel deep-codex

Could not answer (6)

Uh oh!

agtk-code-review Bot left a comment

Choose a reason for hiding this comment

Review by agtk — panel deep-codex

What's good

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

agtk-code-review Bot left a comment

Choose a reason for hiding this comment

Review by agtk — panel deep-codex

What's good

Uh oh!

Uh oh!

Uh oh!

Uh oh!

agtk-code-review Bot left a comment

Choose a reason for hiding this comment

Review by agtk — panel deep-codex

What's good

Already on this pull request (1)

Uh oh!

Uh oh!

agtk-code-review Bot left a comment

Choose a reason for hiding this comment

Review by agtk — panel deep-codex

Could not answer (5)

Uh oh!

agtk-code-review Bot left a comment

Choose a reason for hiding this comment

Review by agtk — panel deep-codex

What's good

Uh oh!

Uh oh!

Uh oh!

agtk-code-review Bot left a comment

Choose a reason for hiding this comment

Review by agtk — panel deep-codex

What's good

Uh oh!

Uh oh!

agtk-code-review Bot left a comment

Choose a reason for hiding this comment

Review by agtk — panel deep-codex

Could not answer (6)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Review by `agtk` — panel `deep-codex`

github-actions Bot commented Sep 9, 2026 •

edited

Loading

Review by `agtk` — panel `deep-codex`

Review by `agtk` — panel `deep-codex`

Review by `agtk` — panel `deep-codex`

Review by `agtk` — panel `deep-codex`

Review by `agtk` — panel `deep-codex`

Review by `agtk` — panel `deep-codex`

Review by `agtk` — panel `deep-codex`

Review by `agtk` — panel `deep-codex`