Skip to main content

review-work

Post-implementation gate review: run manual QA on the real surface yourself, then launch ONE gate reviewer (never a panel) to audit goal, constraints, code quality, security, missed context, and QA evidence. Use before a PR handoff or when the user explicitly asks to review completed work.

معلومات المصدر

المستودع
code-yeongyu/lazycodex
آخر نشاط في المصدر
٢٣ سبتمبر ٢٠٢٦ في ١١:١٠
لغة SKILL.md المكتشفة
الإنجليزية
النجوم
٣٬٦٩٧
التفرعات
٢٣٢

خيارات التثبيت

يُحدَّد Prompt الذي يراجع المصدر أولًا بشكل افتراضي. يمكنك التبديل إلى أمر مباشر أو تنزيل نسخة محلية.

مراجعة ملفات المصدر

اقرأ SKILL.md وأي ملفات مرافقة يعرضها SkillsMP قبل أن تقرر التثبيت.

مستكشف الملفات
2 ملفات

عرض SKILL.md

SKILL.md
تعليمات المصدر · معاينة للقراءة فقط
name
review-work
description
Post-implementation gate review: run manual QA on the real surface yourself, then launch ONE gate reviewer (never a panel) to audit goal, constraints, code quality, security, missed context, and QA evidence. Use before a PR handoff or when the user explicitly asks to review completed work.
## Codex Harness Tool Compatibility This skill may include examples copied from the OpenCode harness. In Codex, do not call OpenCode-only tools such as `call_omo_agent(...)`, `task(...)`, `background_output(...)`, or `team_*(...)` literally. Translate those examples to Codex native tools: | OpenCode example | Codex tool to use | | --- | --- | | `call_omo_agent(subagent_type="explore", ...)` | `multi_agent_v1.spawn_agent({"message":"TASK: act as an explorer. ...","agent_type":"explorer","fork_context":false})` | | `call_omo_agent(subagent_type="librarian", ...)` | `multi_agent_v1.spawn_agent({"message":"TASK: act as a librarian. ...","agent_type":"librarian","fork_context":false})` | | `task(subagent_type="plan", ...)` | `multi_agent_v1.spawn_agent({"message":"TASK: act as a planning agent. ...","agent_type":"plan","fork_context":false})` | | `task(subagent_type="oracle", ...)` for final verification | By default, record a self-review in the notepad: re-read the diff, run diagnostics, and capture evidence for every acceptance criterion. Only when the user demanded strict, rigorous, or high-accuracy review, use `multi_agent_v1.spawn_agent({"message":"TASK: act as a rigorous reviewer. ...","agent_type":"lazycodex-gate-reviewer","fork_context":false})`; include `fork_context: false`. | | `task(category="...", ...)` for implementation or QA | `multi_agent_v1.spawn_agent({"message":"TASK: act as an implementation or QA worker. ...","agent_type":"lazycodex-worker-medium","fork_context":false})` | | `background_output(task_id="...")` | `multi_agent_v1.wait_agent(...)` for mailbox signals | | `team_*(...)` | Use Codex native subagents via `multi_agent_v1.spawn_agent` and `multi_agent_v1.wait_agent`; use `multi_agent_v1.send_input` and `multi_agent_v1.close_agent` only when exposed in the active tools list | Role-specific behavior must be described in a self-contained `message`. Use `fork_context: false` to start the child with only the initial prompt (no parent history); use `fork_context: true` only when full parent history is truly required. Include any required conversation context, files, diffs, constraints, and requested skill names directly in the spawned agent's `message`. OMO installs these selectable agent roles into `~/.codex/agents/`: `explorer`, `librarian`, `plan`, `momus`, `metis`, `lazycodex-code-reviewer`, `lazycodex-qa-executor`, and `lazycodex-gate-reviewer` - pass the matching name as `agent_type` so the child gets that role's model and instructions. Inspect the actual spawn schema: whenever `agent_type` is exposed, EVERY spawn MUST select an exact LazyCodex role, on V1 or V2. Implementation difficulty selects `lazycodex-worker-low`, `lazycodex-worker-medium`, or `lazycodex-worker-high`; clone QA can select `lazycodex-clone-fidelity-reviewer`. Never select generic `worker` or `default`. If a code block below conflicts with this section, this section wins. Codex exposes ONE of two subagent tool surfaces per session; check your own tool list and route accordingly. If `multi_agent_v1.*` tools exist, use the table above as written. If instead a flat `spawn_agent` with a required `task_name` exists (`multi_agent_v2`), rewrite every `multi_agent_v1.*` example: `multi_agent_v1.spawn_agent({...,"fork_context":false})` becomes `spawn_agent({"task_name":"<lowercase_digits_underscores>","message":...,"agent_type":...,"fork_turns":"none"})` (`"all"` only when full parent history is truly required); `send_input` becomes `send_message`; do not call `close_agent`/`resume_agent` (finished agents end on their own; `followup_task` re-tasks one, `interrupt_agent` stops one); `wait_agent` takes only `timeout_ms` and returns on any child mailbox activity. Do not infer role support from V1/V2 or a model version. Legacy-schema exception: only if the actual schema lacks `agent_type`, omit that unsupported field and carry the complete role instructions in `message`, with history explicitly disabled. This cannot select a specialized TOML; an installed managed default supplies the medium worker for unnamed non-forks. The guard has no schema metadata and rejects unnamed calls, so report incompatible routing instead of retrying generically. Every deliberate full-history fork must still name its role: Codex skips role application on unnamed full-history forks, an upstream gap no LazyCodex default can repair. If a code block below conflicts with this section, this section wins. `fork_context` is rejected on `multi_agent_v2` (`fork_context is not supported in MultiAgentV2; use fork_turns instead`). When translating `load_skills=[...]`, include the requested skill names in the spawned agent's `message`. If a code block below conflicts with this section, this section wins. Omit optional keys you do not set. Never send `items: []`, `message: ""`, `model: ""`, `reasoning_effort: ""`, or `service_tier: ""` — Codex rejects them (`Items can't be empty`, `reasoning_effort must not be empty`). For work likely to exceed one wait cycle, require the child to send `WORKING: <task> - <current phase>` before long passes and `BLOCKED: <reason>` only when progress stops. A `multi_agent_v1.wait_agent` timeout only means no new mailbox update arrived; back off between waits (double the timeout up to ~5 minutes) instead of spinning short cycles. Treat a running child as alive. Fallback only when the child is completed without the deliverable, ack-only after followup, explicitly `BLOCKED:`, or no longer running. ## Codex Subagent Reliability Every `multi_agent_v1.spawn_agent` message must be self-contained. Start with `TASK: <imperative assignment>`, then name `DELIVERABLE`, `SCOPE`, and `VERIFY`. State that it is an executable assignment, not a context handoff. Role or specialty instructions belong inside `message`. Use `fork_context: false` unless full history is truly required; paste only the review context that worker needs. Review lanes are leaf agents: a lane does its own reading, running, and judging inline and never spawns sub-reviewers of its own. Reviewers are one-shot: a lane ends at its verdict; a re-review after fixes is a fresh spawn scoped to the delta plus current evidence, never a `followup_task` to a long-lived reviewer carrying stale context. Plan and reviewer agents may run for a long time; spawn them in the background and keep doing independent root work. Between `multi_agent_v1.wait_agent` calls, back off — double the timeout up to ~5 minutes — instead of spinning short cycles. Treat child status as a progress signal, not a timeout counter. For work likely to exceed one wait cycle, require the child to send `WORKING: <task> - <current phase>` before long reading, testing, or review passes, and `BLOCKED: <reason>` only when it cannot progress. While any child is active, keep the parent visibly alive with active subagent count, agent names, latest `WORKING:` phase, and whether the parent is waiting for mailbox updates. Track spawned agent names locally. Use `multi_agent_v1.wait_agent` for mailbox signals, not proof of completion. A timeout only means no new mailbox update arrived. Treat a running child as alive. Fallback only when the child is completed without the deliverable, ack-only after followup, explicitly `BLOCKED:`, or no longer running. Then mark that review lane `INCONCLUSIVE`, do not count it as PASS or approval, close if safe, and respawn a smaller `fork_context: false` reviewer with the missing deliverable. Preserve completed lane results immediately. If the retry budget is exhausted, keep the lane `INCONCLUSIVE` and still emit a final aggregate result. # Review Work - Gate Review Orchestrator Review completed implementation work through exactly two lanes: your own hands-on manual QA on the real surface, and ONE gate reviewer sub-agent that audits the whole change set against the goal, the constraints, and your QA evidence. The review passes only when the QA matrix has no failing row AND the gate reviewer returns APPROVE. On Codex, use `review-work` only when the user asks for a review or demands strict, rigorous, or high-accuracy verification, not automatically for a PR or completion claim. Your own manual QA and the main session's self-review are the default. Spawn ONE `lazycodex-gate-reviewer` only for an explicit demand for strict review; otherwise run the review checklist inline and record the main session's verdict instead of spawning or waiting for a reviewer. These Codex rules override the mandatory-reviewer instructions in the shared skill. When `review-work` is used as a final implementation, PR, or `$ulw-execute` gate, the selected review is blocking. A timeout, missing deliverable, ack-only response, explicit `BLOCKED:`, or inconclusive lane is not a pass. Treat that lane as failed, investigate the underlying uncertainty with the `debugging` skill when runtime behavior may be wrong, fix with evidence, and rerun the affected lane before claiming completion, creating or handing off a PR, or merging. After each lane reaches PASS, immediately append a durable task-evidence record to the active ledger with the lane name, exact full commit SHA, PASS verdict, and report artifact/source. Before reusing coverage after continuation or compaction, re-read that record and require the exact lane/SHA pair. Memory, chat history, or an unstamped report is not coverage; a new commit requires fresh applicable lane records. A rejecting lane must name its blockers inline in its final message — each blocker cites the violated goal criterion or requirement plus an evidence pointer. A bare REJECT/FAIL token without findings is not a verdict; treat it as an inconclusive lane (one bounded respawn, then record it inconclusive with that reason). When reviewing a PR or branch, collect diff, file contents, and verification results from a dedicated review worktree attached to that branch. Never checkout, test, or edit the review branch in the main worktree. Review evidence must be safe to share. Redact or mask secrets and sensitive user data before including evidence in logs, PR bodies, or handoffs. Never include raw tokens, credentials, auth headers, cookies, API keys, env dumps, private logs, or PII; summarize with lengths, hashes, and short non-sensitive prefixes when identity is needed. One reviewer, not a panel. A single gate reviewer holding the full context (goal, diff, history, QA evidence) catches what a fan-out of narrow reviewers misses between their seams, and it costs one agent instead of five. Never add review lanes; widen the gate reviewer's checklist instead. | Lane | Who runs it | Question it answers | |------|-------------|---------------------| | Manual QA | You, the orchestrator, on the real surface | Does it actually work? | | Gate review | One gate reviewer sub-agent (`oracle` on OpenCode; the surface's gate-reviewer agent elsewhere) | Did we build what was asked - correctly, safely, well, and without missing context? | --- ## Phase 0: Gather Review Context Before running anything, collect these inputs. Extract from conversation history first - the user's original request, constraints discussed, and decisions made are usually already in the thread. Only ask if truly missing. <required_inputs> - **GOAL**: The original objective. What was the user trying to achieve? Pull from the initial request in this conversation. - **CONSTRAINTS**: Rules, requirements, or limitations. Tech stack restrictions, performance targets, API contracts, design patterns to follow, backward compatibility needs. - **BACKGROUND**: Why this work was needed. Business context, user stories, related systems, prior decisions that informed the approach. - **CHANGED_FILES**: Auto-collect via `git diff --name-only HEAD~1` or against the appropriate base (branch point, specific commit). - **DIFF**: Auto-collect via `git diff HEAD~1` or against the appropriate base. - **FILE_CONTENTS**: The full content of each changed file plus the neighboring files that show the established patterns. Required verbatim when the reviewer cannot read files (`oracle`); when your surface's gate reviewer can read files and run commands, pass the paths and the diff instead of pasting everything. - **RUN_COMMAND**: How to start/run the application. Check `package.json` scripts, `Makefile`, `docker-compose.yml`, or ask the user. - **CONTEXT_MINING**: What the history and the trackers say about this area (collected below). </required_inputs> Review PRs and branches from a dedicated review worktree only: create or attach one with `git worktree add <path> <branch>` before collecting changed files, diff, file contents, or running checks, then immediately lock it with `git worktree lock <path> --reason "review:<pr-or-branch>"`. The main worktree is read-only context; never checkout, test, or edit the review branch there. **Auto-collection sequence:** ```bash # 1. Get changed files git diff --name-only HEAD~1 # or: git diff --name-only main...HEAD # 2. Get diff git diff HEAD~1 # or: git diff main...HEAD # 3. Detect run command # Check package.json -> "scripts.dev" or "scripts.start" # Check Makefile -> default target # Check docker-compose.yml -> services # 4. Mine the context the implementation may have missed (keep the output short) git log --oneline -20 -- <each changed file> # recent changes and their reasons git log --all --oneline --grep="<keywords from goal>" # related commits, reverts gh issue list --search "<keywords>" --state all # related issues (when gh is available) gh pr list --search "<keywords>" --state all # related PRs and their review comments rg -n "TODO|FIXME|HACK" <changed files> # warnings left by previous authors # plus: files that import the changed modules, tests touching the same paths, # docs and config that reference the changed behavior ``` Record CONTEXT_MINING as a short list: source -> finding -> why it matters for this change. Slack, Notion, and Discord searches belong here too when those tools exist. For GOAL, CONSTRAINTS, BACKGROUND - review the full conversation history. The user's original message almost always contains the goal. Constraints often emerge during discussion. If anything critical is ambiguous, ask ONE focused question - not a checklist. --- ## Phase 1: Manual QA (you run it) You are the QA lane. Do not delegate hands-on QA to a sub-agent: the orchestrator owns the real-surface proof, exactly as the ulw-loop final gate records `manualQa` under the main session. 1. **Reuse first.** If this session already captured real-surface evidence for the FINAL tree (an ultrawork or ulw-loop evidence directory, a `visual-qa` verdict on this same build), consume it as QA rows instead of re-running. A fix committed after a capture stales that capture: re-run the rows it covered. 2. **Pick the channel that faithfully exercises the surface** and capture the artifact: - HTTP: `curl -i` (or an API request context) - status line, headers, body. - CLI / TUI: a real pty - drive the command and keep the transcript; for color or layout evidence render through a browser-based terminal, never a `tmux capture-pane` dump. - Web: omowright from js eval (staged in the `browser` skill) — the owned engine (`connectPipe` on a task-owned profile, or `connectCloakProfile` for bot-scored targets) for unauthenticated pages, the attached engine (`connectBrowserSkill()` in the user's signed-in browser) when the page needs their login; never a clone of or a launch against the live profile. Capture action log plus screenshot. - Desktop / GUI: OS-level automation against the running app - action log plus screenshot. - Library / SDK: a script that imports and exercises the public API - transcript. - Data-shaped work (migrations, configs, generated files): the resulting artifact itself, diffed or dumped. 3. **Cover at least**: the happy path the goal names, the riskiest edge (empty, boundary, malformed, or concurrent input), and one regression on adjacent behavior the change could have broken. Add a row for every stated success criterion. 4. **Build the QA matrix** - one row per scenario: | # | Scenario | Exact command / action | Expected | Observed | Verdict | Artifact | |---|----------|------------------------|----------|----------|---------|----------| A row without an artifact path is not PASS. If the application cannot even start, that is an immediate FAIL. Any FAIL ends the review here: report **REVIEW FAILED** with the failing rows and skip the gate reviewer - reviewing code that does not work wastes the reviewer. Fix first, then re-enter at Phase 0 with the delta. --- ## Phase 2: Launch the Gate Reviewer (one agent) Launch exactly one reviewer, in the background, then keep doing independent root work (teardown prep, report scaffolding) while it runs. `oracle` cannot read files or run commands: it receives everything inline (DIFF + FILE_CONTENTS + CONTEXT_MINING + the QA matrix). If your surface's gate reviewer has read and shell tools, still paste the diff and the QA matrix, and hand it file paths instead of full contents. ``` task( subagent_type="oracle", run_in_background=true, load_skills=[], description="Gate-review the completed work against goal, constraints, and QA evidence", prompt=""" <review_type>GATE REVIEW</review_type> <original_goal> {GOAL - paste the user's original request and any clarifications} </original_goal> <constraints> {CONSTRAINTS - every rule, requirement, or limitation discussed} </constraints> <background> {BACKGROUND - why this work was needed, broader context} </background> <changed_files> {CHANGED_FILES - list of modified file paths} </changed_files> <file_contents> {FILE_CONTENTS - full content of every changed file plus neighboring files that show existing patterns; or the paths, when the reviewer can read files} </file_contents> <diff> {DIFF - the actual git diff} </diff> <context_mining> {CONTEXT_MINING - git history, related issues and PRs, docs and config that reference the changed behavior, warnings from previous authors} </context_mining> <manual_qa_matrix> {QA MATRIX - every row with its artifact path} </manual_qa_matrix> Role: final gate reviewer. You do not implement fixes. Assume every success claim is unverified until you reproduce it from the artifacts: executors can be wrong, tests can be too narrow, success prose can be misleading. Review from the user's perspective first: what did they originally want, what result did they expect to receive, and does the shipped change actually satisfy that outcome? Then work the checklist. Be obsessively thorough - the point of this review is to catch what the implementer missed. REVIEW CHECKLIST:
عرض على GitHub
ملف SKILL.md هذا كبير جدا، لذلك يعرض SkillsMP القسم الاول فقط هنا. عرض على GitHub