WVDSH-1924: Add achievement list command

[cloud9c]

Summary

• add wavedash achievement list
• show achievement IDs and metadata in a readable table
• support --json for scripted usage, including image keys
• preserve the existing --game-id / config resolution behavior

Linear

WVDSH-1924 — Add CLI command to list achievements

Backend dependency

• https://github.com/wvdsh/wavedash/pull/886

This CLI command uses the new authenticated GET /api/games/{gameId}/achievements endpoint introduced by the backend PR.

Testing

• cargo clippy --all-targets -- -D warnings
• cargo test (63 passed)
• cargo run -- achievement list --help
• WAVEDASH_GAME_ID=… wavedash-dev achievement list --json | jq 'map(.identifier)'
• no-image create → table list → JSON list → delete E2E
• git diff --check

[cloud9c]

Updated E2E after review fixes. The test achievement was created without --image; the API returned "image": "".

$ wavedash-dev achievement create --game-id k970hpptdqtm3phwtr3719wx6s8brbyk --identifier E2E_NO_IMAGE_TEST --title "E2E No Image Test" --description "Achievement created without an image"
✓ Created achievement "E2E No Image Test" (id: pd752fmf0hyj6a331ppwmveqad8c0ycs, identifier: E2E_NO_IMAGE_TEST)

$ wavedash-dev achievement list --game-id k970hpptdqtm3phwtr3719wx6s8brbyk
╭──────────────────────────────────┬─────────────────────┬─────────────────────┬──────────────────────────────────────┬────────┬─────────┬───────────╮
│ ID                               ┆ Identifier          ┆ Title               ┆ Description                          ┆ Secret ┆ Stat ID ┆ Threshold │
╞══════════════════════════════════╪═════════════════════╪═════════════════════╪══════════════════════════════════════╪════════╪═════════╪═══════════╡
│ pd752fmf0hyj6a331ppwmveqad8c0ycs ┆ E2E_NO_IMAGE_TEST   ┆ E2E No Image Test   ┆ Achievement created without an image ┆ no     ┆ -       ┆ -         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┤
│ pd7b6964y7q9btfb68qndm0y0s8c17y8 ┆ QA_ACHIEVEMENT_LIST ┆ QA Achievement List ┆ Temporary achievement for CLI QA     ┆ yes    ┆ -       ┆ -         │
╰──────────────────────────────────┴─────────────────────┴─────────────────────┴──────────────────────────────────────┴────────┴─────────┴───────────╯

$ wavedash-dev achievement list --game-id k970hpptdqtm3phwtr3719wx6s8brbyk --json | jq ".[] | select(.identifier == \"E2E_NO_IMAGE_TEST\")"
{
  "_id": "pd752fmf0hyj6a331ppwmveqad8c0ycs",
  "identifier": "E2E_NO_IMAGE_TEST",
  "displayName": "E2E No Image Test",
  "description": "Achievement created without an image",
  "image": "",
  "secret": false
}

$ wavedash-dev achievement delete --game-id k970hpptdqtm3phwtr3719wx6s8brbyk --id pd752fmf0hyj6a331ppwmveqad8c0ycs
✓ Deleted achievement pd752fmf0hyj6a331ppwmveqad8c0ycs

$ wavedash-dev achievement list --game-id k970hpptdqtm3phwtr3719wx6s8brbyk --json | jq "[.[] | select(.identifier == \"E2E_NO_IMAGE_TEST\")]"
[]

[claude]

Claude finished @cloud9c's task in 3m 40s —— View job

---

Code review

• Gather context (diff, surrounding code)
• Review src/achievements.rs
• Review src/main.rs
• Post review findings

The command is well-built and lines up almost exactly with handle_team_list / handle_project_list in src/init.rs:476-526 — same preset, same --json-before-empty-check ordering, same empty-state message. Narrowing the create path onto its own CreatedAchievement struct instead of making the new fields Option on a shared one is the right call. One issue worth fixing before merge, plus some smaller things.

Findings

1. --json can emit a non-JSON first line — src/main.rs:691 (medium)

