Skip to content

Make review evidence reusable and installed skills traceable - #28

Merged
nadaverell merged 5 commits into
mainfrom
improve-review-continuity
Oct 12, 2026
Merged

nadaverell merged 5 commits into
mainfrom
improve-review-continuity

Conversation

@nadaverell

@nadaverell nadaverell commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Review follow-ups need to reuse trustworthy evidence without mistaking a clean mechanical fix for a settled feature. Review packets also need to explain the complete change to someone who has not followed the conversation, and stale installed adapters need to be diagnosable.

The shared commands define separate outcomes for a focused follow-up and whole-PR convergence. Follow-ups retain the reviewed revision and check coverage, batch known fixes before expensive QA, and explain which checks need repeating. A settled verdict still requires a clean final-head review of the complete feature, end-to-end verification, required CI, and feedback closure.

Review packets explain the problem, before/after behavior, complete journey, supported scope, architecture, and evidence limits. Updates rebuild that narrative from the final state; logs and review history support it separately. The number of decisions follows the work. Publishing a snapshot is allowed while named, linked CI checks are pending, without implying readiness.

The existing Codex installer prints and saves its source revision, reports adapter command references, and warns about competing user-level adapters. It preserves those copies and does not assert loader precedence. Custom-location installs warn when existing adapters reference the default command bundle. Sources without readable Git metadata report an unavailable revision without failing after copying files.

Validation: nine isolated installer tests pass on macOS, exercising fresh/repeated installs, updates, competing copies, symlink aliases, custom locations, ignored local commands, archives (including inside another checkout), and piped installation including failed-fetch cleanup. Shell syntax, changed skill validation, and diff checks pass. A PR workflow runs the installer tests on macOS and Linux. No visual capture is needed for these instructions and CLI diagnostics.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Preserve review evidence and trace installed skill sources

✨ Enhancement 📝 Documentation 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Distinguish focused fix checks from whole-PR convergence while retaining valid, revision-specific
 evidence.
• Require review packets to explain the complete change and disclose pending verification.
• Make installed skill sources and competing adapters diagnosable, with isolated installer tests in
 CI.
Diagram

graph TD
  U["Review operator"] --> C["Shared commands"] --> E["Revision evidence"] --> P["Review packet"]
  I["Codex installer"] --> A["Installed adapters"] --> C
  W["PR workflow"] --> T["Installer tests"] --> I
Loading
High-Level Assessment

Keep the canonical procedures in shared commands and the Codex skills as thin adapters. Recording the copied source revision and warning about competing adapters improves diagnosis without claiming loader precedence or altering other installations; a separate evidence store or adapter-management system would add complexity without serving this change.

Files changed (14) +376 / -24

Enhancement (6) +64 / -10
SKILL.mdResolve autodev command references from the installed skill +7/-3

Resolve autodev command references from the installed skill

• Replaces fixed home-directory references to the canonical autodev, PR, and fix-PR-loop commands with paths relative to the installed adapter.

codex/skills/autodev/SKILL.md

SKILL.mdMake the research command reference install-relative +5/-1

Make the research command reference install-relative

• Directs the adapter to its sibling installed command bundle rather than the default Codex home, and identifies the source command location in the repository.

codex/skills/competitive-research/SKILL.md

SKILL.mdMake the plan-loop command reference install-relative +5/-1

Make the plan-loop command reference install-relative

• Points the adapter at the command bundle next to its installed directory, including when Codex uses a custom home.

codex/skills/plan-loop/SKILL.md

SKILL.mdMake the product-review command reference install-relative +5/-1

Make the product-review command reference install-relative

• Replaces the fixed default-home command path with a sibling-bundle reference and documents the repository source location.

codex/skills/product-review/SKILL.md

SKILL.mdCarry follow-up and convergence rules into the Codex adapter +11/-3

Carry follow-up and convergence rules into the Codex adapter

• Uses install-relative command references and clarifies that focused fix checks may retain valid evidence but cannot settle a PR. It preserves the requirement for final-head, full-scope review, end-to-end verification, CI, and feedback closure.

