Skip to content

[codex] Fix background review skill read-before-write - #59175

Closed
doncazper wants to merge 1 commit into
NousResearch:mainfrom
doncazper:codex/skill-read-before-write-review
Closed

[codex] Fix background review skill read-before-write#59175
doncazper wants to merge 1 commit into
NousResearch:mainfrom
doncazper:codex/skill-read-before-write-review

Conversation

@doncazper

Copy link
Copy Markdown
Contributor

Summary

  • Require background review prompts to call skill_view(name='<umbrella>') and read the full SKILL.md before adding support files.
  • Keep the review whitelist unchanged while rendering the exact allowed tool names (memory when enabled, skills_list, skill_view, skill_manage) in prompt/error language.
  • Extend the background-review read-before-write guard so new support-file writes require the umbrella SKILL.md to be loaded first, and make guard errors agent-actionable.

Fixes #58475.

Tests

  • scripts/run_tests.sh tests/run_agent/test_review_prompt_class_first.py tests/run_agent/test_background_review_toolset_restriction.py tests/tools/test_skill_manager_tool.py tests/tools/test_skill_improvements.py -q

Note: the first wrapper attempt found no pytest in the shared fallback venv, so I created a checkout-local .venv with uv sync --extra dev --locked and reran the same command successfully.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) P3 Low — cosmetic, nice to have labels Jul 5, 2026
@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 15, 2026
@doncazper
doncazper force-pushed the codex/skill-read-before-write-review branch from 6bcdde9 to 6011ce9 Compare August 1, 2026 02:37
@doncazper
doncazper force-pushed the codex/skill-read-before-write-review branch from 6011ce9 to da50802 Compare August 1, 2026 23:06

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #75020. That PR explicitly carries this implementation forward, preserves doncazper as the original author, adds the exact corrective skill_view(...) coverage/messages, and has green checks. Keeping both open would leave maintainers with duplicate copies of the same fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Background review agent repeatedly gets stuck — prompt ambiguity, vague tool whitelist, and incomprehensible error message

3 participants