Skip to content

feat(safeoutputs): add dynamic tags with allowed-tags to create/update work item#420

Merged
jamesadevine merged 3 commits into
mainfrom
copilot/update-ado-aw-workflow-tags
May 6, 2026
Merged

feat(safeoutputs): add dynamic tags with allowed-tags to create/update work item#420
jamesadevine merged 3 commits into
mainfrom
copilot/update-ado-aw-workflow-tags

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 6, 2026

create-work-item only accepted agent-supplied title and description; tags were static operator config only. update-work-item had no tag allowlist. Both tools now follow the same allow-list pattern used by create-pull-request (labels) and add-build-tag — and mirrors the create-issue pattern in gh-aw.

Summary

create-work-item

  • New agent parameter tags — agents can now supply tags dynamically; each tag is validated to reject semicolons (ADO's tag separator)
  • New config allowed-tags — restricts which agent-supplied tags are accepted; empty = any (default, same as gh-aw's allowed-labels). Supports prefix wildcards ("agent-*")
  • Merge semantics — static tags from config always applied; agent tags appended after allowlist check (case-insensitive dedup)

update-work-item

  • New config allowed-tags — when tags: true, restricts which tags the agent may set; same prefix wildcard semantics; empty = any (default)

This aligns with the gh-aw create-issue pattern: labels (static, always applied) + allowed-labels (allowlist for agent-provided labels, empty = any labels allowed, including creating new ones). The only ADO-specific extension is prefix wildcard support (e.g. "agent-*"), consistent with the existing add-build-tag behaviour.

Allowlist matching (tag_matches_pattern)

  • Bug fix — prefix wildcard matching was case-sensitive (tag.starts_with(prefix)) while exact matching was case-insensitive (eq_ignore_ascii_case). An operator writing allowed-tags: ["Agent-*"] would silently fail to match an agent-provided tag "agent-created". Both branches are now fully case-insensitive.
  • Refactor — the duplicated inline closure in create_work_item.rs and update_work_item.rs has been extracted into a shared pub(crate) fn tag_matches_pattern(tag: &str, pattern: &str) -> bool in src/safeoutputs/mod.rs, so future fixes only need to be made in one place.

Example front matter

safe-outputs:
  create-work-item:
    tags:
      - automated          # always applied
    allowed-tags:
      - "agent-*"          # agent may add any tag starting with "agent-"
      - review

  update-work-item:
    tags: true
    target: "*"
    allowed-tags:
      - "agent-*"

Test plan

cargo test — unit tests cover: semicolon rejection, allowlist enforcement, case-insensitive prefix wildcard matching (including mixed-case patterns), exact case-insensitive matching, *-only wildcard, config deserialization defaults, and tag merging. The tag_matches_pattern helper tests live in src/safeoutputs/mod.rs.

…e work item

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/35468e9f-e355-404b-bfdf-b5f8b34d9d85

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

@copilot does this align with GitHub/gh-aw approach for issues?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 6, 2026

@copilot does this align with GitHub/gh-aw approach for issues?

Yes — this follows the same pattern gh-aw uses for create-issue (create_issue.go):

Field gh-aw (create-issue) ado-aw (create-work-item)
Static tags/labels always applied labels tags
Agent-provided tags/labels agent labels param agent tags param
Allowlist (empty = any) allowed-labels allowed-tags

One small extension beyond the gh-aw baseline: ado-aw supports prefix wildcards in allowed-tags (e.g. "agent-*"), which is also the existing pattern in add-build-tag. This suits ADO's free-form tag model where tags aren't pre-created like GitHub labels are.

The same allowed-tags pattern was also added to update-work-item (gated by tags: true), matching how gh-aw's update-issue tool exposes its allowlist.

Copilot AI requested a review from jamesadevine May 6, 2026 14:50
@jamesadevine
Copy link
Copy Markdown
Collaborator

/rust-review

@jamesadevine
Copy link
Copy Markdown
Collaborator

@copilot What is gh-aw's allowed default labels? Does it allow all agent provided labels by default?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Rust PR Reviewer completed successfully!

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 6, 2026

@copilot What is gh-aw's allowed default labels? Does it allow all agent provided labels by default?

Yes — from create_issue.go in gh-aw:

AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Optional list of allowed labels. If omitted, any labels are allowed (including creating new ones).

When allowed-labels is omitted (empty slice), gh-aw allows the agent to use any label, including creating new ones that don't exist yet on the repo.

Our allowed-tags follows the exact same default: an empty allowed-tags list means any agent-provided tag is accepted. This is also consistent with add-build-tag's allowed-tags default in ado-aw.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🔍 Rust PR Review

Summary: Mostly solid — one correctness bug in the allowlist implementation, plus a DRY concern for a security-critical path.

Findings

🐛 Bugs / Logic Issues

  • create_work_item.rs:312 / update_work_item.rs:476 — prefix wildcard matching is case-sensitive, exact matching is not

    Both files share this same pattern:

    if let Some(prefix) = pattern.strip_suffix('*') {
        tag.starts_with(prefix)          // ← case-SENSITIVE
    } else {
        pattern.eq_ignore_ascii_case(tag) // ← case-insensitive
    }

    An operator who writes allowed-tags: ["Agent-*"] will silently fail to match an agent-provided tag "agent-created". This is inconsistent and will be surprising. The dedup logic on the merge path already uses eq_ignore_ascii_case, so the intent is clearly case-insensitive throughout.

    Fix:

    if let Some(prefix) = pattern.strip_suffix('*') {
        tag.to_ascii_lowercase().starts_with(&prefix.to_ascii_lowercase())
    } else {
        pattern.eq_ignore_ascii_case(tag)
    }

⚠️ Suggestions

  • Duplicate allowlist matching logic — the same security-critical closure is copy-pasted into both create_work_item.rs and update_work_item.rs. Bugs must be fixed in two places (as evidenced by the case-sensitivity bug above being present in both). A shared fn tag_matches_pattern(tag: &str, pattern: &str) -> bool in src/safeoutputs/mod.rs or a sibling tags.rs module would solve this.

✅ What Looks Good

  • Semicolon validation in CreateWorkItemParams::validate() is clean and well-placed — happens at the earliest possible point before any execution.
  • check_field_permissions gating ensures config.tags == true before the allowed-tags check is even reached in update_work_item.rs — correct ordering.
  • Merge semantics (static config tags always applied, agent tags appended with case-insensitive dedup) are intuitive and correctly implemented.
  • Test coverage is comprehensive: semicolon rejection, allowlist enforcement, prefix wildcards, config deserialization defaults, and merge behavior are all exercised.

Generated by Rust PR Reviewer for issue #420 · ● 256.4K ·

@jamesadevine
Copy link
Copy Markdown
Collaborator

@copilot create_work_item.rs:312 / update_work_item.rs:476 — prefix wildcard matching is case-sensitive, exact matching is not

Both files share this same pattern:

if let Some(prefix) = pattern.strip_suffix('') {
tag.starts_with(prefix) // ← case-SENSITIVE
} else {
pattern.eq_ignore_ascii_case(tag) // ← case-insensitive
}
An operator who writes allowed-tags: ["Agent-
"] will silently fail to match an agent-provided tag "agent-created". This is inconsistent and will be surprising. The dedup logic on the merge path already uses eq_ignore_ascii_case, so the intent is clearly case-insensitive throughout.

Fix:

if let Some(prefix) = pattern.strip_suffix('*') {
tag.to_ascii_lowercase().starts_with(&prefix.to_ascii_lowercase())
} else {
pattern.eq_ignore_ascii_case(tag)
}
⚠️ Suggestions
Duplicate allowlist matching logic — the same security-critical closure is copy-pasted into both create_work_item.rs and update_work_item.rs. Bugs must be fixed in two places (as evidenced by the case-sensitivity bug above being present in both). A shared fn tag_matches_pattern(tag: &str, pattern: &str) -> bool in src/safeoutputs/mod.rs or a sibling tags.rs module would solve this.

…ensitive prefix matching

- Add shared `pub(crate) fn tag_matches_pattern(tag, pattern)` to
  src/safeoutputs/mod.rs; both prefix-wildcard and exact branches are
  now fully case-insensitive (fixes the bug where `allowed-tags: ["Agent-*"]`
  silently failed to match `"agent-created"`)
- Replace the duplicated inline closures in create_work_item.rs and
  update_work_item.rs with calls to the shared helper
- Add unit tests covering case-insensitive prefix wildcard matching in
  mod.rs, create_work_item.rs, and update_work_item.rs

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/82fb0a02-078e-4c88-bf92-f4b0b0464462

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
@jamesadevine jamesadevine marked this pull request as ready for review May 6, 2026 15:33
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 6, 2026

@copilot create_work_item.rs:312 / update_work_item.rs:476 — prefix wildcard matching is case-sensitive, exact matching is not

Both files share this same pattern:
...

Fixed in commit 4501384 (with 9d6e367 removing duplicate tests).

Both issues are addressed:

Bug fix — prefix wildcard matching is now case-insensitive, consistent with exact matching:

// Before (case-sensitive prefix, case-insensitive exact — inconsistent)
if let Some(prefix) = pattern.strip_suffix('*') {
    tag.starts_with(prefix)
} else {
    pattern.eq_ignore_ascii_case(tag)
}

// After — both branches case-insensitive
tag.to_ascii_lowercase().starts_with(&prefix.to_ascii_lowercase())

Refactor — the shared pub(crate) fn tag_matches_pattern(tag: &str, pattern: &str) -> bool is now in src/safeoutputs/mod.rs. Both create_work_item.rs and update_work_item.rs call super::tag_matches_pattern(...). Unit tests for all cases (exact/prefix, upper/lower) live in mod.rs and cover: case-insensitive exact, case-insensitive prefix, *-only wildcard, and mismatches.

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:

  • spsprodeus21.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-c193135c9f7e56eb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-c193135c9f7e56eb r50ypad349u04bs0e2u6pix.0x3e93o.rcgu.o ba7ueo9j0r7yn28586nysz5.0x3e93o.rcgu.o 64-REDACTED-linux-gnu/bin/gcc-ld/ld.lld n4nujh7gcuyo9q2c67nbowd.0x3e93o.rcgu.o ykbv405lv8ym7267cjc8n0r.0x3e93o.rcgu.o 1hovb6m5ewtoyx740dxgu8j.0x3e93o.rcgu.o wjxbawanmrgepbmr0hgtexj.0x3e93o.rcgu.o dni3ce49fvnwwfz7c75b8d9.0x3e93o.rcgu.o n3s0sgyfdypdylib4tuazlv.0x3e93o.rcgu.o hbis5ftvz5a4njp5ikq21ei.0x3e93o.rcgu.o c3q4310756z2swkshymtuci.0x3e93o.rcgu.o 9aek�� xcky6705ydypephb6lyz5bz.0x3e93o.rcgu.o 78xmbzv9cwr6uc8cotv20kc.0x3e93o.rcgu.o mbols.o fe0dd3cb6f860c.cbash fe0dd3cb6f860c.c--norc fe0dd3cb6f860c.c--noprofile fe0dd3cb6f860c.c/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-a6fa967058a9d19relro (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-c193135c9f7e56eb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-c193135c9f7e56eb cfg(docsrs,test) --check-cfg cfg(feature, values(&#34;default&#34;, &#34;tracing&#34;)) targ�� r-filter-tier1-a--error-format=json target/debug/dep--json=diagnostic-rendered-ansi,artifacts,future-incompat bin/rustc k/ado-aw/ado-aw//home/REDACTED/.rustup/toolchains/stable-x86_64-REDACTED-linux-gnu/bin/rustc src/safeoutputs/mod.rs; both prefix-wildcard and exact branches are now fully c�� k/ado-aw/ado-aw/serde_path_to_error bin/rustc k/ad�� k/ado-aw/ado-aw/--error-format=json k/ado-aw/ado-aw/--json=diagnostic-rendered-ansi,artifacts,future-incompat f/tower-0.5.3/sr--crate-type k/ado-aw/ado-aw/git k/ado-aw/ado-aw/diff k/ado-aw/ado-aw/--name-only k/ado-aw/ado-aw/HEAD (dns block)
  • spsprodweu4.vssps.visualstudio.com
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-c193135c9f7e56eb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-c193135c9f7e56eb r50ypad349u04bs0e2u6pix.0x3e93o.rcgu.o ba7ueo9j0r7yn28586nysz5.0x3e93o.rcgu.o 64-REDACTED-linux-gnu/bin/gcc-ld/ld.lld n4nujh7gcuyo9q2c67nbowd.0x3e93o.rcgu.o ykbv405lv8ym7267cjc8n0r.0x3e93o.rcgu.o 1hovb6m5ewtoyx740dxgu8j.0x3e93o.rcgu.o wjxbawanmrgepbmr0hgtexj.0x3e93o.rcgu.o dni3ce49fvnwwfz7c75b8d9.0x3e93o.rcgu.o n3s0sgyfdypdylib4tuazlv.0x3e93o.rcgu.o hbis5ftvz5a4njp5ikq21ei.0x3e93o.rcgu.o c3q4310756z2swkshymtuci.0x3e93o.rcgu.o 9aek�� xcky6705ydypephb6lyz5bz.0x3e93o.rcgu.o 78xmbzv9cwr6uc8cotv20kc.0x3e93o.rcgu.o mbols.o fe0dd3cb6f860c.cbash fe0dd3cb6f860c.c--norc fe0dd3cb6f860c.c--noprofile fe0dd3cb6f860c.c/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw_derive-200f38d8081bcd5a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/mcp_http_tests-a6fa967058a9d19relro (dns block)
    • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-c193135c9f7e56eb /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-c193135c9f7e56eb cfg(docsrs,test) --check-cfg cfg(feature, values(&#34;default&#34;, &#34;tracing&#34;)) targ�� r-filter-tier1-a--error-format=json target/debug/dep--json=diagnostic-rendered-ansi,artifacts,future-incompat bin/rustc k/ado-aw/ado-aw//home/REDACTED/.rustup/toolchains/stable-x86_64-REDACTED-linux-gnu/bin/rustc src/safeoutputs/mod.rs; both prefix-wildcard and exact branches are now fully c�� k/ado-aw/ado-aw/serde_path_to_error bin/rustc k/ad�� k/ado-aw/ado-aw/--error-format=json k/ado-aw/ado-aw/--json=diagnostic-rendered-ansi,artifacts,future-incompat f/tower-0.5.3/sr--crate-type k/ado-aw/ado-aw/git k/ado-aw/ado-aw/diff k/ado-aw/ado-aw/--name-only k/ado-aw/ado-aw/HEAD (dns block)

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

@jamesadevine jamesadevine merged commit d02997a into main May 6, 2026
15 checks passed
@jamesadevine jamesadevine deleted the copilot/update-ado-aw-workflow-tags branch May 6, 2026 15:40
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