Skip to content

Commit f50a1a9

Browse files
jamesadevineCopilot
andcommitted
feat(compile): inject conditional Azure CLI advisory into the agent prompt
When the always-on AzureCli extension detects azure-cli on the host (AW_AZ_MOUNTS non-empty), append an Azure CLI advisory section to the agent prompt so the agent knows az is on PATH inside the sandbox, what it's good for, and the auth model. Skip the append when az is missing so the agent never tries to call az on a runner that doesn't have it. Design ====== AzureCliExtension::prepare_steps() now returns TWO YAML steps: 1. Detection (existing) — sets AW_AZ_MOUNTS to the two --mount args or empty string. 2. NEW: "Append Azure CLI prompt" — a single-quoted heredoc that cats an Azure CLI advisory into /tmp/awf-tools/agent-prompt.md, gated by `condition: ne(variables['AW_AZ_MOUNTS'], '')`. The CompilerExtension trait API is unchanged. wrap_prompt_append is unchanged. The single call site in common.rs:2311 is unchanged. prompt_supplement() on AzureCli stays None. The conditional injection is entirely self-contained inside the extension's own prepare_steps Vec. Why not extend the trait. The existing prompt_supplement() hook doesn't carry a step-level condition. Adding one would require a new trait method, a new wrap_prompt_append signature, an enum-macro arm update, and a call-site change in common.rs — disproportionate for a 15-line advisory that only one extension wants gated. Advisory content ================ The advisory assumes az IS available (no "may be" hedging — the step only runs when it is) and covers: - az devops family — autoauthed via $AZURE_DEVOPS_EXT_PAT when permissions: read: is declared - Azure Resource Manager — separate identity required, not provisioned by ado-aw - Microsoft Graph — same caveat as ARM - Fallback — file a missing-tool safe output naming azure-cli Heredoc terminator is SINGLE-QUOTED ('AZURE_CLI_PROMPT_EOF') so $AZURE_DEVOPS_EXT_PAT and similar literals are appended verbatim rather than being shell-expanded to the runner's PAT value. Locked in by test_azure_cli_prompt_append_uses_single_quoted_heredoc. Tests ===== Five new unit tests in src/compile/extensions/azure_cli.rs: - test_azure_cli_prompt_append_step_is_conditional - test_azure_cli_prompt_append_step_targets_agent_prompt_file - test_azure_cli_prompt_append_step_has_advisory_anchors - test_azure_cli_prompt_append_uses_single_quoted_heredoc - test_azure_cli_prompt_append_displayname_matches_lint_list Existing test_azure_cli_prepare_steps_detects_az_before_setting_var updated for the new vec length (2 instead of 1). Integration test test_default_pipeline_mounts_az_and_allows_azure_hosts extended to assert the displayName, the condition expression in the same step (proximity check), and the advisory anchor strings. tests/bash_lint_tests.rs REQUIRED_STEP_DISPLAY_NAMES gains "Append Azure CLI prompt" so shellcheck exercises the new heredoc. tests/safe-outputs/azure-cli.lock.yml regenerated. Docs ==== docs/network.md and docs/tools.md "Always-on Azure CLI" sections gain a paragraph describing the conditional advisory injection. The same edits also correct a small carryover inaccuracy from commit 7fe562f: the previous text said "leaves AW_AZ_MOUNTS unset" — the graceful-degradation fix actually sets it to the empty string. Now documented correctly with the rationale. Validation ========== - cargo build: clean - cargo test: 1753 unit + 119 compiler + all integration pass - cargo clippy --all-targets --all-features -- -D warnings: clean Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 7fe562f commit f50a1a9

6 files changed

Lines changed: 301 additions & 37 deletions

File tree