codex/skills/review-loop/SKILL.md

install-codex.shReport installed revision, references, and competing adapters +31/-1

Report installed revision, references, and competing adapters

• Saves and prints the source revision, marks modified checkouts, and reports unavailable Git metadata without failing after files are copied. Lists adapter command references, warns about distinct user-level copies without removing them, and guards temporary-checkout cleanup against a substituted symlink.

scripts/install-codex.sh

Tests (1) +172 / -0
test_install_codex.pyCover isolated local, custom, and piped installations +172/-0

Cover isolated local, custom, and piped installations

• Adds seven fixture-based tests for fresh and repeated installs, updates, competing copies and symlink aliases, custom locations, archives without Git metadata, and successful or failed piped fetches. Assertions also verify installed command references, revision reporting, preservation of other skills, and temporary cleanup.

tests/test_install_codex.py

Documentation (6) +117 / -14
README.mdDocument installer provenance and diagnostics +15/-1

Document installer provenance and diagnostics

• Explains the saved source revision, relative command references, competing-adapter warnings, and behavior without Git metadata. Adds the command for running installer tests without touching installed skills.

README.md

SKILL.mdClarify complete-story and pending-CI packet guidance +4/-0

Clarify complete-story and pending-CI packet guidance

• Instructs Codex to write a self-contained final-state narrative and distinguish publishing an evidence snapshot from a settled verdict.

codex/skills/review-packet/SKILL.md

autodev.mdRequire final-head feature review before declaring autodev done +6/-3

Require final-head feature review before declaring autodev done

• Clarifies that contextual fix checks support iteration but do not replace a clean review of the complete feature and its end-to-end behavior on the final head.

plugins/skyhook-skills/commands/autodev.md

fix-findings.mdRetain applicable evidence during finding fixes +12/-1

Retain applicable evidence during finding fixes

• Directs follow-ups to record reviewed revisions, batch known fixes, repeat invalidated checks, and avoid treating a clean fix check as whole-PR settlement.

plugins/skyhook-skills/commands/fix-findings.md

review-loop.mdSeparate focused follow-ups from PR convergence +41/-4

Separate focused follow-ups from PR convergence

• Defines a “follow-up checked” outcome, revision-scoped evidence reuse, and explicit gaps when prior coverage is unknown. Reserves convergence for a clean final-head full-scope review and end-to-end verification, with required CI and feedback closure for a settled PR verdict.

plugins/skyhook-skills/commands/review-loop.md

review-packet.mdMake review packets self-contained evidence snapshots +39/-5

Make review packets self-contained evidence snapshots

• Adds a cold-reader check covering the problem, complete journey, supported scope, design, and verification limits; updates must rebuild the final-state narrative. Allows publication with named, linked pending CI while withholding a merge-ready verdict until completion gates pass.

plugins/skyhook-skills/commands/review-packet.md

Other (1) +23 / -0
test-installer.ymlRun installer tests on Linux and macOS +23/-0

Run installer tests on Linux and macOS

• Adds a path-filtered pull-request workflow that checks shell syntax and runs the isolated Python installer suite on both operating systems.

.github/workflows/test-installer.yml

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Ignored commands get a clean revision ✓ Resolved
Description
The installer copies matching command files regardless of Git ignore rules, but `git status
--porcelain` omits ignored files when deciding whether to label the revision as modified. If a local
source includes an ignored .md command, the installed bundle can contain that file while
.source-revision claims an unmodified commit.
Code

scripts/install-codex.sh[R42-43]

+  if [ -n "$(git -C "$REPO_DIR" status --porcelain -- codex/skills plugins/skyhook-skills/commands scripts/install-codex.sh)" ]; then
+    REVISION="$REVISION (modified checkout)"
Evidence
The command copy uses a filesystem glob, whereas the newly added provenance decision relies on Git
status. Git-ignored files can therefore be installed without appearing in the status result that
determines the marker.

