chore(just): adopt shared phenotype.just recipe library#49
Conversation
Replaces the hand-rolled 29-line boilerplate with `import "phenotype.just"`. The 9 most common recipes (default, build, test, lint, fmt, audit, unused, ci, docs) are now defined once in just/phenotype.just and parameterized over the build system. This repo's justfile is now a thin shell that imports the library and adds stack-specific recipes. Library features: - Auto-detects build system (cargo/npm/pnpm/yarn/bun/uv/poetry/go/mix) - 14 recipes total: 9 originals + dev, clean, typecheck, info - Optional tools (cargo-deny, cargo-audit, cargo-machete, cargo-watch, ruff, mypy, govulncheck) gracefully skip if not installed Refs: NEXT-STEPS-DAG.md Tier 0+1, FLEET_100TASK_DAG_V4.md V12 EXTENSION
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
codeant-ai
Bot
added
the
size:L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a shared recipe library, just/phenotype.just, which auto-detects the project's build system and runs standard development tasks, simplifying the main justfile into a thin wrapper. The review feedback identifies a critical issue across several recipes (_audit, _unused, _typecheck, _docs, and _dev) where the shell pattern A && B || C is used to check for tool availability. This pattern accidentally masks actual tool failures (such as security vulnerabilities or type-check errors) by executing the fallback echo statement and exiting with a success status code. It is recommended to use explicit if statements to verify command existence before execution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @if [ "{{build_system}}" = "cargo" ]; then \ | ||
| (command -v cargo-deny >/dev/null && cargo deny check || echo "cargo-deny not installed; skip") && \ | ||
| (command -v cargo-audit >/dev/null && cargo audit || echo "cargo-audit not installed; skip"); \ | ||
| elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \ | ||
| uv run pip-audit 2>/dev/null || poetry run pip-audit 2>/dev/null || echo "pip-audit not installed; skip"; \ | ||
| elif [ "{{build_system}}" = "pnpm" ]; then pnpm audit --prod; \ | ||
| elif [ "{{build_system}}" = "bun" ]; then bun audit; \ | ||
| elif [ "{{build_system}}" = "npm" ]; then npm audit --omit=dev; \ | ||
| elif [ "{{build_system}}" = "go" ]; then govulncheck ./... 2>/dev/null || echo "govulncheck not installed; skip"; \ | ||
| else echo "no audit tool for {{build_system}}"; fi |
There was a problem hiding this comment.
The current shell logic for checking and running audit tools uses the (command -v tool && tool || echo "skip") pattern. In shell scripting, A && B || C is evaluated as (A && B) || C. If A (the command check) succeeds, but B (the audit tool) fails (e.g., because it found security vulnerabilities), the shell will proceed to execute C (echo "skip") and exit with a 0 status code. This silently masks actual security vulnerabilities and audit failures in CI.
We should use an explicit if statement to check if the command exists before running it, ensuring that any failure of the tool itself correctly propagates and fails the recipe.
For uv and poetry, we can also determine the runner prefix (uv run or poetry run) and check if the tool is installed (e.g., via --version) before running it, avoiding similar masking of failures.
@if [ "{{build_system}}" = "cargo" ]; then \\
if command -v cargo-deny >/dev/null 2>&1; then cargo deny check; else echo "cargo-deny not installed; skip"; fi && \\
if command -v cargo-audit >/dev/null 2>&1; then cargo audit; else echo "cargo-audit not installed; skip"; fi; \\
elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \\
if [ "{{build_system}}" = "uv" ]; then RUN="uv run"; else RUN="poetry run"; fi; \\
if $RUN pip-audit --version >/dev/null 2>&1; then $RUN pip-audit; else echo "pip-audit not installed; skip"; fi; \\
elif [ "{{build_system}}" = "pnpm" ]; then pnpm audit --prod; \\
elif [ "{{build_system}}" = "bun" ]; then bun audit; \\
elif [ "{{build_system}}" = "npm" ]; then npm audit --omit=dev; \\
elif [ "{{build_system}}" = "go" ]; then \\
if command -v govulncheck >/dev/null 2>&1; then govulncheck ./...; else echo "govulncheck not installed; skip"; fi; \\
else echo "no audit tool for {{build_system}}"; fi
| @if [ "{{build_system}}" = "cargo" ]; then \ | ||
| (command -v cargo-machete >/dev/null && cargo machete || echo "cargo-machete not installed; skip"); \ | ||
| elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \ | ||
| uv run deptry . 2>/dev/null || poetry run deptry . 2>/dev/null || echo "deptry not installed; skip"; \ | ||
| elif [ "{{build_system}}" = "pnpm" ]; then pnpm dlx depcheck; \ | ||
| else echo "no unused-dep tool for {{build_system}}"; fi |
There was a problem hiding this comment.
Similar to the _audit recipe, the _unused recipe uses the A && B || C and A || B || C patterns, which silently mask actual unused dependency detection failures (e.g., when cargo-machete or deptry exits with a non-zero code upon finding unused dependencies).
We should use explicit if statements to check for tool availability before running them.
@if [ "{{build_system}}" = "cargo" ]; then \\
if command -v cargo-machete >/dev/null 2>&1; then cargo machete; else echo "cargo-machete not installed; skip"; fi; \\
elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \\
if [ "{{build_system}}" = "uv" ]; then RUN="uv run"; else RUN="poetry run"; fi; \\
if $RUN deptry --version >/dev/null 2>&1; then $RUN deptry .; else echo "deptry not installed; skip"; fi; \\
elif [ "{{build_system}}" = "pnpm" ]; then pnpm dlx depcheck; \\
else echo "no unused-dep tool for {{build_system}}"; fi
| @if [ "{{build_system}}" = "cargo" ]; then cargo check --workspace --all-targets; \ | ||
| elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \ | ||
| (uv run mypy . 2>/dev/null || poetry run mypy . 2>/dev/null || echo "mypy not installed; skip"); \ | ||
| elif [ "{{build_system}}" = "pnpm" ]; then pnpm -r typecheck; \ | ||
| elif [ "{{build_system}}" = "bun" ]; then bun run typecheck 2>/dev/null || echo "no typecheck script"; \ | ||
| elif [ "{{build_system}}" = "npm" ]; then npm run typecheck --workspaces --if-present; \ | ||
| elif [ "{{build_system}}" = "go" ]; then go build ./... && go vet ./...; \ | ||
| else echo "no typechecker for {{build_system}}"; fi |
There was a problem hiding this comment.
The _typecheck recipe also suffers from the failure-masking issue:
- For
uv/poetry, ifmypyfinds type errors and exits with a non-zero code, it falls back toecho "mypy not installed; skip"and returns0. - For
bun, ifbun run typecheckfails due to actual type errors, it falls back toecho "no typecheck script"and returns0.
We should check for tool/script existence first before running them. For bun, we can check if the typecheck script is defined in package.json using grep.
@if [ "{{build_system}}" = "cargo" ]; then cargo check --workspace --all-targets; \\
elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \\
if [ "{{build_system}}" = "uv" ]; then RUN="uv run"; else RUN="poetry run"; fi; \\
if $RUN mypy --version >/dev/null 2>&1; then $RUN mypy .; else echo "mypy not installed; skip"; fi; \\
elif [ "{{build_system}}" = "pnpm" ]; then pnpm -r typecheck; \\
elif [ "{{build_system}}" = "bun" ]; then \\
if grep -q '"typecheck":' package.json; then bun run typecheck; else echo "no typecheck script"; fi; \\
elif [ "{{build_system}}" = "npm" ]; then npm run typecheck --workspaces --if-present; \\
elif [ "{{build_system}}" = "go" ]; then go build ./... && go vet ./...; \\
else echo "no typechecker for {{build_system}}"; fi
| @if [ "{{build_system}}" = "cargo" ]; then cargo doc --no-deps --workspace; \ | ||
| elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \ | ||
| uv run sphinx-build -b html docs/ docs/_build/ 2>/dev/null || \ | ||
| poetry run sphinx-build -b html docs/ docs/_build/ 2>/dev/null || \ | ||
| echo "no sphinx config; skip"; \ | ||
| else echo "no doc generator for {{build_system}}"; fi |
There was a problem hiding this comment.
The _docs recipe also masks documentation build failures (e.g., if sphinx-build fails due to syntax or configuration errors) by falling back to echo "no sphinx config; skip".
We should check if sphinx-build is available in the environment first.
@if [ "{{build_system}}" = "cargo" ]; then cargo doc --no-deps --workspace; \\
elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \\
if [ "{{build_system}}" = "uv" ]; then RUN="uv run"; else RUN="poetry run"; fi; \\
if $RUN sphinx-build --version >/dev/null 2>&1; then $RUN sphinx-build -b html docs/ docs/_build/; else echo "no sphinx config; skip"; fi; \\
else echo "no doc generator for {{build_system}}"; fi
| @if [ "{{build_system}}" = "cargo" ]; then \ | ||
| (command -v cargo-watch >/dev/null && cargo watch -x build -x test || \ | ||
| echo "cargo-watch not installed; install with: cargo install cargo-watch"); \ | ||
| elif [ "{{build_system}}" = "bun" ]; then bun run dev; \ | ||
| elif [ "{{build_system}}" = "pnpm" ]; then pnpm -r dev; \ | ||
| elif [ "{{build_system}}" = "npm" ]; then npm run dev --workspaces --if-present; \ | ||
| else echo "no dev script for {{build_system}}"; fi |
There was a problem hiding this comment.
The _dev recipe for Cargo uses the A && B || C pattern, which can print "cargo-watch not installed" if cargo watch fails or is terminated with a non-zero exit code.
We should use an explicit if statement to check for cargo-watch availability.
@if [ "{{build_system}}" = "cargo" ]; then \\
if command -v cargo-watch >/dev/null 2>&1; then cargo watch -x build -x test; \\
else echo "cargo-watch not installed; install with: cargo install cargo-watch"; fi; \\
elif [ "{{build_system}}" = "bun" ]; then bun run dev; \\
elif [ "{{build_system}}" = "pnpm" ]; then pnpm -r dev; \\
elif [ "{{build_system}}" = "npm" ]; then npm run dev --workspaces --if-present; \\
else echo "no dev script for {{build_system}}"; fi
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.
Reviewed by Cursor Bugbot for commit 6486ae7. Configure here.
| _audit: | ||
| @if [ "{{build_system}}" = "cargo" ]; then \ | ||
| (command -v cargo-deny >/dev/null && cargo deny check || echo "cargo-deny not installed; skip") && \ | ||
| (command -v cargo-audit >/dev/null && cargo audit || echo "cargo-audit not installed; skip"); \ |
There was a problem hiding this comment.
Installed audit failures exit zero
High Severity
The cargo _audit and _unused wrappers use (command -v tool && tool run || echo "…not installed; skip"), so when cargo deny, cargo audit, or cargo machete is installed but the check fails, the || echo branch runs and the recipe still exits successfully. The previous justfile propagated those failures, so just audit, just unused, and just ci can now pass after real policy or vulnerability findings.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 6486ae7. Configure here.
| else if build_system == "npm" { "npm run lint --workspaces --if-present" } \ | ||
| else if build_system == "uv" { "uv run ruff check . && uv run ruff format --check ." } \ | ||
| else if build_system == "poetry" { "poetry run ruff check . && poetry run ruff format --check ." } \ | ||
| else if build_system == "go" { "go vet ./... && gofmt -l ." } \ |
There was a problem hiding this comment.
Go lint ignores formatting drift
Medium Severity
For the go build system, lint_cmd ends with gofmt -l ., which lists unformatted files but exits 0 even when output is non-empty, so just lint and just ci do not fail on formatting violations unlike other stacks that use explicit format checks.
Reviewed by Cursor Bugbot for commit 6486ae7. Configure here.
| (command -v cargo-deny >/dev/null && cargo deny check || echo "cargo-deny not installed; skip") && \ | ||
| (command -v cargo-audit >/dev/null && cargo audit || echo "cargo-audit not installed; skip"); \ | ||
| elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \ | ||
| uv run pip-audit 2>/dev/null || poetry run pip-audit 2>/dev/null || echo "pip-audit not installed; skip"; \ |
There was a problem hiding this comment.
Poetry path runs uv first
Medium Severity
Branches keyed on build_system uv or poetry always try uv run … before poetry run …, even when detection chose poetry, so audits and unused-deps checks may run under the wrong toolchain or silently skip meaningful results.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6486ae7. Configure here.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
|
||
| _audit: | ||
| @if [ "{{build_system}}" = "cargo" ]; then \ | ||
| (command -v cargo-deny >/dev/null && cargo deny check || echo "cargo-deny not installed; skip") && \ |
There was a problem hiding this comment.
Suggestion: The cargo-deny branch treats any non-zero exit from cargo deny check as if the tool were missing because of ... && cargo deny check || echo .... That masks real vulnerability failures and makes just audit/just ci pass when they should fail. Split the "tool installed" check from the command execution so actual audit findings propagate a non-zero exit code. [security]
Severity Level: Critical 🚨
- ❌ `just audit` never fails on cargo-deny findings.
- ⚠️ `just ci` reports success despite deny vulnerabilities.
- ⚠️ Local security workflows relying on just get false assurance.Steps of Reproduction ✅
1. From the repository root `/workspace/Tasken`, run `just audit`; the entrypoint Justfile
at `justfile:14` imports `just/phenotype.just` defining the `audit` recipe at
`just/phenotype.just:102-116`.
2. Because `Cargo.toml` exists at the repo root (`/workspace/Tasken/Cargo.toml`), the
`build_system` variable at `just/phenotype.just:17-29` resolves to `"cargo"`.
3. The `audit` recipe calls `_audit` (`just/phenotype.just:106`), which in the cargo
branch executes line `108` `(command -v cargo-deny >/dev/null && cargo deny check || echo
"cargo-deny not installed; skip") && \`.
4. With `cargo-deny` installed and configured (see `deny.toml` at
`/workspace/Tasken/deny.toml`), if `cargo deny check` detects a violation it exits
non-zero; this makes `command -v ... && cargo deny check` as a whole fail, triggering the
`|| echo "cargo-deny not installed; skip"` part, which returns exit code 0 so `_audit`,
`audit`, and transitively `ci` (`just/phenotype.just:145`) all succeed even though the
deny check failed.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** just/phenotype.just
**Line:** 108:108
**Comment:**
*Security: The cargo-deny branch treats any non-zero exit from `cargo deny check` as if the tool were missing because of `... && cargo deny check || echo ...`. That masks real vulnerability failures and makes `just audit`/`just ci` pass when they should fail. Split the "tool installed" check from the command execution so actual audit findings propagate a non-zero exit code.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| _audit: | ||
| @if [ "{{build_system}}" = "cargo" ]; then \ | ||
| (command -v cargo-deny >/dev/null && cargo deny check || echo "cargo-deny not installed; skip") && \ | ||
| (command -v cargo-audit >/dev/null && cargo audit || echo "cargo-audit not installed; skip"); \ |
There was a problem hiding this comment.
Suggestion: The cargo audit invocation has the same masking pattern: if cargo audit finds vulnerabilities and exits non-zero, the || echo ... branch runs and returns success. This causes security checks to silently pass despite detected issues, so preserve non-zero exit codes from the audit tool. [security]
Severity Level: Critical 🚨
- ❌ `just audit` ignores non-zero cargo-audit vulnerability exits.
- ⚠️ `just ci` gives green status despite RustSec advisories.
- ⚠️ Developers running just locally get misleading security results.Steps of Reproduction ✅
1. From `/workspace/Tasken`, run `just audit`; `justfile:14` imports
`just/phenotype.just`, where `audit` and `_audit` are defined at
`just/phenotype.just:102-116`.
2. With `Cargo.toml` present, `build_system` resolves to `"cargo"`
(`just/phenotype.just:17-29`), so `_audit` takes the cargo branch (line 107) and executes
the cargo-audit command at line 109: `(command -v cargo-audit >/dev/null && cargo audit ||
echo "cargo-audit not installed; skip");`.
3. When `cargo-audit` is installed (validated by `command -v cargo-audit >/dev/null`) and
it finds vulnerable dependencies, the `cargo audit` process exits with a non-zero status,
causing the `command -v ... && cargo audit` part to fail.
4. That failure triggers the `|| echo "cargo-audit not installed; skip"` portion, which
prints the skip message and exits 0, so `_audit`, `audit`, and any `just ci` invocation
(`just/phenotype.just:145`) all report success even when `cargo-audit` actually detected
vulnerabilities; current GitHub workflows run `cargo-audit` separately
(`.github/workflows/cargo-audit.yml`) so CI still catches issues, but the Just-based flow
is incorrect.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** just/phenotype.just
**Line:** 109:109
**Comment:**
*Security: The `cargo audit` invocation has the same masking pattern: if `cargo audit` finds vulnerabilities and exits non-zero, the `|| echo ...` branch runs and returns success. This causes security checks to silently pass despite detected issues, so preserve non-zero exit codes from the audit tool.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| (command -v cargo-deny >/dev/null && cargo deny check || echo "cargo-deny not installed; skip") && \ | ||
| (command -v cargo-audit >/dev/null && cargo audit || echo "cargo-audit not installed; skip"); \ | ||
| elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \ | ||
| uv run pip-audit 2>/dev/null || poetry run pip-audit 2>/dev/null || echo "pip-audit not installed; skip"; \ |
There was a problem hiding this comment.
Suggestion: The Python audit fallback chain conflates "tool missing" with "audit failed": a real pip-audit failure from uv run can fall through to poetry run and then to echo, returning success. This suppresses detected vulnerabilities in CI. Check tool availability first, then run exactly one auditor and propagate its exit status. [security]
Severity Level: Critical 🚨
- ❌ Python `just audit` can never fail on pip-audit issues.
- ⚠️ `just ci` in Python stacks silently skips dependency vulnerabilities.
- ⚠️ Security posture weaker for uv/poetry-based Phenotype repositories.Steps of Reproduction ✅
1. In a Phenotype repo that imports `just/phenotype.just` in its justfile and has a Python
project layout (e.g. `pyproject.toml` and `uv.lock` so `build_system` resolves to `"uv"`
or `"poetry"` via `just/phenotype.just:24-26`), run `just audit`.
2. The `audit` recipe (`just/phenotype.just:102-105`) delegates to `_audit`
(`just/phenotype.just:106-116`); for `build_system` `"uv"` or `"poetry"`, line 110 chooses
the Python branch.
3. Line 111 executes `uv run pip-audit 2>/dev/null || poetry run pip-audit 2>/dev/null ||
echo "pip-audit not installed; skip";`: if `uv run pip-audit` is available and detects
vulnerabilities, `pip-audit` exits non-zero, causing the whole `uv run pip-audit` command
to fail and the shell to fall through to `poetry run pip-audit` instead of failing.
4. If the poetry-based invocation is missing or also returns non-zero (e.g. more
vulnerabilities or failures), the pipeline falls through again to `echo "pip-audit not
installed; skip"`, which exits 0 and makes `_audit`, `audit`, and `ci`
(`just/phenotype.just:145`) succeed even though a Python auditor actually reported
dependency vulnerabilities.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** just/phenotype.just
**Line:** 111:111
**Comment:**
*Security: The Python audit fallback chain conflates "tool missing" with "audit failed": a real `pip-audit` failure from `uv run` can fall through to `poetry run` and then to `echo`, returning success. This suppresses detected vulnerabilities in CI. Check tool availability first, then run exactly one auditor and propagate its exit status.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix|
|
||
| _unused: | ||
| @if [ "{{build_system}}" = "cargo" ]; then \ | ||
| (command -v cargo-machete >/dev/null && cargo machete || echo "cargo-machete not installed; skip"); \ |
There was a problem hiding this comment.
Suggestion: The unused-dependency check for cargo masks real findings: if cargo machete exits non-zero (unused deps detected), the || echo ... branch runs and returns success. That prevents unused/ci from failing when dependency hygiene issues exist. Keep the non-zero result when the tool is present and reports problems. [incorrect condition logic]
Severity Level: Major ⚠️
- ⚠️ `just unused` never fails when cargo-machete finds issues.
- ⚠️ `just ci` cannot enforce Rust dependency hygiene locally.
- ⚠️ Unused deps linger, increasing maintenance and potential attack surface.Steps of Reproduction ✅
1. From `/workspace/Tasken`, run `just unused` or `just ci`; `justfile:14` imports
`just/phenotype.just`, where `unused` and `_unused` are defined at
`just/phenotype.just:118-128`, and `ci` depends on `unused` at line 145.
2. With `Cargo.toml` present, `build_system` evaluates to `"cargo"`
(`just/phenotype.just:17-29`), so `_unused` takes the cargo branch at line 123.
3. Line 124 executes `(command -v cargo-machete >/dev/null && cargo machete || echo
"cargo-machete not installed; skip");`: when `cargo-machete` is installed, `command -v`
succeeds and `cargo machete` runs, exiting non-zero if it finds unused dependencies (the
normal mode of operation for a linter-like tool).
4. That non-zero exit from `cargo machete` causes the whole `command -v ... && cargo
machete` expression to fail, triggering the `|| echo "cargo-machete not installed; skip"`
branch, which exits 0 so `_unused`, `unused`, and `ci` all succeed even though unused
dependencies were detected.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** just/phenotype.just
**Line:** 124:124
**Comment:**
*Incorrect Condition Logic: The unused-dependency check for cargo masks real findings: if `cargo machete` exits non-zero (unused deps detected), the `|| echo ...` branch runs and returns success. That prevents `unused`/`ci` from failing when dependency hygiene issues exist. Keep the non-zero result when the tool is present and reports problems.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| _typecheck: | ||
| @if [ "{{build_system}}" = "cargo" ]; then cargo check --workspace --all-targets; \ | ||
| elif [ "{{build_system}}" = "uv" ] || [ "{{build_system}}" = "poetry" ]; then \ | ||
| (uv run mypy . 2>/dev/null || poetry run mypy . 2>/dev/null || echo "mypy not installed; skip"); \ |
There was a problem hiding this comment.
Suggestion: The mypy typecheck command has the same logic bug: type errors from uv run mypy are treated like "tool not installed" because of the || fallback chain and final echo. This makes typecheck pass even when type checking fails. Distinguish missing-tool detection from actual typecheck failures and return non-zero on errors. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ `just typecheck` passes even when mypy finds type errors.
- ⚠️ `just ci` in Python projects skips typecheck failures.
- ⚠️ Developers may merge code with undetected typing regressions.Steps of Reproduction ✅
1. In a Phenotype repository that imports `just/phenotype.just` and uses a Python
toolchain (`pyproject.toml` with either `uv.lock` or `poetry.lock` so `build_system`
becomes `"uv"` or `"poetry"` per `just/phenotype.just:24-26`), run `just typecheck` or
`just ci` (which depends on `typecheck` at `just/phenotype.just:145`).
2. The `typecheck` recipe (`just/phenotype.just:130-132`) calls `_typecheck`
(`just/phenotype.just:134-142`); for `"uv"`/`"poetry"` the branch at lines 136-137 runs
`(uv run mypy . 2>/dev/null || poetry run mypy . 2>/dev/null || echo "mypy not installed;
skip");`.
3. If `uv run mypy .` is available and finds type errors, `mypy` exits with a non-zero
status, causing the first command to fail and the shell to fall through to `poetry run
mypy .` instead of failing immediately.
4. If the poetry-based mypy invocation is missing or also finds type errors, the
expression falls through again to `echo "mypy not installed; skip"`, which returns 0 so
`_typecheck`, `typecheck`, and `ci` all succeed even when static type checking failed.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** just/phenotype.just
**Line:** 137:137
**Comment:**
*Incorrect Condition Logic: The mypy typecheck command has the same logic bug: type errors from `uv run mypy` are treated like "tool not installed" because of the `||` fallback chain and final `echo`. This makes `typecheck` pass even when type checking fails. Distinguish missing-tool detection from actual typecheck failures and return non-zero on errors.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
1 participant
User description
Summary
Replaces the hand-rolled 29-line
Justfileboilerplate withimport "phenotype.just". The 9 most common recipes (default, build, test, lint, fmt, audit, unused, ci, docs) are now defined once injust/phenotype.justand parameterized over the build system.Changes
just/phenotype.just(195 LOC) — shared recipe library with auto-detection across 9 build systems (cargo/pnpm/yarn/bun/npm/uv/poetry/go/mix)Justfile(29 → ~15 LOC) — now a thin shell:import "just/phenotype.just"+ stack-specific recipesLibrary features
default,build,test,lint,fmt,audit,unused,typecheck,ci,docs,dev,clean,infoAdoption plan
Per
/Users/kooshapari/CodeProjects/Phenotype/repos/.remember/NEXT-STEPS-DAG.mdTier 1, this is one of 5 mainline repos that have the standard 29-linePhenotype-org standard justfileboilerplate. Each gets its own 1-PR adoption for clean revertability.Refs: NEXT-STEPS-DAG.md Tier 0+1, FLEET_100TASK_DAG_V4.md V12 EXTENSION
Note
Low Risk
Developer tooling only; main risk is changed
just ci/just lintbehavior if CI relied on the old recipe set or strict audit failures.Overview
Introduces
just/phenotype.just, a vendored shared just recipe library that auto-detects the workspace build system (cargo, Node package managers, Python, Go, Elixir) and mapsbuild,test,lint, and related tasks to the right commands.The root
justfiledrops ~29 lines of cargo-only recipes in favor ofimport "just/phenotype.just", so standard tasks are defined once and reused across Phenotype repos.Behavior shifts for this cargo workspace:
cinow runstypecheckand uses orderlint→typecheck→test→audit→unused(previouslylint→test→audit→unused).lintruns clippy with--all-targets.audit/unusedskip gracefully when optional tools (cargo-deny,cargo-audit,cargo-machete) are missing instead of failing outright. The library also exposesdev,clean, andinforecipes not present in the old justfile.Reviewed by Cursor Bugbot for commit 6486ae7. Bugbot is set up for automated code reviews on this repo. Configure here.
CodeAnt-AI Description
Adopt the shared
phenotype.justrecipe set for this repoWhat Changed
phenotype.justlibrary instead of keeping its own full set of commonjustcommands.justfileis reduced to a thin entry point, while repo-specific commands remain here.Impact
✅ Same just commands across repos✅ Less duplicated setup in each project✅ Faster setup for new contributors💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.