docs/network.md

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,11 @@ Instead, the prepare step does the detection itself at pipeline time:
7777
via `##vso[task.setvariable]`.
7878
* If either is missing, the step emits a
7979
`##vso[task.logissue type=warning]` explaining `az` won't be
80-
available inside the agent sandbox and leaves `AW_AZ_MOUNTS` unset
81-
(which expands to the empty string).
80+
available inside the agent sandbox and sets `AW_AZ_MOUNTS` to the
81+
*empty string* (also via `##vso[task.setvariable]` — leaving the
82+
variable undefined would make ADO render the literal `$(AW_AZ_MOUNTS)`
83+
in the AWF bash step, where bash would interpret it as a `$(...)`
84+
command substitution and kill the step under `set -e`).
8285

8386
The AWF invocation in the compiled YAML then includes a literal
8487
`$(AW_AZ_MOUNTS) \` line on its own in the `--mount` chain.
@@ -87,6 +90,23 @@ script: when az is present the two `--mount` args appear; when it's
8790
absent the line collapses to empty whitespace + the `\` continuation,
8891
which is a no-op.
8992

93+
### Agent prompt advisory (conditional)
94+
95+
When (and only when) `AW_AZ_MOUNTS` is non-empty, a follow-up
96+
*Append Azure CLI prompt* step appends an Azure CLI advisory section
97+
to `/tmp/awf-tools/agent-prompt.md`. The agent reads the prompt on
98+
startup and learns that `az` is on PATH, what it's good for
99+
(`az devops` autoauthed via `$AZURE_DEVOPS_EXT_PAT`, ARM and Graph
100+
requiring separate auth), and the fallback path (`missing-tool`
101+
safe output naming `azure-cli`).
102+
103+
The step is gated by `condition: ne(variables['AW_AZ_MOUNTS'], '')`,
104+
which reuses the same pipeline variable the detection step writes.
105+
On runners where `az` is missing, the advisory step is skipped
106+
entirely — the agent never sees Azure CLI guidance and never tries
107+
to call `az`, avoiding the "told to use `az`, fails with command
108+
not found" failure mode.
109+
90110
### Operator implications
91111

92112
- **Microsoft-hosted `ubuntu-latest`**: `az` is detected, mounted, and

docs/tools.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,11 @@ two things at pipeline time:
108108
pipeline variable
109109
`AW_AZ_MOUNTS=--mount /opt/az:/opt/az:ro --mount /usr/bin/az:/usr/bin/az:ro`.
110110
2. If either is missing, it emits a yellow ADO warning
111-
(`##vso[task.logissue type=warning]`) and leaves the variable
112-
unset.
111+
(`##vso[task.logissue type=warning]`) and sets the variable to
112+
the *empty string* (leaving it undefined would make ADO render
113+
the literal `$(AW_AZ_MOUNTS)` in the AWF bash step, where bash
114+
would interpret it as command substitution and kill the step
115+
under `set -e`).
113116

114117
The AWF invocation includes a `$(AW_AZ_MOUNTS) \` line in its
115118
`--mount` chain. ADO expands the variable at step start: present →
@@ -120,6 +123,15 @@ exist" on runners without `az`. See
120123
[`docs/network.md`](network.md#always-on-azure-cli-az) for the full
121124
design.
122125

126+
**Conditional agent prompt advisory.** When (and only when) `az` is
127+
detected, a follow-up *Append Azure CLI prompt* step appends an
128+
Azure CLI advisory section to the agent prompt. The agent then knows
129+
`az` is on PATH and what it's good for (use cases and auth model
130+
below). The step is gated by
131+
`condition: ne(variables['AW_AZ_MOUNTS'], '')`; on runners without
132+
`az` it is skipped and the agent never sees Azure CLI guidance —
133+
preventing "told to use `az`, fails with command not found" loops.
134+
123135
| Host posture | What you get |
124136
| ------------------------------------- | --------------------------------------------------------- |
125137
| Microsoft-hosted `ubuntu-latest` | Detected → mounted → `az` available in the sandbox |

src/compile/extensions/azure_cli.rs

Lines changed: 203 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -97,36 +97,57 @@ impl CompilerExtension for AzureCliExtension {
9797
}
9898

9999
fn prepare_steps(&self, _ctx: &CompileContext) -> Vec<String> {
100-
// Runtime detection step. Runs in the Agent job's prepare phase
101-
// (NOT a separate Setup job) so it shares the same pipeline-
102-
// variable scope as the subsequent AWF bash step. ADO pipeline
103-
// variables set via `##vso[task.setvariable]` are visible as
104-
// `$(NAME)` in later steps of the same job.
100+
// Returns two YAML steps, in order:
105101
//
106-
// Detection checks both /usr/bin/az (the launcher shim) AND
107-
// /opt/az (the Python venv that az actually runs in). Mounting
108-
// only one of the two would leave az partially available and
109-
// produce confusing errors inside the sandbox.
102+
// 1. Detection — runs in the Agent job's prepare phase (NOT a
103+
// separate Setup job) so it shares the same pipeline-variable
104+
// scope as the later AWF bash step. Sets `AW_AZ_MOUNTS` to
105+
// either the two `--mount` args or empty string, depending
106+
// on whether the host has azure-cli installed.
110107
//
111-
// The setvariable value uses spaces between args so bash
112-
// word-splits the unquoted `$(AW_AZ_MOUNTS)` expansion in the
113-
// AWF invocation into clean `--mount <spec>` tokens. The value
114-
// contains only path chars, `:`, and spaces — no shell
115-
// metachars — so unquoted expansion is safe.
108+
// 2. Conditional prompt append — appends an "Azure CLI" section
109+
// to `/tmp/awf-tools/agent-prompt.md` so the agent knows
110+
// `az` is on PATH inside the sandbox, what it's good for,
111+
// and the auth model. Gated by
112+
// `condition: ne(variables['AW_AZ_MOUNTS'], '')` so the
113+
// agent only sees the advisory on runners where az was
114+
// actually detected. The detection step above is the source
115+
// of truth for that variable and MUST run first.
116116
//
117-
// Both branches MUST set the variable (the else branch sets it
118-
// to empty string). If left undefined, ADO leaves the literal
119-
// `$(AW_AZ_MOUNTS)` in subsequent bash steps, where bash
120-
// interprets it as a `$(...)` command substitution, tries to
121-
// run a program named `AW_AZ_MOUNTS`, gets exit 127, and the
122-
// AWF invocation step dies under `set -e` — the opposite of
123-
// graceful degradation. Defining the variable as empty makes
124-
// ADO expand it to nothing, leaving a harmless `\`-continuation.
125-
//
126-
// Warning text is intentionally short and operator-facing.
127-
// Agents that don't invoke `az` are unaffected; agents that do
128-
// will get a normal "command not found" inside the sandbox.
129-
let step = r###"- bash: |
117+
// We do not implement `prompt_supplement()` because the
118+
// existing `wrap_prompt_append` helper doesn't emit a
119+
// `condition:` field. Emitting our own step here keeps the
120+
// trait API unchanged and confines the conditionality entirely
121+
// to this extension.
122+
vec![self.detection_step(), self.prompt_append_step()]
123+
}
124+
}
125+
126+
impl AzureCliExtension {
127+
/// Bash step that detects azure-cli on the host and sets the
128+
/// `AW_AZ_MOUNTS` pipeline variable. Always runs.
129+
///
130+
/// Detection checks BOTH `/usr/bin/az` (the launcher shim) and
131+
/// `/opt/az` (the Python venv that az actually runs in). Mounting
132+
/// only one of the two would leave az partially available and
133+
/// produce confusing errors inside the sandbox.
134+
///
135+
/// The setvariable value uses spaces between args so bash
136+
/// word-splits the unquoted `$(AW_AZ_MOUNTS)` expansion in the
137+
/// AWF invocation into clean `--mount <spec>` tokens. The value
138+
/// contains only path chars, `:`, and spaces — no shell
139+
/// metachars — so unquoted expansion is safe.
140+
///
141+
/// Both branches MUST set the variable (the else branch sets it
142+
/// to empty string). If left undefined, ADO leaves the literal
143+
/// `$(AW_AZ_MOUNTS)` in subsequent bash steps, where bash
144+
/// interprets it as a `$(...)` command substitution, tries to
145+
/// run a program named `AW_AZ_MOUNTS`, gets exit 127, and the
146+
/// AWF invocation step dies under `set -e` — the opposite of
147+
/// graceful degradation. Defining the variable as empty makes
148+
/// ADO expand it to nothing, leaving a harmless `\`-continuation.
149+
fn detection_step(&self) -> String {
150+
r###"- bash: |
130151
set -eo pipefail
131152
if [ -f /usr/bin/az ] && [ -d /opt/az ]; then
132153
echo "##vso[task.setvariable variable=AW_AZ_MOUNTS]--mount /opt/az:/opt/az:ro --mount /usr/bin/az:/usr/bin/az:ro"
@@ -136,8 +157,50 @@ impl CompilerExtension for AzureCliExtension {
136157
echo "##vso[task.logissue type=warning]Azure CLI not detected on this runner (missing /usr/bin/az or /opt/az). The az command will not be available inside the agent sandbox. Install azure-cli on the runner image to enable it."
137158
fi
138159
displayName: "Detect Azure CLI on host (for AWF mount)"
139-
"###;
140-
vec![step.to_string()]
160+
"###
161+
.to_string()
162+
}
163+
164+
/// Conditional `cat >>` step that appends an Azure CLI advisory
165+
/// section to the agent prompt file at pipeline time, only when
166+
/// the detection step above set `AW_AZ_MOUNTS` to non-empty.
167+
///
168+
/// Uses a SINGLE-QUOTED heredoc delimiter (`<< 'AZURE_CLI_PROMPT_EOF'`)
169+
/// so `$AZURE_DEVOPS_EXT_PAT` and any other dollar references inside
170+
/// the prompt body are appended literally rather than expanded by
171+
/// bash. The closing delimiter is indented to match the bash block
172+
/// scalar style used by `wrap_prompt_append`.
173+
///
174+
/// The `condition:` clause uses an ADO runtime expression. ADO
175+
/// evaluates it at step start against the variables visible at
176+
/// that moment — the detection step above has already run by
177+
/// then (steps execute sequentially within a job), so the
178+
/// expression sees the value just written by `setvariable`.
179+
///
180+
/// displayName must stay in sync with the entry in
181+
/// `tests/bash_lint_tests.rs::REQUIRED_STEP_DISPLAY_NAMES`.
182+
fn prompt_append_step(&self) -> String {
183+
r#"- bash: |
184+
cat >> "/tmp/awf-tools/agent-prompt.md" << 'AZURE_CLI_PROMPT_EOF'
185+
186+
---
187+
188+
## Azure CLI (`az`)
189+
190+
The Azure CLI is available inside this sandbox at `/usr/bin/az`. Prefer it over hand-rolled curl calls when it covers what you need:
191+
192+
- **Azure DevOps management** — `az devops`, `az pipelines`, `az repos`, `az boards`. These are authenticated automatically from `$AZURE_DEVOPS_EXT_PAT` when the pipeline declares `permissions: read:`. List/inspect operations Just Work; write operations honour the PAT's scopes.
193+
- **Azure Resource Manager** — `az resource`, `az account`, `az group`. These require a separate Azure identity that ado-aw does not provision out of the box; sign in with `az login` using credentials supplied by another mechanism (e.g. a service connection writing them into your sandbox env) before invoking them.
194+
- **Microsoft Graph** — `az ad`, `az rest`. Same caveat as ARM.
195+
196+
If a command you need isn't covered above, file a `missing-tool` safe output naming `azure-cli` so the operator can extend coverage rather than blocking on it silently.
197+
AZURE_CLI_PROMPT_EOF
198+
199+
echo "Azure CLI prompt appended"
200+
displayName: "Append Azure CLI prompt"
201+
condition: ne(variables['AW_AZ_MOUNTS'], '')
202+
"#
203+
.to_string()
141204
}
142205
}
143206

@@ -189,22 +252,27 @@ mod tests {
189252
let fm = fm();
190253
let ctx = CompileContext::for_test(&fm);
191254
let steps = ext.prepare_steps(&ctx);
255+
// Two prepare steps: [0] detection (always runs), [1] conditional
256+
// prompt-append (skipped when AW_AZ_MOUNTS is empty). The
257+
// detection step MUST stay at index 0 — it is what sets the
258+
// pipeline variable that the prompt-append step's
259+
// `condition:` reads.
192260
assert_eq!(
193261
steps.len(),
194-
1,
195-
"expected exactly one prepare step (the az detection step), got: {steps:?}"
262+
2,
263+
"expected two prepare steps (detection, conditional prompt-append), got: {steps:?}"
196264
);
197265
let step = &steps[0];
198266
// Detection must check both the launcher shim and the venv
199267
// directory — mounting only one would leave az partially
200268
// available and produce confusing errors inside the sandbox.
201269
assert!(
202270
step.contains("[ -f /usr/bin/az ]"),
203-
"prepare step must test for /usr/bin/az launcher: {step}"
271+
"first prepare step (detection) must test for /usr/bin/az launcher: {step}"
204272
);
205273
assert!(
206274
step.contains("[ -d /opt/az ]"),
207-
"prepare step must test for /opt/az venv directory: {step}"
275+
"first prepare step (detection) must test for /opt/az venv directory: {step}"
208276
);
209277
}
210278

@@ -316,6 +384,108 @@ mod tests {
316384
);
317385
}
318386