scripts/install-codex.sh[38-49]
tests/test_install_codex.py[75-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Copied, Git-ignored source files can differ from the recorded clean commit.
## Fix Focus Areas
- scripts/install-codex.sh[38-49]
- tests/test_install_codex.py[75-83]
## Recommended Fix
Make the modified-source check cover every file the installer copies, including ignored files, or exclude ignored files from the copied payload. Add a test with an ignored command file and assert that the marker does not claim a clean revision.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unpacked archives record another project's revision ✓ Resolved
Description
git -C "$REPO_DIR" rev-parse --verify HEAD accepts Git metadata from a parent checkout without
verifying that the repository root is $REPO_DIR. When an extracted archive sits inside an
unrelated checkout, the installer prints and writes that checkout’s commit to .source-revision,
often marks it “(modified checkout)” because the archive files are untracked, and skips the
unavailable-revision warning.
Code

scripts/install-codex.sh[R41-43]

+if REVISION="$(git -C "$REPO_DIR" rev-parse --verify HEAD 2>/dev/null)"; then
+  if [ -n "$(git -C "$REPO_DIR" status --porcelain -- codex/skills plugins/skyhook-skills/commands scripts/install-codex.sh)" ]; then
+    REVISION="$REVISION (modified checkout)"
Evidence
Local-source detection requires only codex/skills (scripts/install-codex.sh lines 24-27), while
the revision lookup falls back to unavailable only if rev-parse fails (lines 41-49). Git
searches parent directories, so that lookup succeeds inside an unrelated work tree; the subsequent
status check can report the archive paths as untracked. The archive test
(tests/test_install_codex.py lines 118-130) covers a source outside a Git repository, not one nested
inside another checkout.

scripts/install-codex.sh[41-49]
tests/test_install_codex.py[118-130]
scripts/install-codex.sh[24-27]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The installer can record an enclosing checkout’s revision for an extracted source archive because it trusts `git -C "$REPO_DIR" rev-parse --verify HEAD` without confirming that Git’s repository root is the source directory.
## Fix Focus Areas
- scripts/install-codex.sh[24-27]
- scripts/install-codex.sh[41-49]
- tests/test_install_codex.py[118-130]
## Recommended Fix
Before recording `HEAD` or checking status, verify that `git -C "$REPO_DIR" rev-parse --show-toplevel` resolves to the same directory as `REPO_DIR`, using `pwd -P` or `-ef` for the comparison. If the lookup fails or the directories differ, write the unavailable marker and emit the warning. Add a test that places an extracted archive inside a separate `git init` checkout and asserts that its revision is reported as unavailable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. README points to the wrong revision file location ✓ Resolved
Description
The README says the revision is always saved at
~/.codex/skills/skyhook-skills-commands/.source-revision, but the script writes it to
$CODEX_HOME/skills/skyhook-skills-commands/.source-revision. Users with a custom CODEX_HOME,
which the same paragraph says is supported, will look for the file in the wrong place.
Code

README.md[R132-133]

+each adapter's command reference, saves the revision in
+`~/.codex/skills/skyhook-skills-commands/.source-revision`, and warns about
Evidence
The script sets SHARED_DIR="$CODEX_HOME/skills/skyhook-skills-commands" and writes
.source-revision there. CODEX_HOME defaults to ~/.codex but can be overridden.

scripts/install-codex.sh[19-21]
scripts/install-codex.sh[49-49]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The README gives a fixed `~/.codex/skills/...` path for `.source-revision`, but the installer writes it under `$CODEX_HOME/skills/...`.
## Fix Focus Areas
- README.md[131-136]
## Recommended Fix
Change the path to `$CODEX_HOME/skills/skyhook-skills-commands/.source-revision` and note that `CODEX_HOME` defaults to `~/.codex`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread scripts/install-codex.sh Outdated
Comment thread scripts/install-codex.sh Outdated
Comment thread README.md Outdated
@nadaverell
nadaverell merged commit 4107365 into main Oct 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant