Skip to content

refactor(cli): extract shared ADO REST helpers into src/ado/mod.rs#580

Merged
jamesadevine merged 2 commits into
mainfrom
refactor/ado-mod-extraction
May 17, 2026
Merged

refactor(cli): extract shared ADO REST helpers into src/ado/mod.rs#580
jamesadevine merged 2 commits into
mainfrom
refactor/ado-mod-extraction

Conversation

@jamesadevine
Copy link
Copy Markdown
Collaborator

Why

Foundation for the Phase 1 CLI overhaul (planned in the session). Lifts the shared Azure DevOps REST helpers out of src/configure.rs into a new src/ado/mod.rs so the forthcoming pipeline lifecycle commands (�nable, disable,
emove, list,
un, status, secrets) can draw from one well instead of pub use-ing each other's internals.

No behaviour change: ado-aw configure still walks the same code paths via a thin orchestration shim.

What moves to src/ado/mod.rs (now all pub)

  • AdoContext, parse_ado_remote, get_git_remote_url, normalize_org_url, normalize_ado_yaml_path
  • AdoAuth + apply, resolve_auth, try_azure_cli_token, resolve_ado_context
  • DefinitionListResponse / DefinitionSummary / ProcessInfo
  • MatchMethod, MatchedDefinition, FuzzyMatchResult, fuzzy_match_by_name
  • list_definitions, match_definitions, get_definition_name, update_pipeline_variable, resolve_definitions
  • All shared unit tests (parse_ado_remote, normalize_org_url, fuzzy match, YAML path)

New stub signatures

Added so PRs 2-8 can depend on stable surfaces from day one — bodies fill in as each command lands:

  • get_repository_id (enable)
  • get_definition_full (enable / disable)
  • patch_queue_status (enable / disable)
  • delete_definition (remove)
  • create_definition (enable)
  • queue_build (run)
  • get_build (run)
  • get_latest_build (list / status)

What stays in src/configure.rs

Shrinks to ~155 lines of GITHUB_TOKEN-specific orchestration that calls into crate::ado::*:

  • resolve_token (interactive GITHUB_TOKEN prompt)
  • apply_token_updates (per-definition fail-soft loop)
  • pub async fn run (the CLI entry point)

Call-site updates

  • src/main.rs declares pub mod ado; and switches configure::get_git_remote_urlado::get_git_remote_url in the GitHub-remote guard.
  • src/compile/extensions/mod.rs switches crate::configure::AdoContext / get_git_remote_url / parse_ado_remotecrate::ado::.

git grep 'crate::configure::' outside the entry point returns zero.

Verification

  • cargo build — clean
  • cargo test — 1471 unit + 108 integration tests passing (no regression)
  • cargo clippy --all-targets --all-features — only pre-existing warnings (the two useless_vec lints in the moved test fixtures existed on main before this PR; clippy-fixer already handles them)

Next

After this lands, PRs 2-8 are independent and parallelize cleanly:

  • PR 2: feat(cli): add ado-aw enable
  • PR 3: feat(cli): add ado-aw disable
  • PR 4: feat(cli): add ado-aw remove
  • PR 5: feat(cli): add ado-aw list
  • PR 6: feat(cli): add ado-aw run
  • PR 7: feat(cli): add ado-aw status
  • PR 8: feat(cli): rename configure -> secrets with subcommands

Lifts the shared Azure DevOps REST helpers out of src/configure.rs into
a new src/ado/mod.rs so the forthcoming pipeline lifecycle commands
(enable, disable, remove, list, run, status, secrets) can draw from one
well instead of pub use-ing each other's internals.

No behaviour change: �do-aw configure still walks the same code paths
via a thin orchestration shim.

What moves to src/ado/mod.rs (now all pub):

- AdoContext, parse_ado_remote, get_git_remote_url, normalize_org_url,
  normalize_ado_yaml_path
- AdoAuth + apply, resolve_auth, try_azure_cli_token,
  resolve_ado_context
- DefinitionListResponse / DefinitionSummary / ProcessInfo
- MatchMethod, MatchedDefinition, FuzzyMatchResult, fuzzy_match_by_name
- list_definitions, match_definitions, get_definition_name,
  update_pipeline_variable, resolve_definitions
- All shared unit tests (parse_ado_remote, normalize_org_url, fuzzy
  match, YAML path)

New stub signatures (unimplemented!()) added so PRs 2-8 can depend on
stable surfaces from day one:

- get_repository_id, get_definition_full, patch_queue_status,
  delete_definition, create_definition, queue_build, get_build,
  get_latest_build

src/configure.rs shrinks to ~155 lines of GITHUB_TOKEN orchestration
that calls into crate::ado::*. src/main.rs and
src/compile/extensions/mod.rs switch from crate::configure:: to
crate::ado:: for the relocated symbols.

Foundation for the Phase 1 CLI overhaul tracked in the session plan.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: Clean extraction — looks good overall, with one actionable concern on the stubs and one minor edge case to note.

Findings

