fix(cli): resolve list/wait projects by repo URL, not directory name (COR-1577) by juangaitanv · Pull Request #122 · Corgea/cli

juangaitanv · 2026-06-29T21:00:32Z

What & why

corgea list, corgea list --issues, and standalone corgea wait resolved "which project" by the current-directory basename and asked the backend for an exact, case-sensitive match against Project.name. When the checkout directory name differs from the stored project name — the Bank of Hope case, dir dotnet-azure-web-tsb vs project bohappdev/dotnet-azure-web-tsb — three commands misbehaved:

corgea list --issues → exit 1, "Project with name '…' doesn't exist."
corgea list → exit 0, silent empty scan table (no error)
corgea wait (no --scan-id) → exit 1, "Error querying scan list"

This is the residual of COR-1493 (which fixed only the corgea scan → report-results path). Relates to COR-1577.

Approach (CLI-only)

The ticket suggested threading project_id/scan_id, but the listing endpoints accept neither as a query param. Instead this uses an identity the CLI already has and the backend already indexes — the repo URL:

Resolve the canonical project via GET /api/v1/projects?repo_url=<git remote slug>, then drive the existing name-based query_scan_list / get_scan_issues with the returned canonical name.
Old-backend safety guard: keep only candidates whose returned repo_url contains the slug. Pre-COR-1426 doghouse ignores the param and returns all projects; the guard rejects that, so resolution falls back cleanly.
CWD-name fallback when there is no git remote (no regression for non-git checkouts or older backends).
Drop the now-wrong client-side scan.project == project_name filter.
Add mutually-exclusive --project-name / --repo override flags to list and wait.
Replace the silent empty table and the bare "Error querying scan list" with clear miss messages that name the identifier tried.

No doghouse changes, no new persisted CLI state, no new backend query param.

Files

src/utils/generic.rs — extract_repo_slug (org/repo slug; keeps Azure _git).
src/utils/api.rs — /projects resolver (resolve_project_by_repo) with the old-backend guard, and the resolve_project orchestrator.
src/main.rs — --project-name / --repo flags on List/Wait, threaded through.
src/list.rs — resolve once (non-SCA arm), drop the client filter, clear miss messaging, keep --json stdout clean.
src/wait.rs — resolve once, build the scan URL from the resolved numeric id, actionable miss message.
src/scan.rs — semgrep/snyk paths now thread their existing project_name as the override (the wait::run project_id arg was not dead after all). BLAST flow untouched.
tests/ — 14 new integration tests (list_resolution.rs, wait_resolution.rs) + shared helpers, plus 5 new unit tests.

Testing

cargo build, cargo clippy --all-targets -- -D warnings, cargo fmt --check: clean.
cargo test: 456 passed, 0 failed.
The three baseline failures are reproduced and proven fixed by the new integration tests (mock backend faithful to the doghouse contract).

Notes for reviewers / ship-gates

