fix: harden area path matching and improve code clarity

- Replace expect() with unreachable!() for the impossible branch
  where allows_id returns None but area_path_prefix is also None
- Require path separator boundary in area path prefix matching so
  prefix "4x4" does not accidentally match "4x4Production"
- Add comments explaining why max budget is consumed before
  execution (prevents unbounded retries against failing endpoints)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: jamesadevine

src/execute.rs

@@ -107,7 +107,9 @@ pub async fn execute_safe_outputs(
             entry_json
         );
 
-        // Enforce update-work-item max: skip excess entries rather than aborting the whole batch
+        // Enforce update-work-item max: skip excess entries rather than aborting the whole batch.
+        // Budget is consumed before execution so that failed attempts (target policy rejection,
+        // network errors) still count — this prevents unbounded retries against a failing endpoint.
         if entry.get("name").and_then(|n| n.as_str()) == Some("update-work-item") {
             if update_wi_executed >= max_update_wi {
                 let wi_id = entry
@@ -139,7 +141,8 @@ pub async fn execute_safe_outputs(
             update_wi_executed += 1;
         }
 
-        // Enforce comment-on-work-item max: skip excess entries rather than aborting the whole batch
+        // Enforce comment-on-work-item max: same skip-and-continue pattern as update-work-item.
+        // Budget is consumed before execution so that failed attempts still count.
         if entry.get("name").and_then(|n| n.as_str()) == Some("comment-on-work-item") {
             if comment_wi_executed >= max_comment_wi {
                 let wi_id = entry

src/tools/comment_on_work_item.rs

@@ -220,10 +220,15 @@ impl Executor for CommentOnWorkItemResult {
                 )));
             }
             None => {
-                // Area path validation — need to fetch the work item
-                let prefix = target.area_path_prefix().expect(
-                    "allows_id returned None but area_path_prefix is also None; this is a bug",
-                );
+                // Area path validation — need to fetch the work item.
+                // Invariant: allows_id returns None only for StringTarget(s != "*"),
+                // and area_path_prefix returns Some for exactly that case.
+                let prefix = match target.area_path_prefix() {
+                    Some(p) => p,
+                    None => unreachable!(
+                        "allows_id returned None but area_path_prefix is also None"
+                    ),
+                };
                 debug!(
                     "Validating area path for work item #{} against prefix '{}'",
                     self.work_item_id, prefix
@@ -232,8 +237,15 @@ impl Executor for CommentOnWorkItemResult {
                     .await
                 {
                     Ok(area_path) => {
-                        // ADO area paths are case-insensitive
-                        if !area_path.to_lowercase().starts_with(&prefix.to_lowercase()) {
+                        // ADO area paths are case-insensitive and use backslash separators.
+                        // Require the match to land on a path boundary so that prefix "4x4"
+                        // doesn't accidentally match "4x4Production".
+                        let ap = area_path.to_lowercase();
+                        let pf = prefix.to_lowercase();
+                        let is_match = ap == pf
+                            || (ap.starts_with(&pf)
+                                && ap.as_bytes().get(pf.len()) == Some(&b'\\'));
+                        if !is_match {
                             return Ok(ExecutionResult::failure(format!(
                                 "Work item #{} has area path '{}' which is not under allowed prefix '{}'",
                                 self.work_item_id, area_path, prefix