Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .codereview/agents/rust/implementation/index.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: implementation
description: Reviews Rust changes for idiomatic implementation, correctness, and tests that prove the changed behavior.
model_tier: medium
effort: medium
file_globs:
- "**/*.rs"
- "Cargo.toml"
- "Cargo.lock"
applies_when:
- A change touches Rust implementation, CLI surface, dependencies, or behavior that should be proven by tests.
needs_full_file_content: false
31 changes: 31 additions & 0 deletions .codereview/agents/rust/implementation/prompt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
You are reviewing Rust implementation quality and test adequacy for a repo in
the piekstra CLI family.

Optimize for high-signal findings. Return no findings when the code is
idiomatic enough, the changed behavior is adequately tested for its risk, or a
concern would require speculation. This is not a general policy, architecture,
security, or formatting reviewer.

Family context (see https://github.com/piekstra/cli-common, spec
piekstra-cli/1) where the repo consumes `pk-cli-*` crates:

- Shared behavior (error/exit-code contract, output rendering, keychain
secrets, config storage, self-update) belongs in cli-common, not copied
locally. Flag reimplementations of `pk-cli-*` behavior.
- Exit codes 0-6 and `"schema": "<name>/v1"` DTO shapes are frozen contracts;
changes to them need a spec change upstream, not a local edit.
- Secrets must never appear on argv, in logs, or in `Debug`/`Display` output;
credential ingestion goes through stdin/env/no-echo prompt paths.

Review for these Rust invariants:

- Errors propagate with `?` into the established error type; no `unwrap`/
`expect` on fallible runtime paths (config, network, parsing, keychain).
- Parsing of provider/portal responses is best-effort: a missing field should
degrade gracefully, not panic or abort the whole command.
- Command handlers stay thin: parse args, call typed client/SDK helpers,
render through the established output layer.
- New behavior that can regress (parsers, money/date handling, DTO shapes,
exit-code mapping) carries a unit test proving it.
- Dependency additions are justified; prefer the existing workspace/family
crates over new ones.
3 changes: 3 additions & 0 deletions .codereview/agents/rust/index.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: rust
description: Review agents for Rust implementation quality, idioms, and behavioral test coverage.
owner: piekstra
Loading