Review: fix(ado): apply --types/--states/--no-create filters to push operations

Verdict: LGTM — clean, well-tested fix

The Problem

--types, --states, and --no-create flags only applied to the pull (import) side of ADO sync. When using --push-only, these filters were silently ignored — all local beads issues were pushed to ADO regardless of type or state, and new work items were created even with --no-create.

The Fix

2 files changed, +203 −3. One new function + 5 tests.

buildADOPushHooks (ado.go, ~47 lines):

Creates a tracker.PushHooks with a ShouldPush callback that implements three filters:

1. Type filter: Maps ADO type names (e.g., "Bug", "Task") to beads types via mapper.TypeToBeads(), builds a lookup map, rejects issues whose IssueType is not in the set.

2. State filter: Maps ADO state names (e.g., "Active", "New") to beads statuses via mapper.StatusToBeads(), builds a lookup map, rejects issues whose Status is not in the set.

3. No-create filter: When noCreate is true, rejects issues that have no ExternalRef or whose ref is not recognized by isExternalRef(). Only already-linked issues pass through.

Returns nil when no filters are active — the engine treats nil PushHooks as "push everything," avoiding unnecessary function call overhead.

Correctness ✅

• Uses the right hook. PushHooks.ShouldPush is the engine's designated extension point for custom push filtering. It's checked in both the single-issue push loop (line 669) and collectBatchPushIssues (line 780). No engine changes needed.

• Mapping direction is correct. The filters are specified in ADO terms ("Bug", "Active") but filtering happens on beads-side fields (issue.IssueType, issue.Status). The PR correctly maps ADO→beads using TypeToBeads/StatusToBeads at hook construction time, not at filter evaluation time. This means the map lookup in ShouldPush is O(1).

• isExternalRef is a method value (at.IsExternalRef), bound to the tracker instance. This correctly delegates to the ADO tracker's URL pattern matching without coupling the hook to the *ado.Tracker type.

• Nil-safe on filters. Both type and state filter blocks check filters != nil before accessing slice fields. The no-create check handles nil ExternalRef, empty string, and non-ADO refs.

• No interference with engine-level filtering. The engine's shouldPushIssue() checks opts.TypeFilter and opts.ExcludeTypes, but ADO doesn't populate those fields in SyncOptions. The PushHooks.ShouldPush callback runs after shouldPushIssue, so both layers compose cleanly without conflict or double-filtering.

Edge Cases 🔍

1. Many-to-one type mapping. Multiple ADO types can map to the same beads type (e.g., "User Story" and "Product Backlog Item" both → TypeFeature). If a user specifies --types "User Story", the push filter will allow all TypeFeature beads issues, including ones that may have originated from "Product Backlog Item". This is correct for bidirectional sync — the filter defines a category, not an origin.

2. Custom type/state maps. NewFieldMapper(nil, nil) uses Agile defaults in the tests. Production callers can pass custom stateMap/typeMap that remap ADO↔beads. Since buildADOPushHooks uses the mapper interface, custom maps work transparently.

3. --no-create semantics change. The old flag description was "Pull-only mode: never create issues in ADO" — which was already misleading (it said "in ADO" but only affected pull). The new description "Never create new items in either direction (pull or push)" is more accurate. On push, it means: only update existing linked ADO work items, never create new ones. This is the right semantic for incremental sync scenarios.

4. filters is nil when no --types/--states/config values are set. buildADOPushHooks handles this correctly — both allowedTypes and allowedStatuses remain nil, and if noCreate is also false, it returns nil (no hooks).

5. Area path / iteration path filters are not applied on push. This is correct — those filters are WIQL query constraints for ADO's API (pull-side only). They have no beads-side equivalent.

Performance ✅

The type and state maps are built once at hook construction time. ShouldPush does two map lookups + one function call per issue — negligible. Returns nil when no filters are active, so unfiltered syncs have zero overhead.

Style ✅

• 5 tests covering each filter independently + combined: no-filters (nil return), type-only, state-only, no-create-only, all three combined. Table-driven with descriptive names.
• strPtr helper duplicated locally in test file with a comment explaining why (avoids importing internal/tracker's unexported helper). Clean.
• Doc comment on buildADOPushHooks is precise about the mapping direction and noCreate semantics.
• Help text update in ado.go clearly explains the pull vs push behavior of each filter.

Ship it. Clean, focused fix that uses the existing engine hook pattern exactly as designed. No engine changes needed, tests are thorough, edge cases are handled.