387+
// ── Conditional prompt-append step (step index 1) ──────────────────────
388+
389+
#[test]
390+
fn test_azure_cli_prompt_append_step_is_conditional() {
391+
// The prompt-append step MUST be gated by the AW_AZ_MOUNTS
392+
// pipeline variable so the agent only sees Azure CLI guidance
393+
// on runners where az was actually detected. Without this
394+
// gate the agent on a runner without az would be told to use
395+
// `az devops ...` and then fail with "command not found".
396+
let ext = AzureCliExtension;
397+
let fm = fm();
398+
let ctx = CompileContext::for_test(&fm);
399+
let steps = ext.prepare_steps(&ctx);
400+
let append = &steps[1];
401+
assert!(
402+
append.contains("condition: ne(variables['AW_AZ_MOUNTS'], '')"),
403+
"prompt-append step must be gated by condition: \
404+
ne(variables['AW_AZ_MOUNTS'], '') so it is skipped when \
405+
az is not detected on the host. Step:\n{append}"
406+
);
407+
}
408+
409+
#[test]
410+
fn test_azure_cli_prompt_append_step_targets_agent_prompt_file() {
411+
// Must `cat >>` to the same path other extensions' supplements
412+
// use (the conventional `wrap_prompt_append` target) so the
413+
// agent reads everything from one file.
414+
let ext = AzureCliExtension;
415+
let fm = fm();
416+
let ctx = CompileContext::for_test(&fm);
417+
let append = &ext.prepare_steps(&ctx)[1];
418+
assert!(
419+
append.contains(r#"cat >> "/tmp/awf-tools/agent-prompt.md""#),
420+
"prompt-append step must append to /tmp/awf-tools/agent-prompt.md \
421+
(matching wrap_prompt_append). Step:\n{append}"
422+
);
423+
}
424+
425+
#[test]
426+
fn test_azure_cli_prompt_append_step_has_advisory_anchors() {
427+
// Lock the advisory wording to the load-bearing parts: tool
428+
// names, env var, and the missing-tool escape hatch. Style
429+
// changes elsewhere in the prose are free; these anchors aren't.
430+
let ext = AzureCliExtension;
431+
let fm = fm();
432+
let ctx = CompileContext::for_test(&fm);
433+
let append = &ext.prepare_steps(&ctx)[1];
434+
for anchor in [
435+
"Azure CLI",
436+
"/usr/bin/az",
437+
"az devops",
438+
"AZURE_DEVOPS_EXT_PAT",
439+
"missing-tool",
440+
] {
441+
assert!(
442+
append.contains(anchor),
443+
"prompt-append step must contain anchor `{anchor}`. Step:\n{append}"
444+
);
445+
}
446+
}
447+
448+
#[test]
449+
fn test_azure_cli_prompt_append_uses_single_quoted_heredoc() {
450+
// The advisory body contains `$AZURE_DEVOPS_EXT_PAT` and other
451+
// literal dollar references. Single-quoting the heredoc
452+
// delimiter (`<< 'DELIM'`) is what prevents bash from
453+
// expanding them while building the prompt file. If anyone
454+
// ever swaps to an unquoted heredoc, `$AZURE_DEVOPS_EXT_PAT`
455+
// would be replaced by the runner's PAT value (a secret) and
456+
// baked into the agent prompt — a real leak.
457+
let ext = AzureCliExtension;
458+
let fm = fm();
459+
let ctx = CompileContext::for_test(&fm);
460+
let append = &ext.prepare_steps(&ctx)[1];
461+
assert!(
462+
append.contains("<< 'AZURE_CLI_PROMPT_EOF'"),
463+
"prompt-append heredoc delimiter must be single-quoted to \
464+
prevent expansion of $AZURE_DEVOPS_EXT_PAT and similar \
465+
literals inside the prompt body. Step:\n{append}"
466+
);
467+
}
468+
469+
#[test]
470+
fn test_azure_cli_prompt_append_displayname_matches_lint_list() {
471+
// The lint test in tests/bash_lint_tests.rs has a coverage
472+
// list (REQUIRED_STEP_DISPLAY_NAMES) keyed on displayName.
473+
// If we ever rename this step the lint stops exercising it
474+
// silently. Lockstep the exact string here so a future rename
475+
// forces an explicit update in both places.
476+
let ext = AzureCliExtension;
477+
let fm = fm();
478+
let ctx = CompileContext::for_test(&fm);
479+
let append = &ext.prepare_steps(&ctx)[1];
480+
assert!(
481+
append.contains(r#"displayName: "Append Azure CLI prompt""#),
482+
"prompt-append step displayName must be exactly \
483+
\"Append Azure CLI prompt\" to match the coverage entry \
484+
in tests/bash_lint_tests.rs::REQUIRED_STEP_DISPLAY_NAMES. \
485+
Step:\n{append}"
486+
);
487+
}
488+
319489
#[test]
320490
fn test_azure_cli_required_bash_commands_includes_az() {
321491
let ext = AzureCliExtension;

tests/bash_lint_tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ const REQUIRED_STEP_DISPLAY_NAMES: &[&str] = &[
115115
"Resolve ADO organization", // src/engine.rs copilot_install_from_nuget (1ES, no org)
116116
"Append Node prompt", // src/runtimes/node/extension.rs via wrap_prompt_append("Node")
117117
"Append dotnet prompt", // src/runtimes/dotnet/extension.rs via wrap_prompt_append("dotnet")
118+
"Append Azure CLI prompt", // src/compile/extensions/azure_cli.rs conditional prompt-append step
118119
"ado-aw", // src/compile/extensions/ado_aw_marker.rs metadata marker step
119120
];
120121

0 commit comments

Comments
 (0)