⚠️ Suggestions

  • src/ado/mod.rs — stub functions panic instead of returning Err

    All 8 placeholder functions (get_repository_id, get_definition_full, patch_queue_status, delete_definition, create_definition, queue_build, get_build, get_latest_build) use unimplemented!(), which panics rather than returning Err. In an async context with proper error handling, a panic is strictly worse — it unwinds across await boundaries unpredictably and skips any ?-based cleanup in callers.

    Prefer anyhow::bail!() so accidental calls before PRs 2–8 land produce a graceful, reportable Err instead of a panic:

    // before
    unimplemented!("filled in by PR 2 (ado-aw enable)")
    
    // after
    anyhow::bail!("not yet implemented: filled in by PR 2 (ado-aw enable)")
  • src/ado/mod.rs:normalize_org_url — undocumented edge case for dotted non-URL input

    The bare-name fast-path only fires when the input contains no .:

    if !trimmed.contains("://") && !trimmed.contains('/') && !trimmed.contains('.') {
        return format!("(dev.azure.com/redacted), trimmed);
    }

    A user passing something like my-corp.com (a dotted name that isn't a *.visualstudio.com URL) falls through both branches and returns as-is. url::Url::parse("my-corp.com") fails (no scheme), so it's returned verbatim — silently producing a broken org URL at the call site.

    ADO org names can't contain dots per ADO naming rules, so this isn't a real-world bug today. But given the function is now pub and will be called from 7+ future commands, it's worth either adding a doc-comment callout or an explicit error for the ambiguous case.

✅ What Looks Good

  • Perfect call-site hygiene: all three callers (src/main.rs, src/compile/extensions/mod.rs, src/configure.rs) are updated to crate::ado::*; git grep 'crate::configure::' outside the entry point is clean.
  • list_definitions pagination: headers are captured before resp.text().await consumes the response — correct ordering.
  • update_pipeline_variable preserves allowOverride on the round-trip GET→PUT cycle rather than clobbering it, and the non-atomicity is explicitly called out in the doc comment.
  • Error messages include response body snippets (body.chars().take(500)) — exactly the right level of diagnosability without risking log bloat.
  • All moved tests carry over intact with comprehensive coverage for parse_ado_remote, normalize_org_url, fuzzy matching, and YAML path normalization.

Generated by Rust PR Reviewer for issue #580 · ● 1.4M ·

@jamesadevine
Copy link
Copy Markdown
Collaborator Author

@copilot implement suggestions

…nput behavior

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/df14608b-778c-4bf3-863c-acebcb62281f

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

@copilot implement suggestions

Implemented in d41d20f. I replaced all lifecycle stub unimplemented!() calls with anyhow::bail!(...) so accidental calls fail gracefully, and I added a normalize_org_url doc note plus a unit test for dotted ambiguous input (my-corp.com). No UI changes in this PR, so no screenshot is applicable.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/githubnext/ado-aw/issues
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-5cd66b4699336750 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-5cd66b4699336750 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.1h/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.1ncjg9gt7i28q63lzni0kq21a.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.1p/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.1nk393ltxqzceir9ord5krk4x.0ayla29.rcgu.o 64-REDACTED-linux-gnu/bin/rust-lld /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.2l/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.21q8rz6jwze09ned2p4fr6d69.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.2m/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.29dxqit223xw1ccv0p5f07f5o.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.30/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.2kq3z0mupcfqeawng0febdrsj.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.31/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.31z1fnzn8j03byt5sao8tgkb2.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.42/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.3aa2rfid63xo9srith47b6a2b.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.4934yc�� /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.4sdmiar95uab0g9sh7pkk3spi.18zo7cz.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.4yo6asuqvic1apurk0bpj38qb.18zo7cz.rcgu.o g/de�� s/zerovec_derive/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.acc s/zerovec_derive/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a-m64 bin/rustc -a69babd28d2d3b2cc -a69babd28d2d3b2-m64 -a69babd28d2d3b2/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcI0vRWM/symbols.o bin/rustc (http block)
  • spsprodeus21.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-5cd66b4699336750 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-5cd66b4699336750 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.1h/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.1ncjg9gt7i28q63lzni0kq21a.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.1p/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.1nk393ltxqzceir9ord5krk4x.0ayla29.rcgu.o 64-REDACTED-linux-gnu/bin/rust-lld /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.2l/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.21q8rz6jwze09ned2p4fr6d69.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.2m/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.29dxqit223xw1ccv0p5f07f5o.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.30/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.2kq3z0mupcfqeawng0febdrsj.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.31/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.31z1fnzn8j03byt5sao8tgkb2.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.42/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.3aa2rfid63xo9srith47b6a2b.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.4934yc�� /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.4sdmiar95uab0g9sh7pkk3spi.18zo7cz.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.4yo6asuqvic1apurk0bpj38qb.18zo7cz.rcgu.o g/de�� s/zerovec_derive/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.acc s/zerovec_derive/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a-m64 bin/rustc -a69babd28d2d3b2cc -a69babd28d2d3b2-m64 -a69babd28d2d3b2/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcI0vRWM/symbols.o bin/rustc (dns block)
  • spsprodweu4.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-5cd66b4699336750 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-5cd66b4699336750 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.1h/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.1ncjg9gt7i28q63lzni0kq21a.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.1p/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.1nk393ltxqzceir9ord5krk4x.0ayla29.rcgu.o 64-REDACTED-linux-gnu/bin/rust-lld /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.2l/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.21q8rz6jwze09ned2p4fr6d69.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.2m/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.29dxqit223xw1ccv0p5f07f5o.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.30/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.2kq3z0mupcfqeawng0febdrsj.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.31/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.31z1fnzn8j03byt5sao8tgkb2.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.42/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-06c14a42ec911039.3aa2rfid63xo9srith47b6a2b.0ayla29.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.4934yc�� /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.4sdmiar95uab0g9sh7pkk3spi.18zo7cz.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/init_tests-fe8c41cc83486af2.4yo6asuqvic1apurk0bpj38qb.18zo7cz.rcgu.o g/de�� s/zerovec_derive/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.acc s/zerovec_derive/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a-m64 bin/rustc -a69babd28d2d3b2cc -a69babd28d2d3b2-m64 -a69babd28d2d3b2/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcI0vRWM/symbols.o bin/rustc (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants