Skip to content

Make the declared Python venv and add jpdev codex review (#23, #13) - #25

Merged
kikashy merged 2 commits into
mainfrom
rows-23-13
Oct 22, 2026
Merged

kikashy merged 2 commits into
mainfrom
rows-23-13

Conversation

@kikashy

@kikashy kikashy commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Closes #23. Closes #13.

What it adds

Checks

The focused tests passed locally (343). The whole check list could not complete on the building machine, which has no shellcheck; CI runs it.

Not in this PR

The other v0.6.0 rows (#12, #16, #8) and the re-vendor of adopting repositories.

A Codex session wrote the change; the coordinator committed the completed tree after the builder stopped at the missing local shellcheck.

setup and check create the venv dev.json declares, resolving the interpreter
from the PATH or a pyenv version directory, and run every Python step with it
(#23). jpdev codex review runs a read-only Codex review from a brief, writes the
record to a file and prints the reviewed SHA; brief-template --review prints the
review skeleton (#13).

Signed-off-by: kikashy <kikashy@hotmail.com>
@kikashy

kikashy commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Review round 1 (OpenAI gpt-5.6-sol, effort medium, 2026-10-22), reviewed SHA 9d51811

Clean-room clone at the head, probes under .review/; CI green before the round. The report verbatim:

HIGH — templates/check.sh:25: venv accepts absolute paths, .., and the repository root. A manifest using ../outside successfully created and executed an environment outside the clone. Validate it as a non-root relative child path in the schema and both setup paths before creating anything.

HIGH — bin/jpdev:456: default workspace-write explicitly enables network access. The prompt cannot guarantee “never pushes or posts”; the reviewer can use available credentials to mutate remote state without leaving a local diff. Remove the network override or narrow the documented guarantee and provide an enforced offline review path.

MEDIUM — tests/run.sh:172: tests exercise only python3.X on PATH, not the required pyenv fallback or newest-version selection. Add pyenv-only fixtures and ensure removing/reordering that fallback fails.

MEDIUM — tests/run.sh:904: the “no git writes” test checks final repository state but does not log Git/gh calls. A prohibited push/post attempt that leaves the clone unchanged is not caught. Use stand-ins and assert the exact read-only command set and zero posting calls.

LOW — templates/codex-review-brief.md:19: the skeleton prints the literal merge: yes | after fixes, rather than either valid result line. List merge: yes and merge: after fixes separately and update its test.

Verified

  • Both runner copies selected a pyenv-only Python, created the venv, prepended its bin to PATH, and produced identical missing-version refusals.
  • codex review closed stdin, passed model/effort/sandbox flags, wrote the final output, printed the SHA, refused a tracked edit, and issued only read-only Git calls in the stand-in probe.
  • Mutation probes caught broken PATH ordering and inherited stdin.
  • templates/check.sh and scripts/dev/check.sh are byte-identical; workflow/actionlint files are unchanged.
  • The single tests/run.sh attempt was externally stopped after 30 seconds with 62 passing assertions and no observed failure; it did not complete. shellcheck was unavailable.

merge: after fixes
reviewed SHA: 9d51811

Disposition (the coordinator's)

# Finding Severity Disposition
1 venv in dev.json accepts absolute paths, .. and the repository root HIGH Fix in this pull request: the value is validated as a relative child path of the clone, not the root, before anything is created, in the schema and both setup paths; a test per rejected form
2 codex review enables network access under its default workspace-write sandbox, so "never pushes or posts" is not enforced HIGH Fix in this pull request: no network by default (the sandbox's own default), a --network flag the caller must pass, and the docs say what the guarantee then is
3 Tests cover only a python3.X on the PATH, not the pyenv fallback or newest-version selection MEDIUM Fixed in the same push: pyenv-only fixtures; removing or reordering the fallback fails
4 The no-git-writes test checks final state, not the commands run MEDIUM Fixed in the same push: stand-ins log every git and gh call; the test asserts the read-only set and zero posting calls
5 The review skeleton prints the literal merge: yes | after fixes LOW Fixed in the same push

The same reviewer re-reads the fixed head. Same vendor as the builder, not a cross-vendor claim.

Refuse virtual-environment paths that can escape or replace the clone root.
Keep Codex review network access disabled unless the caller opts in, and
record the network choice before the run. Cover pyenv ordering, review
subprocesses, and the two valid review verdicts.

Co-Authored-By: Codex <noreply@openai.com>
Signed-off-by: kikashy <kikashy@hotmail.com>
@kikashy

kikashy commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Re-read after the fix (OpenAI gpt-5.6-sol, effort medium, 2026-10-23), reviewed SHA 12544c1

The same reviewer, clean-room clone at the fixed head, probes under .review/; CI green. The report verbatim:

Findings: none.

Verified:

  • All rejected venv forms are covered; validation precedes creation, and both runners are byte-identical.
  • Review network is disabled by default; only logged --network enables it.
  • Pyenv-only coverage selects the newest match and fails without the fallback.
  • Git/gh stand-ins enforce the exact read-only command set and catch push attempts.
  • Skeleton result lines are separate.
  • Focused tests: 373 passed, 0 failed.

merge: yes
12544c1

Disposition: all five round-1 findings are fixed; no further round. Merged on the standing rule. Same vendor as the builder, not a cross-vendor claim.

@kikashy
kikashy merged commit 20a1b47 into main Oct 22, 2026
3 checks passed
@kikashy
kikashy deleted the rows-23-13 branch October 22, 2026 18:47
@github-actions

Copy link
Copy Markdown

🎉 Thank you for your contribution to Judgment Pack!

If you’d like another task, choose a path that interests you:

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

Labels

None yet

Projects

None yet

1 participant