On-prem version floor: the repo-URL fix takes effect on doghouse ≥ COR-1426 (2026-06-11). Older instances keep working via the CWD-name fallback (no regression) until upgraded.
src/scan.rs change is display-only for the semgrep/snyk post-scan results URL; the primary BLAST scan flow uses a separate UploadZipResult and never calls the modified wait::run.
Manual checks needing live backends (out of scope for this PR's automated tests): list/list --issues/wait on a mismatched checkout against doghouse ≥ COR-1426; the fallback path against a pre-COR-1426 instance; --repo <unknown slug>; the semgrep/snyk results link.

…(COR-1577) list/wait now resolve the canonical project via GET /api/v1/projects?repo_url= (with an old-backend safety guard and CWD-name fallback when there's no git remote); dropped the brittle client-side scan.project==name filter; added mutually-exclusive --project-name/--repo override flags; replaced the silent empty table and bare "Error querying scan list" with clear miss messages. CLI-only — no backend/persisted-state changes. Residual of COR-1493.

Addresses four review findings on the COR-1577 repo-URL resolver: 1. Boundary-aware repo match (was a substring `contains`): the backend's `repo_url__icontains` returns siblings like `org/repo-v2` for slug `org/repo`, and the old guard could confirm the wrong project. Now matches on a path-segment boundary (equal or ends-with `/<slug>`, after stripping `.git`/trailing slash). 2. Subdirectory invocation: resolution used `Repository::open`, which only succeeds at the repo root, so `list`/`wait` from a subdir fell back to the subdir basename. New `discover_repo_url` walks up via `Repository::discover`. 3. Surface hard resolver failures: `resolve_project` now returns a Result and propagates network/auth/5xx from `/projects` instead of silently falling back to the local-directory project; a clean no-match (or 404 on an old backend) stays a soft fallback. 4. Skip redundant resolution: `list --issues --scan-id` hits `/scan/{id}/issues`, which ignores the project, so the extra `/projects` round-trip is now skipped. Adds unit tests (boundary match, sibling rejection, 404-soft vs 5xx-hard) and integration tests (no `/projects` call for `--issues --scan-id`, resolution from a subdirectory).

Ibrahimrahhal

can we reduce the amount of slop comments

Resolutions: - wait.rs: keep COR-1577 resolve_project signature (main's determine_project_name(None) is the bug being fixed). - generic.rs: keep both discover_repo_url (PR) and is_at_repo_root (main). - list.rs: keep the PR's resolution flow; port main's COR-1639 SHA column (header + format_short_sha cell).

- Compare the whole post-host repo path, not a trailing org/repo slug: extract_repo_slug -> extract_repo_path, repo_url_matches_slug -> repo_url_matches_path. A nested mirror (…/mirrors/acme/api) or a differently-rooted path (…/org/team/repo) no longer matches; Azure `_git` and GitLab subgroup paths now compare in full. - Restore ScanUploadResult.project_id and thread it into wait::run, which skips resolution entirely when it has both a scan id and an upload project id, and only lists scans when no scan id was given. The semgrep/snyk post-scan wait makes zero /projects and /scans calls and links the id-form URL again. - Validate the project name before building the name-form scan URL, so a slash-only name can no longer produce /project//?scan_id=. - The unresolved-project listing miss exits 1 in both human and --json mode (JSON still prints its envelope first); a confirmed project with no scans stays exit 0. - Trim comment density to match the surrounding files. New tests: whole-path matching boundaries, wait resolving from a subdirectory, the post-upload wait issuing no /projects or /scans, trailing slash trimming, and the miss/empty exit codes.

juangaitanv · 2026-07-28T13:27:43Z

@Ibrahimrahhal cut the comment density down: the diff went from 209 comment lines on 1383 added (15%) to 158 on 1613 (9.8%), and most of what is left is in the tests. src/utils/api.rs and src/utils/generic.rs took the bulk of it — multi-line doc blocks collapsed to one or two lines, inline restatements of the code below them dropped.

I kept three that carry information not in the code: the doghouse normalize_repo_url citation on extract_repo_path, the old-backend /projects guard rationale, and the TCP-RST note on the api.rs test stub.

Also in this push: merged origin/main (conflicts resolved, COR-1639's SHA column ported into the re-indented block), and the five open review threads answered in code — whole-path repo matching, restored project_id threaded into wait, project-name validation before the scan URL, and exit 1 on an unresolved listing miss.

running_in_ci() is true under GitHub Actions, which sent upload_scan down the GITHUB_REPOSITORY branch and panicked. The test covers the wait phase, so remove CI/GITHUB_ACTIONS from its env and exercise one path everywhere.

Behavior-preserving: - api.rs test stub reuses `read_http_request`/`http_response` instead of hand-rolling the drain loop and the response `format!`. - drop the never-read `ProjectsResponse.status` (serde ignores unknown fields, so it bought no validation). - extract `resolve_or_exit` for the error handler duplicated verbatim in `list` and `wait`. - list: resolve per branch, dropping the `Option<ResolvedProject>` and its `expect`; collapse the duplicated unresolved-miss block to one copy (the JSON envelope still prints before the error). - wait: construct a `ResolvedProject` on the skip path, dropping the four `.as_ref().map(...)` chains; that path's name now falls back to "unknown" like `resolve_project` instead of an empty string. - generic.rs: share the origin-URL extraction via `origin_url`. - resolve_project: drop the `repo_was_explicit` flag and the lazy `cwd` closure. - generic.rs test: run its git commands through one helper that scrubs the inherited GIT_* env, as tests/cli_deps.rs already does, so the temp repo is built even when the suite runs from a git hook.

`wait::run` took five positional params, four of them Option<String>; `repo_override` and `project_id_override` were adjacent and same-typed, so transposing them at the scan.rs call sites compiled silently and changed resolution. `list::run` took nine and carried #[allow(clippy::too_many_arguments)]. - `ProjectSelector { name, repo }` replaces the two Option<&str> resolver params, so the pair travels as one value. - `WaitArgs`/`ListArgs` name every argument at the call sites; the too_many_arguments allow is gone rather than institutionalized. - list::run now owns its arguments, so two `is_some()` + `unwrap()` pairs clippy started flagging become `if let Some(id)`.

`spawn_stub` and `run_list`/`run_wait` were near-identical across list_resolution.rs and wait_resolution.rs, and the Arc<Mutex<Vec<String>>> hit-recording closure was copy-pasted at seven sites. - `common::Routes` is one route table for every endpoint the two suites stub; an unset field 404s, so "this endpoint is never dialed" tests just leave it out. - `spawn_recording_http_stub` wraps `spawn_http_stub` with the hit log, mirroring `vuln_api_stub::spawn_capturing_vuln_api_stub`. - `run_corgea(subcommand, ...)` replaces the two copies of the runner. Scaffolding only: no assertion changed.

The diff wrapped ~290 lines in a new `} else {`. The SCA branch ends at its print_table call, so returning there puts the issues and scan-listing arms back at their original indent level. Reviewable with `git show --ignore-all-space`: the removed `else`, the `return;`, one closing brace, and one line rustfmt rejoined now that it fits. Nothing else.

juangaitanv · 2026-07-28T14:18:16Z

Pushed a cleanup pass on top of the reviewed diff (fb7774a..da0848d) — no force-push, so the existing review threads are intact. No test assertion changed; this is quality only, not new behavior.

Four commits, each reviewable on its own:

1. d77a3e7 — core cleanups

The api.rs test stub now calls the existing vuln_api_stub::read_http_request / http_response instead of hand-rolling a header-drain loop and a response format!.
Deleted the never-read ProjectsResponse.status (serde ignores unknown fields, so it bought no validation).
Extracted resolve_or_exit — the same ~10-line resolver error handler was duplicated verbatim in list.rs and wait.rs.
list.rs: resolve per branch, dropping Option<ResolvedProject> and its .expect(...); the copy-pasted unresolved-miss block collapses to one copy (the JSON envelope still prints before the error — list_json_miss_is_valid_empty_envelope pins it).
wait.rs: construct a ResolvedProject directly on the skip path, dropping four .as_ref().map(...) chains. This also fixes a small inconsistency it surfaced: that path fell back to an empty string while resolve_project uses "unknown".
generic.rs: discover_repo_url and get_repo_info shared 4 identical lines → origin_url.
resolve_project: dropped the repo_was_explicit flag and the lazy cwd closure.

2. 175f2af — args structs
wait::run took 5 positional params, and repo_override / project_id_override were adjacent and same-typed — transposing them at the scan.rs call sites compiled silently and changed resolution. list::run took 9 and carried #[allow(clippy::too_many_arguments)]. Now ProjectSelector { name, repo } + WaitArgs / ListArgs, and the too_many_arguments allow is gone rather than institutionalized.

3. 0b0abdd — e2e harness dedup (tests only)
spawn_stub and the runner were near-duplicates across list_resolution.rs / wait_resolution.rs, and the Arc<Mutex<Vec<String>>> hit-recording closure was copy-pasted at 7 sites. Now one common::Routes table (an unset field 404s, so "this endpoint is never dialed" tests just leave it out), spawn_recording_http_stub, and run_corgea(subcommand, …).

4. da0848d — list.rs re-indent, alone
The original diff wrapped ~290 lines in a new } else {. The SCA branch now early-returns, putting the rest back at its original indent. Worth reading with git show --ignore-all-space da0848d — it's the removed else, a return;, one brace, and one line rustfmt rejoined.

Verified: cargo build, cargo clippy --all-targets (clean), cargo fmt --check, and the full suite under CI=1 GITHUB_ACTIONS=true (542 passing).

Deliberately not done here

Skipping the /projects round trip in corgea wait <scan-id> (raised twice in review). That call is what produces the id-form URL /project/17/?scan_id=…; without it we fall back to the CWD-basename name-form URL, which reintroduces the COR-1577 bug. Doing it properly means threading ScanResponse.project out of blast::check_scan_status — a behavior change, not a cleanup.
formatted_repo hand-parsing in list.rs (split('/').nth(3) assumes HTTPS and gets the wrong owner for scp-form remotes). Pre-existing code this PR only re-indented.

Filed as follow-up: COR-1718 — corgea scan and corgea list resolve projects by different keys. The write path (determine_project_name → last URL segment, CWD basename off the worktree root) and the read path (full org/repo, discovered upward) disagree in two verified cases: from a subdirectory, scan names the project after the subdir while list resolves the root project; and for org-qualified projects, scan sends ?project=repo while list resolves org/repo — the shape of this PR's own CANON fixture. Changing where scans land has backend implications well outside COR-1577's read-side scope, so it stays out of this PR.

… review) Two resolver gaps found in review: - `/api/v1/projects` is `@paginated(default_page_size=20, max_page_size=50)` over a `repo_url__icontains` filter ordered by `-created_at` (doghouse api/views/core.py:1640-1683, api/decorators.py:158). Searching only page 1 meant enough `acme/api-*` siblings could push the exact `acme/api` onto page 2, the guard reported no match, and resolution fell back to the CWD name — the very miss COR-1577 fixes. Now requests page_size=50 and walks pages until an exact match or the last page, bounded by PROJECTS_MAX_PAGES so a bogus total_pages cannot loop forever. - A 200 carrying an unparseable body (proxy HTML, incompatible schema) was converted to a clean miss, taking the same CWD-name fallback. That contradicts the function's own contract — only a 404 or a valid envelope with no match is soft — and can silently resolve a DIFFERENT project. It now propagates, like the 5xx path.

- Rename `resolve_or_exit` -> `resolve_project_or_exit` so it reads consistently with its sibling `resolve_project`. - Expose the project id `wait` already accepted internally from an upload response as a `--project-id` flag, and rename the field to `project_id` now that it has two sources. Paired with a scan id it skips resolution entirely, which is what a CI job passing the id between steps wants — and it makes `wait` usable during a /projects outage. `list` gets no equivalent flag: `/api/v1/issues` and `/api/v1/scans` accept only `project` (exact name) or `repo` (icontains), never an id (doghouse api/views/core.py:707,2057), so an id cannot drive those queries until the backend takes one.

Four resolver defects from the second review pass: - `repo_url_matches_path` compared paths only, so `github.com/acme/api` and `gitlab.com/acme/api` were interchangeable — the backend's hostless `icontains` returns both, and whichever sorted first won. Candidates are now held to the origin's host when both sides carry one; a bare `--repo org/repo` or a stored bare `acme/api` still matches, having no host to contradict. `extract_repo_host` joins `extract_repo_path` over a shared `split_remote`. - The unconfirmed fallback queried the checkout directory name, but the pre-COR-1577 CLI queried `determine_project_name(None)` — the repo basename at the worktree root. Cloning `acme/api` into `build-123` thus made an old or not-yet-onboarded backend miss a project it used to find. It now calls that same helper, so the fallback is unchanged from before. - `corgea list --project-name X` exited 1 when X had no scans: an explicit name never sets `confirmed`, and /scans answers 200-empty both for "no scans" and "no such project". The caller's exact name is the better authority, so this is now the same valid empty result as a confirmed project with no scans. - `--project-id` without a scan id only relabeled the link while the scan was still picked by the resolved name, so it could print another project's URL. clap now requires the scan id and rejects an empty value.

- `ProjectsResponse.projects` was `#[serde(default)] Option<Vec<_>>`, so a 200 error envelope like `{"status":"error"}` parsed clean, read as "no matches", and took the legacy-name fallback — defeating the hard-failure contract on the neighbouring parse arm. doghouse `@paginated` emits the key on every 200, empty array included (api/decorators.py:228,241,259), so the field is now required and a 200 without it fails the parse. - `--project-name` / `--repo` were accepted with `--sca-issues` and then ignored: the SCA branch returns before any resolution and `get_sca_issues` sent only pagination, so a script asking for one project silently got company-wide findings. `list_sca_issues` does take `project` (api/views/core.py:1179-1195), so the resolved name is now threaded into the request. Only an explicit selector scopes it — unflagged `--sca-issues` keeps returning the company-wide latest scan, since narrowing that default is a separate decision.

- `split_remote` read the first segment of any 3+ segment value as a host, so a bare GitLab `--repo group/subgroup/repo` resolved as host `group` + path `subgroup/repo` and queried the wrong project. Without a scheme or userinfo the leading segment now counts as a host only if it looks like one, so bare paths keep every segment (and `.git`/trailing `/` are trimmed on that route too). - `--project-name foo/` reached `?project=` verbatim, which the backend matches exactly, so it missed the project `foo`; the trim in wait.rs runs only after a scan is already selected. Normalized in `resolve_project`, where every caller passes through, and a slash-only value is rejected. - The no-repository fallback sent the raw directory basename, but `determine_project_name` sanitized it before this PR — a project onboarded from `my app` is stored as `my_app`. It now uses the same helper as the repo fallback; the raw name stays as the error label. Test fixes in the same pass: - `list_uses_canonical_name_from_repo` and `list_issues_shows_repo_resolved_issue` asserted only on stdout, which the stub supplies whatever the CLI sends, from a checkout named after the repo. Both now run from `build-123` and assert the project actually sent to /scans and /issues — the PR's central claim was previously untested. - `wait_project_name_trailing_slash_is_trimmed` likewise only checked the printed URL, which is trimmed separately; it now asserts the query.

The previous heuristic read a dotted leading segment as a host, which truncates a GitLab namespace that legitimately contains one: `--repo my.group/sub/repo` resolved as host `my.group` + path `sub/repo` and could never find the intended project. Only a scheme, userinfo, or an scp-style colon before the first slash now marks a value as a network remote; anything else is a bare path kept whole. Stored `repo_url`s are unaffected — doghouse `normalize_repo_url` rewrites `git@host:path` to `https://host/path` and never stores a schemeless value (heeler/models.py:224-232). Adds unit coverage for `extract_repo_host` and for bare-vs-marked paths, plus an e2e assertion on the request target for `my.group/sub/repo`.

- `get_scan_issues` interpolated the project name straight into the query string, so `--project-name 'foo&bar'` sent `?project=foo&bar&page=1` and the server read the project as `foo` — returning another project's issues under the name the caller asked for. Built with `query` now, like `query_scan_list` and `get_sca_issues`. - Hitting PROJECTS_MAX_PAGES returned `Ok(None)`, which the caller reads as a clean miss and answers with the legacy-name fallback — so a repo on page 21+ could silently list a different same-basename project and exit 0. A truncated search is now an error naming the reported page count. Also locks in the SSH-config host alias (`corp-github:org/repo`) that the scp-colon rule from fe5b050 already handles: no scheme, no userinfo, no dot, but the colon before the first slash still marks it a remote.

Requiring the origin host to equal the stored project's host broke every SSH-config alias remote — including this repository's own `git@github.com-corgea:Corgea/cli.git`, whose host is `github.com-corgea` and never equals the stored `github.com`. The exact full-path match was discarded and resolution fell back to the checkout basename, leaving COR-1577 unfixed for precisely the enterprise setups it targets. The host now only settles ties, which is all it was ever for: - a candidate on our host wins immediately; - a lone path match is accepted whatever its host, since there is nothing to disambiguate; - several path matches with none on our host is genuinely ambiguous and errors, naming the competing URLs and pointing at --project-name, rather than flipping a coin or falling back to a third project. The cross-forge collision the host check was added for is still caught: with `github.com/acme/api` and `gitlab.com/acme/api` both present, the origin host picks, and an unrelated origin gets the error.

leenk7991 · 2026-07-29T12:12:48Z

+/// none on our host is genuinely ambiguous and errors rather than guessing.
+///
+/// `Err` for hard failures (network/auth/5xx/unparseable body/ambiguity); a
+/// clean "no match" (or a 404 from a backend without the endpoint) is a soft


should we also validate the page index (should be 1)? if the endpoint doesn't exist it won't return a 404 on page 2 or 3...

leenk7991 · 2026-07-29T12:16:02Z

+    if status == reqwest::StatusCode::NOT_FOUND {
+        return Ok(None);
+    }
+    if !status.is_success() {


should we classify 403 and 401 as a soft miss? since we're hitting /projects now, a user may be able to see issues but not projects

leenk7991 · 2026-07-29T12:19:51Z

+        Ok(resolved) => resolved,
+        Err(e) => {
+            log::error!(
+                "Unable to resolve the Corgea project. Please check your connection and ensure that:\n\


nit: is this error message suitable for all the errors that get caught here?

leenk7991 · 2026-07-29T12:21:50Z

+            .filter_map(|p| p.repo_url.as_deref())
+            .collect();
+        return Err(format!(
+            "{} Corgea projects claim repo '{}' ({}); pass --project-name <NAME> to choose one",


projectSummary already has name, can we show the user which project names to choose from? (instead of repo urls)

leenk7991 · 2026-07-29T12:24:35Z

+            conflicts_with = "repo",
+            help = "Query this exact Corgea project name directly (skips repo auto-resolution)."
+        )]
+        project_name: Option<String>,


should we use NonEmptyStringValueParser for project_name and repo?

leenk7991 · 2026-07-29T12:27:44Z

+    if issues {
+        // The --scan-id issue route hits /scan/{id}/issues and ignores the
+        // project, so it needs no resolution.
+        let resolved = scan_id


selector flags are silently ignored when we pass --scan-id, should we add a warning log? or handle it during args parsing?

juangaitanv · 2026-07-29T13:56:19Z

Superseded by a two-PR split, per review feedback that this had grown well past the ticket (2,212 lines across 9 files).

fix(cli): resolve list/wait projects by repo URL, not directory name (COR-1577) #137 — the COR-1577 fix on its own: resolve list/wait by repo URL, with the path re-check that also guards old backends, the unchanged legacy fallback, and --project-name/--repo. 429 lines of production code.
fix(cli): harden project resolution (stacked on #137) #138 — stacked on it: everything review surfaced afterwards (pagination, host tie-breaking, hard-vs-soft failures, query encoding, --project-id, SCA scoping, argument structs, harness dedup).

Every finding resolved here is carried into one of the two — nothing was dropped. Two design conclusions from this thread drove the split and are worth keeping:

The numeric project id was never needed. heeler/urls.py:44 routes project/<str:project_id_or_name>/, so the canonical name links just as well — which removes the only thing /projects uniquely provided for the URL.
The listing endpoints already accept ?repo= (same backend commit as ?repo_url=, 612d274a). We do not use it, because on a pre-COR-1426 backend an ignored ?repo= silently returns every scan in the company, whereas an ignored ?repo_url= is caught by the client-side path re-check.

Closing in favour of #137.

cursor Bot reviewed Jun 29, 2026

View reviewed changes

Comment thread src/utils/api.rs Outdated

Comment thread src/utils/api.rs Outdated

Comment thread src/utils/api.rs Outdated

Comment thread src/list.rs Outdated

juangaitanv requested review from Ibrahimrahhal, leenk7991 and yhoztak June 30, 2026 08:20

leenk7991 reviewed Jul 1, 2026

View reviewed changes

Comment thread src/scan.rs

leenk7991 reviewed Jul 1, 2026

View reviewed changes

Comment thread src/wait.rs Outdated

leenk7991 reviewed Jul 1, 2026

View reviewed changes

Comment thread src/wait.rs

leenk7991 reviewed Jul 1, 2026

View reviewed changes

Comment thread src/utils/api.rs Outdated

leenk7991 reviewed Jul 1, 2026

View reviewed changes

Comment thread src/list.rs Outdated

Ibrahimrahhal requested changes Jul 14, 2026

View reviewed changes

Test added 2 commits July 28, 2026 15:01

juangaitanv requested review from Ibrahimrahhal and leenk7991 July 28, 2026 13:41

cursor Bot reviewed Jul 28, 2026

View reviewed changes

Comment thread src/wait.rs Outdated

Comment thread src/utils/api.rs Outdated

Comment thread src/utils/api.rs Outdated

Test added 4 commits July 28, 2026 16:02

Ibrahimrahhal requested changes Jul 29, 2026

View reviewed changes

Comment thread src/main.rs

Comment thread src/utils/api.rs Outdated

Comment thread src/utils/api.rs

Comment thread src/wait.rs Outdated