feat(forge): Azure DevOps integration — PRs, work items, multi-issue seeds - #351
Conversation
Adds az/azure-devops as a third forge alongside gh and glab: remote detection (dev.azure.com, *.visualstudio.com, ssh.dev.azure.com v3, vs-ssh), CLI + extension + Entra/PAT auth probing, PR status/policies/ comments, Azure Boards work-item listing and issue-task seeding, PR creation, IPC + frontend wiring, sandbox/Docker allowlisting, and docs. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…te-chip echo Codex 0.159 TUI fires sessionStart on the FIRST prompt submit, not at startup, so gating the seeded prompt on hook readiness guaranteed a timeout: every codex task ended in seed-blocked. hooksOwnStartupReadiness excludes codex from hook-owned startup readiness (work-state hooks still gate mid-turn), so it falls back to the painted-and-quiet heuristic with echo verification - which still refuses to submit into a picker dialog. Two stacked delivery bugs then surfaced on that path: the echo listener attached inside the text write's .then (an echo riding the write's own round trip read as 'did not echo' and withheld the CR), and the literal echo check never passes for multi-line prompts because codex renders a bracketed paste as a '[Pasted Content N chars]' chip - the chip IS the echo (a selection dialog can't draw one), so it is accepted for paste-wrapped sends. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The card's identity row is all shrink-0 except the spacer; a fully
populated row (~330px) overran the 280px default right panel and the
window clipped the rightmost 'Open on' button. ReviewChip ('Changes
requested', the widest label) now truncates with a title fallback.
forge.rs: pr_status/pr_comments/issue_list/pr_create re-probe the CLI
path instead of the startup-time cache, so a CLI installed mid-session
works without visiting Settings first.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- az calls scope explicitly (--org/--project/--repository); --detect gone, so PAT remotes cannot silently resolve the wrong org - From a PR works for azure: pr list --creator me, pr show, per-provider fetch refspecs, fork PRs refused instead of fetching the wrong branch - Multi-pick issues seed ONE task; plural WORK_ISSUES_PROMPT tail with provider noun swap; unmodified builtins only, user edits win - agentSend: paste chip counts as echo again for wrapped sends, failure diagnostics restored; cliRpc inject gets readiness + echo verification - NewTaskDialog: stale guards on seed-object identity, pane exclusivity, !N ref parsing, branch latch; CreatePrDialog provider gating - visualstudio.com org URLs drop the collection path (extension rejects it); legacy _ssh remotes parse; sibling-project repo guard Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
cdc9439 to
f15cca7
Compare
Conflict in WelcomeDialog.tsx: keep both new imports — the forge sign-in helpers and markDesktopEntryAsked from the Linux desktop-entry step.
simion
left a comment
simion
left a comment
There was a problem hiding this comment.
Verified locally on top of main: clean merge, cargo test 1202, npm test 2654, tsc -b + e2e typecheck clean, and task.e2e 88/88 plus git.e2e 75/75 on a rebuilt binary.
The argv discipline on the Rust side is right: everything goes through forge.rs::run() as separate argv elements, no shell anywhere, and passing explicit --org/--project/--repository instead of --detect is the correct call for exactly the reason the doc comment gives.
I am merging this, and then pushing three follow-ups straight after rather than holding the branch. Flagging them here so they are on the record and not silently absorbed:
1. Shell injection into an agent-run command. src/lib/forge.ts:93-94 and :105-106 build a command STRING that the agent is instructed to run, and single-quote the remote-derived org/project/repo without escaping embedded quotes.
azureRemoteParts decodes each path segment, so a remote containing %27 yields a literal ' and closes the quote.
Cloning a repo whose remote is crafted is enough to reach it, via commentPromptFor and the New Task issue seed.
This PR already has the fix one file over, in CreatePrDialog.tsx:126, with a comment saying precisely why.
2. The merge-handled localStorage key gained a provider segment (src/store/pr.ts:636) with no read of the legacy key, so on upgrade every already-handled merge is unhandled again and maybeHandleMerged re-fires once: a toast, a notification, and under on_pr_merge: "archive" an archive of a task the user had deliberately kept.
3. Dropping the providerByProject early-out (src/store/pr.ts:190) lets two in-flight resolves race, and the catch arm writes null unconditionally, so a transient failure can overwrite a good "azure" and leave the repo reading as forgeless.
Also worth a mention, not blocking: the new fixtures introduce @x.io, which is a registered domain. CLAUDE.md asks for the vocabulary already in the tree (user@example.com, e2e@termic.dev) precisely because someone owns anything that looks real. I am changing those in the follow-up.
Thank you for this. The remote-parsing work is the part that would have been miserable to get right later: four URL shapes including the SSH v3 forms and PAT-bearing remotes, with remote_for_display guarding against the PAT reaching UI copy, is careful work and it is tested.
feat(tasks): per-member PR/CI rows + update-all for multi-repo tasks Merged locally: GitHub reported the branch as CONFLICTING, but the merge is clean in both directions (merge-tree, and a real merge of main into the branch, both exit 0 with no conflicted paths). Its mergeability was computed before #351 landed and never recomputed. Verified on this exact main: cargo 1216, npm test 2665, git.e2e 75/75, tsc -b and the e2e typecheck clean.
…to run Follow-ups to #351, flagged in its review and fixed here rather than left. **Shell injection.** `azurePrThreadsCommand` and `azureWorkItemCommentsCommand` return a command STRING that is typed into a prompt and that the agent is instructed to execute, so every interpolated value is shell input rather than an argv element. They single-quoted the org, project and repo without escaping, and `azureRemoteParts` percent-DECODES each path segment, so a remote containing `%27` yields a literal quote, closes it, and the rest of the segment is read as shell. Cloning a repo whose remote is crafted is enough to reach it, through the PR-comment prompt and the New Task issue seed. The Rust side was never exposed (it builds argv through `forge.rs::run()`), and CreatePrDialog already escapes its branch exactly this way, with a comment saying why. The same escape is now a shared `shellArg`. The test parses the result with a small POSIX single-quote splitter rather than asserting on substrings, and the first version of it failed on a fix that worked: the correct `'\''` escape CONTAINS the sequence a substring test forbids. The control in the test shows what the unescaped build does, splitting one value into two shell words. **A merge re-announced once on upgrade.** The localStorage key gained a `provider` segment, so every merge already handled under the old spelling read as unhandled exactly once: a toast, a desktop notification, and under `on_pr_merge: "archive"` an archive of a task the user had chosen to keep. It now falls back to the legacy key, on the miss path only. **A transient failure could un-forge a repo.** `resolveProvider` dropped its early-out for good reasons (the real TTL is Rust-side), but its catch wrote null unconditionally, so two dialogs resolving the same project out of order could replace a good "azure" with null. CreatePrDialog reads null as "this repo is not on any forge" and disables draft-with-agent until relaunch. A first failure still records null, which is honest when nothing is known. **Fixtures.** The new ADO fixtures used `@x.io`, a registered domain that appears nowhere else in the tree. CLAUDE.md asks for the vocabulary already here precisely because someone owns anything that looks real; these are `@example.com` now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WBZ9SY1zTBpPW33QAekZC1
|
Merged, and the three follow-ups are on main in e380a5d. The injection one is worth a second look if you write another of these builders: The test for it is the part I would steal rather than the fix. It parses the built command with a small POSIX single-quote splitter and asserts the hostile value stays one argument, because my first attempt asserted on substrings and failed on a fix that worked: the correct Thanks again. The remote parsing is the part that would have been painful to retrofit, and it is tested properly. |
2 participants
Closes #328
Summary
Adds Azure DevOps Services (cloud) as a third forge provider alongside GitHub and GitLab — detected remotes,
azCLI +azure-devopsextension detection, auth detection (no tokens stored), PR status/checks/reviewers/comments, Boards work-item picking, and PR creation, all through the existing forge lifecycle.dev.azure.com/{org}/{project}/_git/{repo},*.visualstudio.com(collection path dropped — the extension's org-URL grammar rejects path segments),ssh.dev.azure.com:v3/...andvs-ssh.visualstudio.com:v3/..., legacy{project}/_ssh/{repo}, PAT-in-URL variants. Allazcalls pass explicit--org/--project/--repositoryrather than--detect, so a PAT-bearing remote can never silently resolve the wrong org.pr createwith--draft/--auto-complete,pr list --creator me,pr showfor the "From a PR" picker. Sibling-project repos with the same name are rejected (org-scoped PR IDs).boardslist for the issue picker, with provider nouns ("work item") throughout the UI and prompts.refs/pull/N/head,refs/merge-requests/N/head,refs/heads/<head>for azure); fork PRs are refused instead of fetching a same-named branch from the wrong repo. Fetches go through the shared noninteractive/deadline guards.WORK_ISSUES_PROMPT, provider-noun aware);!123and/pullrequest/Nquery forms; stale-operation guards keyed on dialog-open identity; exclusive source panes.[Pasted …]/[Paste #N]chips accepted as echo for wrapped sends; CLIinjectpath gets the same readiness + echo verification as seeding..azurestate dir mounted (host creds untouched), Azure hosts in the sandbox allowlist, docs + e2e wording updated.Out of scope: Azure DevOps Server/on-prem discovery (documented boundary).
Test plan
cargo test forge— 33 tests (remote forms, scope args, WIQL, PR/threads parsing, missing-PR classification,.azuremount guard)npx vitest run— 2642 tests (multi-pick prompt composition, plural/provider wording, cap enforcement, control-char stripping, paste-chip echo)tsc --noEmit,cargo checkcleanaz: work-item pick → seeded → codex submitted and ran--creator meand WIQL are exercised only against the stub)Generated with Devin