resolve_game_id announces env overrides through print_override_notice, which writes to stdout:

// src/config.rs:284
fn print_override_notice(text: &str) {
    println!("{} {}", "env override:".yellow(), text);
}

So WAVEDASH_GAME_ID=… wavedash achievement list --json | jq . prints env override: WAVEDASH_GAME_ID → game_id = … ahead of the array and fails to parse. Same for the upload_dir / entrypoint notices reachable on the config-file path (config.rs:692, :832, :911).

This is new here rather than pre-existing: achievement list is the first --json command routed through resolve_game_id — team list and project list take --team-id directly and never touch the notice path. Given --json exists for scripting, worth addressing. Simplest fix is eprintln! in print_override_notice; the notice is diagnostic, not output. Fix this →

2. description / image / secret are required for deserialization — src/achievements.rs:25-27 (low)

A document missing any of them fails the entire list with a raw serde error. Your E2E confirms the backend currently returns "image": "" even when create never sent one, so this works today — but handle_achievement_create only sets image when --image is passed, so presence rests entirely on a backend default. #[serde(default)] on the three is nearly free insurance.

3. Test coverage gap — src/achievements.rs:337-359 (low)

Both deserialize/serialize tests use an achievement with a stat link. The no-stat-link case — the common one, and the one your E2E actually ran — is only covered on the serialize side. Suggested test in the inline comment.

4. if achievement.secret { "yes" } else { "no" }.to_string() — src/achievements.rs:163 (nit)

Parses correctly (expression position), but reads as though .to_string() applies only to the else arm. Explicit parens or achievement.secret.to_string() would be clearer.

5. CreatedAchievement could use a why-comment — src/achievements.rs:11-17 (nit)

Two near-identical structs with no note on the split invites someone merging them back later. The rest of this file is generous with rationale comments, so it'd fit the local style.

6. The E2E output in the PR description is stale (note)

It shows Secret: false and blank Stat ID / Threshold cells, but the current code renders yes/no and -. Worth re-running so reviewers see what the merged code actually prints.

What I could not verify

cargo isn't available in this job's sandbox, so I could not run cargo check / clippy / test myself — findings above come from reading the code against src/config.rs, src/init.rs, and src/auth.rs. I'm relying on your reported cargo test (62 passed) and clippy run for compilation. I also can't see the backend PR (wvdsh/wavedash#886), so finding 2 is a contract-hardening suggestion rather than a confirmed break.

I've left the details as inline comments on the relevant lines.
· feat/list-achievements

[claude]

description, image, and secret are required for deserialization, so any achievement document missing one of them fails the whole list call with a raw serde error rather than degrading gracefully.

Your E2E run confirms today's backend returns "image": "" even for an achievement created without one, so this works right now. But handle_achievement_create only sets image when --image is passed, so the field being present at all depends entirely on a backend default — and older documents (or ones written by a different code path) may not have it.

Cheap hardening:

Suggested change

	description: String,
	image: String,
	secret: bool,
	#[serde(default)]
	description: String,
	#[serde(default)]
	image: String,
	#[serde(default)]
	secret: bool,

Your parses_the_achievement_list_response test would still pass, and a follow-up test with a minimal document would lock the behavior in.

[cloud9c]

The backend contract requires description, image, and secret: the Convex storage schema and API return validator use non-optional validators, and optional write inputs are normalized before storage (image ?? '', imported description ?? '', secret ?? false). I am intentionally keeping the Rust fields required so a backend contract regression fails visibly rather than being silently defaulted.

[claude]

Claude finished @cloud9c's task in 3m 28s —— View job

---

Code review

• Gather context (diff, surrounding code)
• Re-review src/achievements.rs after review fixes
• Check src/config.rs / src/main.rs
• Post findings

Re-reviewed after 8d2f61d. Everything from the last round is addressed, and CI (clippy --all-targets -D warnings + cargo test) is green on this head, so compilation and lint are verified rather than reported. One small thing left.

Finding

checked.len() >= 7 floor is now stale — src/main.rs:856 (low)

main has 7 --game-id args; this PR makes 8. The floor stayed at 7, so every_game_id_arg_rejects_a_blank_value no longer means what its assertion message says.

The validator itself is still pinned — if achievement list kept the arg but dropped parse_non_empty_arg, walk would find it and expect_err would panic. What the floor guards is the other failure: an arg getting renamed or dropped so walk stops seeing it at all. At 7-of-8 there's now one free slot for that to pass silently. Bump to 8. Fix this →

Previously raised, now resolved

• --json stdout pollution — fixed at the root: print_override_notice writes to stderr (src/config.rs:285). I also checked the other stdout writers reachable before a --json command: updater::check_for_update already uses eprintln!, and welcome::show_first_run_if_needed short-circuits on !stdout().is_terminal() (src/welcome.rs:33). So achievement list --json | jq . is clean now, including on a first run.
• #[serde(default)] on description / image / secret — closing this. @cloud9c's explanation holds: the API normalizes to '' / '' / false on both create and bulk import and stores them as required, so there is no document shape that omits them. My original concern was pre-normalization documents; if the stored schema is required, that set is empty. Same reasoning covers the Devin finding on the same lines.
• No-stat-link deserialize test — added (src/achievements.rs:363), and Option fields correctly deserialize absent → None.
• CreatedAchievement rationale comment — added.
• (if … { "yes" } else { "no" }).to_string() — parenthesized.

Notes

• The E2E output in the PR description is still from the pre-fix build: it shows Secret │ false and blank Stat ID / Threshold cells, where the current code renders no and -. Worth a re-run so the description matches what merges.
• handle_achievement_list tracks handle_team_list / handle_project_list (src/init.rs:476-526) exactly — same preset, same --json-before-empty-check ordering, same empty-state phrasing. Including the fact that --json re-serializes through the typed struct, so unmodeled API fields are dropped; that's the existing contract for all three, not something this PR changes.

What I could not verify

cargo isn't runnable in this job's sandbox, so I didn't execute the build myself — I'm relying on the green CI run on this head for compilation, clippy, and the 62 tests. I also can't see wvdsh/wavedash#886, so the image/description/secret normalization above rests on @cloud9c's description of the backend rather than on reading it.
· feat/list-achievements

[claude]

Claude finished @cloud9c's task in 6m 11s —— View job

---

Code review

• Gather context (diff, surrounding code)
• Review the latest commit (3b710d4)
• Post findings

Re-reviewed after 3b710d4. No findings. Everything raised across the previous two rounds is resolved, and CI is green on this head (CI run 31215937584 — clippy --all-targets -D warnings + cargo test), so compilation and lint are verified rather than reported.

Last round's finding, confirmed fixed

checked.len() >= 8 — src/main.rs:856. I counted the --game-id args directly: exactly 8 (src/main.rs lines 147, 292, 312, 334, 358, 376, 415, 450), so the floor is tight again and every_game_id_arg_rejects_a_blank_value means what its assertion message says. The new List arm uses parse_non_empty_arg like its siblings, so the blank-value walk covers it.

What else I re-checked and cleared

• handle_achievement_list (src/achievements.rs:119) still tracks handle_team_list / handle_project_list (src/init.rs:476-526) exactly — same preset, same --json-before-empty-check ordering, same empty-state phrasing. Header (7 cells) and row (7 values) line up; Option fields render -.
• stdout purity for --json: the println! → eprintln! change at src/config.rs:285 covers all five print_override_notice call sites (510, 676, 692, 832, 911). Nothing in the repo asserts the notice on stdout. The other pre-command stdout writers were already safe — welcome::show_first_run_if_needed short-circuits on !stdout().is_terminal() (src/welcome.rs:33), updater uses eprintln!.
• CreatedAchievement staying separate from Achievement: both structs' fields are read, so no dead-code risk under -D warnings.
• statThreshold: 1 (integer) deserializes into f64; absent statId/statThreshold correctly map to None, pinned by the new parses_an_achievement_without_a_stat_link test.

One residual note, not a finding

description / image / secret on Achievement (src/achievements.rs:27-29) remain required, so a document missing any would fail the whole list with a raw serde error. @cloud9c's reasoning holds — the Convex schema and return validator are non-optional and writes normalize (image ?? '', description ?? '', secret ?? false), so no such document shape exists, and keeping them required makes a contract regression fail loudly. Recording it as a deliberate choice, not asking for a change. Same disposition for the Devin finding on the same lines.

The E2E block in the PR description is now current — it shows no and - placeholders, matching what the code renders.

Nothing blocking from my side.
· feat/list-achievements

[claude]

Claude finished @cloud9c's task in 2m 22s —— View job

---

Code review

• Gather context (diff, surrounding code)
• Review changes since last round (33eafe5 merge of main)
• Re-verify previously-raised findings still hold
• Post findings

Re-reviewed at 33eafe5. No findings. The only change since the head I cleared last round (3b710d4) is the merge of main, and it's orthogonal to this PR. CI is green on this head (run 31414229313 — clippy --all-targets -D warnings + cargo test), so compilation and lint are verified rather than reported.

What the merge brought in, and why it doesn't touch this PR

f0c6660 adds a hidden --upload-source flag to wavedash dev and wavedash build push, plus config::UploadSource. Three interactions I checked specifically, since a merge is exactly where a previously-cleared finding quietly regresses:

• The >= 8 floor is still exact. origin/main has 7 long = "game-id" args; this PR adds the 8th on achievement list. main.rs now has exactly 8 (lines 150, 302, 322, 344, 368, 386, 425, 460), so every_game_id_arg_rejects_a_blank_value (src/main.rs:890) still means what its assertion message says — no free slot for a renamed or dropped arg to slip through. --upload-source added no new --game-id.
• The stderr fix survived the merge. main touched src/config.rs (+27, the UploadSource enum and its test) but not print_override_notice, which is still eprintln! at src/config.rs:286. achievement list --json | jq . stays clean.
• No merge-resolution damage in main.rs. The merge diff is purely additive against the PR's List arm and test module — the AchievementCommands::List variant, its run() arm (src/main.rs:720), and achievement_list_accepts_game_id_and_json_output all came through intact.

Previously raised, all still resolved

Finding	Status at 33eafe5
--json stdout pollution via print_override_notice	Fixed at the root (config.rs:286), all five call sites covered
checked.len() floor stale at 7	Bumped to 8, still tight
No-stat-link deserialize test	parses_an_achievement_without_a_stat_link present
CreatedAchievement rationale comment	Present (achievements.rs:11-12)
(if … { "yes" } else { "no" }).to_string()	Parenthesized (achievements.rs:165)
Devin: imageless achievement breaks the whole list	Resolved by Devin; backend normalizes image ?? ''

description / image / secret staying required on Achievement (src/achievements.rs:27-29) remains a deliberate choice per @cloud9c — the Convex schema and return validator are non-optional and writes normalize, so a contract regression fails loudly instead of being silently defaulted. Recording it, not re-litigating it.

Also re-checked and clear

• handle_achievement_list (src/achievements.rs:119) still mirrors handle_team_list / handle_project_list (src/init.rs:476-526) exactly — same preset + UTF8_ROUND_CORNERS + ContentArrangement::Dynamic, same --json-before-empty-check ordering, same empty-state phrasing. Header and row are both 7 cells.
• Error paths route through config::check_api_response, so a bad game ID gives the shared "Game not found… run wavedash init" message rather than a raw status.
• The E2E block in the PR description matches what the code renders (no, - placeholders).

What I could not verify

cargo isn't runnable in this job's sandbox, so I didn't execute the build myself — compilation, clippy, and the test suite come from the green CI run on this head. I also can't read wvdsh/wavedash#886, so the backend normalization claim underpinning the required-fields decision rests on @cloud9c's description.

Nothing blocking from my side.
· feat/list-achievements

[franzwarning]

looks good assuming list achievements backend endpoint is live, merge if it is