harden code review skill for async state and default-resolution bugs#8740
Conversation
Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
![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: 1c59c410de
ℹ️ 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".
`just check-everything` runs `cargo fmt --all` in write mode, which can introduce unstaged formatter diffs into the reviewer's working tree before they collect the file list. The skill is meant to review the author's changes, not formatter output, so the baseline must be check-only.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82bdb69f32
ℹ️ 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".
Without the subshell, `cd ui/desktop && pnpm run lint:check` leaves the working directory in `ui/desktop`, so the next command in the block (`./scripts/check-openapi-schema.sh`) fails to resolve from the repo root when a reviewer runs the block line by line.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 045c59ef56
ℹ️ 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".
* main: fix: preprompt would show after loading session (#8744) commands to acp+ migration: extensions management (#8733) feat: desktop notification when goose finishes a task (#8647) harden code review skill for async state and default-resolution bugs (#8740) Feature/at agent mention (#8571) fix: removed hardcoded dependency of goose-acp-macro (#8753) perf: split agent setup into staged phases to reduce startup blocking (#8746) Add /skills command (#8600) Replace deprecated Claude ACP package links (#8625)
* main: (34 commits) fix(goose-server): cache TLS cert to disk to avoid slow startup on first launch (#8174) feat: add Exa AI-powered search tool (#8487) fix: preprompt would show after loading session (#8744) commands to acp+ migration: extensions management (#8733) feat: desktop notification when goose finishes a task (#8647) harden code review skill for async state and default-resolution bugs (#8740) Feature/at agent mention (#8571) fix: removed hardcoded dependency of goose-acp-macro (#8753) perf: split agent setup into staged phases to reduce startup blocking (#8746) Add /skills command (#8600) Replace deprecated Claude ACP package links (#8625) removed the specific code owner for documentation change (#8749) fix(providers): handle missing delta field in streaming chunks (#8700) refactor(providers): extract http_status module and rename handle_status_openai_compat (#8620) fix(providers/openai): accept streaming chunks with both reasoning fields (#8715) feat: associate threads with projects (#8745) upgrade goose sdk and tui to be compatible with the latest agentclientprotocol/sdk package (#8667) feat: extend goose2 context window ux with auto-compaction (#8721) improve goose2 agent management flows (#8737) alexhancock/tui-improvements (#8736) ...
2 participants
Overview
Category: improvement
User Impact: The code review skill now catches async state, default-resolution, and UI/backend drift bugs that would otherwise slip past review.
Problem: Recent bugs (provider/model selections drifting from the backend, sticky defaults overriding explicit user choices, stale dependent state lingering after a parent change) were not being flagged by the code review skill because its checklist had no language for async state, persistence, or fallback-authority issues. The skill also referenced
just ci, which is not the correct gate command in this repo.Solution: Added a new "Async State, Defaults & Persistence" checklist section, an explicit end-to-end tracing step for stateful UI/async flow changes, and a third self-check pass focused on UI/persisted-state/backend disagreement after failures or handoffs. Replaced the outdated
just cireferences withjust check-everythingand softened the language so the skill defers to whatever the repo's actual pre-push/CI gate is.File changes
.agents/skills/code-review/SKILL.md
Added an "Async State, Defaults & Persistence" checklist covering optimistic updates without rollback, requested-vs-fallback authority, dependent state invalidation, persisted preference validation, fallback compatibility across providers, best-effort lookup degradation, and draft/Home/handoff session paths. Added an end-to-end tracing step (selection -> local state -> persistence -> backend prepare/set -> failure path) for stateful UI changes, plus a third self-check pass that explicitly asks whether UI, persisted state, and backend can ever disagree after a failure, fallback, or handoff. Updated the CI gate command from
just citojust check-everythingand softened wording so the skill defers to whatever pre-push/CI gate the repo actually uses.Reproduction Steps
.agents/skills/code-review/SKILL.mdand confirm the new "Async State, Defaults & Persistence" section appears in the Review Checklist.just cino longer appears anywhere in the file and the gate command is nowjust check-everything, with the surrounding language deferring to the repo's stronger pre-push/CI gate.