feat(scheduler): add FileWatch TriggerConfig variant and wire into factory/API/CLI

[geoffjay]

Add FileWatch variant to TriggerConfig, wire FileWatchStrategy into the strategy factory (runner), add API validation (requires at least one path, valid mode string), register file_path/file_name/file_dir/event_type as known template variables, and add CLI args (--watch-path, --watch-pattern, --watch-event, --debounce-ms, --watch-mode, --watch-poll-interval).

Closes #812

[geoffjay]

Review: feat(scheduler): add FileWatch TriggerConfig variant and wire into factory/API/CLI

Clean, complete implementation that correctly wires the FileWatch trigger through all four layers. A few minor observations, none blocking.

---

Stack context

issue-812 sits third in the stack: epic-804 (#866) → issue-810 (#867) → issue-811 (#868) → issue-812 (#875) → issue-813 (#876). The epic-804 base needs restack against main, so the whole stack inherits that status. The upstream PRs (#867, #868) need to land before this one can merge. Adding needs-restack.

---

types.rs — correct

• FileWatch variant fields, serde defaults, and doc comment are all accurate.
• The paths field intentionally has no #[serde(default)], so deserialization correctly fails when paths is absent.
• default_file_watch_debounce_ms() = 200, default_file_watch_mode() = "auto", default_file_watch_poll_interval() = 5 all match what PR #876's serde tests assert.
• trigger_type() returns "file_watch" ✅
• is_implemented() returns true — correct, implementation is complete ✅
• is_one_shot() uses matches!(self, TriggerConfig::Delay { .. }) so FileWatch implicitly returns false without needing an explicit arm ✅

Minor: The doc comment opens with "Filesystem change trigger (Phase 7)." — the phase number is internal epic tracking language that will read as noise in generated docs. Consider replacing with a brief description like "Filesystem change trigger — fires when files under watched paths change.".

---

api.rs — thorough validation

The four checks are all correct:

1. paths non-empty ✅
2. No blank path strings (trimmed) ✅
3. mode in ["native", "polling", "auto"] ✅
4. Polling mode requires poll_interval_secs > 0 ✅

Minor: auto mode with poll_interval_secs == 0 is not rejected. If auto falls back to polling internally, a zero interval would cause the poller to spin. The default is 5 and the CLI default is also 5, so this only matters for direct API calls with a crafted payload. Low risk given the API is internal, but adding the same poll_interval_secs == 0 guard for auto would be more defensive.

---

runner.rs — correct wiring

The mode string → WatchMode mapping is correct:

• "native" → WatchMode::Native (no interval needed) ✅
• "polling" → WatchMode::Polling { interval_secs } ✅
• _ (catches "auto") → WatchMode::Auto { poll_interval_secs } ✅

The patterns.is_empty() { None } else { Some(patterns.clone()) } idiom correctly maps the serde representation to FileWatchStrategy::new's Option<Vec<String>> parameter ✅

The ? on FileWatchStrategy::new correctly propagates glob validation errors as anyhow::Error ✅

---

template.rs — correct

The four new template variables (file_path, file_name, file_dir, event_type) are documented in the table, placed in the right position, and registered in KNOWN_VARIABLES. The grouping comment // Metadata-backed (file_watch trigger) is consistent with the existing // Metadata-backed (webhook triggers) pattern ✅

---

cli/orchestrator.rs — correct

• All six new args use names matching the FileWatchStrategy parameter naming convention.
• --debounce-ms default (200), --watch-mode default ("auto"), --watch-poll-interval default (5) match the serde defaults. Good consistency.
• Client-side validation checks watch_paths.is_empty() and validates watch_mode against the three valid values — mirrors the server-side checks.
• display_workflow only shows poll interval when mode == "polling" || mode == "auto", which is correct since native mode doesn't use it.
• Existing tests updated with the new positional arguments ✅

Minor: The client-side create_workflow validates mode but does not validate watch_poll_interval > 0 for polling mode the way the server does. A user running --watch-mode polling --watch-poll-interval 0 will get a server-side error rather than a friendly CLI message. Low priority, but worth noting for consistency.

---

Summary

All five files are correct and consistent with existing patterns. The implementation is complete and the tests in PR #876 directly exercise this wiring. Approving; the two minor notes (Phase 7 jargon in docs, auto/zero-interval gap in API validation) are follow-up suggestions, not blockers.