PR18.6: enable trigger poller via config + env

[henrypark133]

Summary

Enable the trigger poller from [trigger_poller] in the Reborn config
file plus two env-var overrides. PR 18 wired the lifecycle; PR 18.6
gives operators the surface to turn it on. Default stays off in
every shipped profile — no behavior change unless explicitly enabled.

What changed

• ironclaw_reborn_config: new [trigger_poller] TOML section
  (TriggerPollerConfigSection) with optional fields: enabled,
  poll_interval_secs, fires_per_tick,
  max_concurrent_fires_per_trigger, startup_jitter_max_secs,
  tick_jitter_max_secs. #[serde(deny_unknown_fields)] matches the
  sibling sections.
• ironclaw_reborn_cli (runtime/trigger_poller.rs): new
  trigger_poller_settings() helper merges three layers — compiled
  default → config-file section → env vars. Precedence is env > config

  default. Env vars:

  • IRONCLAW_TRIGGER_POLLER_ENABLED — 1/true enable,
    0/false disable (case-insensitive); any other value is a
    fatal startup error.
  • IRONCLAW_TRIGGER_POLLER_INTERVAL_SECS — u64, must be > 0.
• V1 invariants enforced at parse time (fail at boot, not at
  spawn): poll_interval_secs > 0, fires_per_tick > 0,
  max_concurrent_fires_per_trigger == 1. The third also re-checks at
  spawn in TriggerPollerWorkerConfig::validate; the CLI guard is
  kept deliberately so misconfig fails during boot-config validation.
• .env.example: documents the two env vars with their accepted
  values and the fatal-on-bad-value behavior.
• Module split: runtime.rs → runtime/mod.rs + new
  runtime/trigger_poller.rs so the poller helper + its tests live
  in their own ~360-line file and mod.rs stays below the 1000-line
  soft norm in .claude/rules/architecture.md.

Test coverage

11 new unit tests in runtime::trigger_poller::tests and 4 in
config_file::tests:

• Defaults: no section, no env → disabled with zero jitter
• Config: full section maps every worker field; partial section leaves
  other fields at defaults
• Config errors: poll_interval_secs = 0, fires_per_tick = 0,
  max_concurrent_fires_per_trigger = 2, unknown TOML key
• Env: ENABLED=true overrides config-false; INTERVAL_SECS=45
  overrides config interval; INTERVAL_SECS=0 errors;
  INTERVAL_SECS="10s" preserves the underlying parse error in the
  message; ENABLED=yes errors with the offending value

Env-mutating tests are serialized via a static LazyLock<Mutex> and
use a panic-safe RAII EnvGuard so a failed assertion cannot leak
mutated env into sibling tests in the same process.

Out of scope

• Actually enabling the poller in any shipped profile — still
  default-off; operator opt-in only.
• Authentication / authz at fire time — tracked in PR 18.5b.
• Tooling for managing triggers in the web UI — tracked in PR 18.9 /
  18.10 (Automations panel).
• E2E that drives a real cron through the poller — tracked in PR 18.7
  (full-path integration) and PR 18.8 (Python e2e).

Test plan

• cargo fmt --all --check
• cargo clippy -p ironclaw_reborn_cli -p ironclaw_reborn_config --all-targets --all-features
• cargo test -p ironclaw_reborn_cli -p ironclaw_reborn_config
• Manual smoke: drop [trigger_poller] enabled = true into a
  Reborn config, start the binary, confirm the poller spawn log
  appears in stderr.
• Manual smoke: same with IRONCLAW_TRIGGER_POLLER_ENABLED=1
  and no config section.

🤖 Generated with Claude Code

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[henrypark133]

Code Review (8 reviewers, sonnet) — head baa7a1b

Intent: Enable trigger poller via Reborn TOML config + env-var overrides. Default off in shipped profiles. Boot-time invariant validation. Module split to keep runtime/mod.rs under 1000-line soft norm.

Stats: 5 findings (from 7 raw, 5 after dedup) across 3 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, pattern-refactor. Reviewers failed: none. Body-only: 0. bugs/performance/conventions/maintainability/pattern-refactor all clean.

Event: COMMENT — no Critical/High ≥75. Clean structurally. Findings are robustness + test-coverage gaps.

Findings

Tests

1. Medium build_runtime_input never asserts trigger_poller wiring reaches RebornRuntimeInput (crates/ironclaw_reborn_cli/src/runtime/mod.rs:260-267, confidence 85) — anchor: :263

   • Per .claude/rules/testing.md Test-Through-the-Caller: trigger_poller_settings() is wired through .with_trigger_poller_settings() inside build_runtime_input_with_options(), but no caller-level test asserts [trigger_poller] enabled=true in config.toml propagates to runtime_input.trigger_poller.enabled. Silent drop in the wiring would pass all existing tests.
   • Fix: add build_runtime_input_maps_trigger_poller_enabled_config writing config.toml + asserting propagation.

2. Low Env ENABLED=false overriding config-enabled poller untested (kill-switch path) (crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:89-96, confidence 65) — anchor: :90

   • Existing tests cover env=true overriding config=false but not reverse. Deploy-time kill-switch path uncovered. The "0" | "false" => arm at lines 89-91 untested.

Security

3. Medium fires_per_tick + jitter fields accept unbounded u32/u64 (crates/ironclaw_reborn_config/src/config_file.rs:264-281, confidence 65) — anchor: crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:64

   • Typo footgun: 86400000 (ms instead of secs) suspends poller ~2.7 years; u64::MAX → effectively forever. fires_per_tick = u32::MAX attempts ~4B dispatches/tick. Boot validation only rejects 0/!=1.
   • Fix: cap fires_per_tick <= 1000, jitter_max_secs <= 3600 in trigger_poller_settings() with anyhow::bail!.

4. Low Raw env-var value echoed into startup error messages (crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:100-107, confidence 55) — anchor: :102

   • {raw:?} / {other:?} echoes full env-var content. Operator copy-paste error (credential into wrong env slot) would land secret in startup error logs / crash reporters. Hardening only.
   • Fix: truncate echoed value to ~64 chars before format.

Local patterns

5. Nit Parse test uses max_concurrent=3 silently (crates/ironclaw_reborn_config/src/config_file.rs:1284-1306, confidence 65) — anchor: :1290
   • trigger_poller_full_section_parses uses 3 to confirm config layer is pure deserializer; V1 invariant (must be 1) is CLI-layer. No inline comment explains intent. Reader could copy 3 into operator config and hit fatal boot error.
   • Fix: inline comment noting deliberate non-prod-valid choice, or change to 1 + add separate test for non-1 parse acceptance.

Other categories

• bugs ✅ clean — drop order on EnvGuards verified correct, env_lock + early-bail layers analyzed
• performance ✅ clean — startup cold-path only; test-env-race acknowledged inline in code
• conventions ✅ clean — pub(super) widening of optional_nonempty_env is intra-module idiomatic
• maintainability ✅ clean — two *_settings() functions don't yet meet threshold for shared "layered config merge" helper
• pattern-refactor ✅ empty — 6 if let Some(x) = ... field-apply repetitions are correct idiom for optional config

Mergeability

✅ Recommend merge once the Medium test gap (#1) is closed. Other findings are robustness improvements that can land in follow-ups.

[henrypark133]

Code Review Round 2 (8 reviewers, sonnet) — head 7bdeedc

Intent: R2 follow-up — cap trigger poller knobs at sane upper bounds + address R1 review feedback.

Round-1 findings — all RESOLVED ✅:

• ✅ R1 Med (conf 85) caller-level test → build_runtime_input_maps_trigger_poller_enabled_config added in mod.rs
• ✅ R1 Med (conf 65) unbounded knobs → MAX_POLL_INTERVAL_SECS=3600 / MAX_JITTER_SECS=3600 / MAX_FIRES_PER_TICK=1000 caps + max_concurrent != 1 strict-eq invariant
• ✅ R1 Low (conf 65) kill-switch test → env_disabled_overrides_config_enabled added
• ✅ R1 Low (conf 55) raw env-var echo → truncate_env_value_for_display (64-char cap) used in all error paths
• ✅ R1 Nit (conf 65) max_concurrent=3 parse test → inline comment explaining parse-layer vs CLI invariant added

Stats: 7 findings (from 10 raw, 7 after dedup) across 3 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, pattern-refactor. Reviewers failed: none. Body-only: 0. security/conventions/pattern-refactor all clean. No deferred-from-R1 items remaining.

Event: COMMENT — no Critical/High ≥75.

Findings

Cross-module test isolation (NEW, highest priority)

1. Medium Caller-level test in mod.rs races with trigger_poller::tests::TRIGGER_ENV_LOCK (crates/ironclaw_reborn_cli/src/runtime/mod.rs:794-854, confidence 82) — anchor: :820
   • New caller-level test mutates IRONCLAW_TRIGGER_POLLER_ENABLED + INTERVAL_SECS via inline EnvGuard without acquiring TRIGGER_ENV_LOCK (module-private to trigger_poller::tests). cargo test runs both modules in same binary in parallel → race against any of 11+ env-touching trigger_poller tests → non-deterministic CI.
   • Also flagged by maintainability/Low: same root cause — EnvGuard duplicated inline at mod.rs:798-819 instead of sharing with canonical EnvGuard in trigger_poller.rs.
   • Fix: expose lock_trigger_env() + EnvGuard as pub(super) from trigger_poller::tests, OR extract both into a #[cfg(test)] mod test_env at runtime/ level. Removes both race AND duplicated inline EnvGuard.

Bugs

2. Low Invalid-value error shows lowercased value, not what operator typed (crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:132-136, confidence 70) — anchor: :133
   • raw.to_ascii_lowercase() → other. truncate_env_value_for_display(other) → error shows lowercased form. Operator with ENABLED=YES sees got "yes", obscuring what to grep in secrets config.
   • Fix: pass &raw (original), not other.

Tests (boundary coverage)

3. Low At-cap boundary acceptance untested for poll_interval=3600 / fires_per_tick=1000 / jitter=3600 (crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:73-102, confidence 75) — anchor: :74

   • Strict > validation means exact cap MUST be accepted. Tests cover above-cap rejection + below-cap acceptance, never exact cap. Off-by-one > → >= regression silently invalidates documented maximum.
   • Fix: add 3 at-cap acceptance tests.

4. Low max_concurrent_fires_per_trigger=0 untested as rejection (crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:93-102, confidence 75) — anchor: :95

   • != 1 guard rejects 0 and 2+. Only 2 tested. 0 has distinct semantic meaning (unlimited concurrency in some systems) — explicit-rejection confirmation valuable.

5. Low Numeric ENABLED aliases '1' and '0' untested (crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:128-138, confidence 75) — anchor: :130

   • Match accepts '1' | 'true' / '0' | 'false'. All env tests use true/false. .env.example documents 1/0 as primary examples. Alias removal regression silently undetected.

6. Low truncate_env_value_for_display not tested with >64-char input (crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:28-37, confidence 50) — anchor: :28

   • Helper hardens against credential-sized values. Truncation … suffix + raw-value exclusion never asserted.

Local patterns

7. Nit Doc-comment cites internal module path runtime/trigger_poller.rs (crates/ironclaw_reborn_config/src/config_file.rs:267-269, confidence 55) — anchor: :268
   • Sibling section docs (WebuiSection, BudgetSection, RunnerSection) describe enforcement in terms of behavior, not internal file paths. Path reference rots silently on file move/rename.
   • Fix: replace with behavior-describing prose matching sibling style.

Other categories

• security ✅ clean — all R1 caps in place, truncate helper working as documented
• conventions ✅ clean — pub(super) widening + module split justified by architecture.md soft norm
• pattern-refactor ✅ empty — sequential if let Some(...) field-apply correct for distinct-typed config fields

Mergeability

✅ All R1 findings RESOLVED. No High/Critical remain.

Recommend before merge: close the cross-module test race (#1, Med conf 82) — same-binary tests with parallel cargo run will hit it. The lowercased-echo bug (#2) is one-line. Test gaps #3-#6 are valuable coverage but can land in follow-ups. #7 is a nit.

[serrrfirat]

Code Review (5 reviewers) — head ddc540f

Intent: Expose Reborn trigger-poller config/env controls while keeping the shipped default off and enforcing startup invariants.

Stats: 4 findings after aggregation across 4 files. Reviewers run: security, bugs, performance/concurrency, tests, conventions. Reviewers failed: none. Security and performance/concurrency returned clean.

Event: COMMENT — no Critical/High findings.

Findings

Bugs

1. Medium Blank trigger-poller env vars are silently ignored (confidence 85) — crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:128
   • optional_nonempty_env() trims and filters empty values, so explicitly setting IRONCLAW_TRIGGER_POLLER_ENABLED= or IRONCLAW_TRIGGER_POLLER_INTERVAL_SECS= behaves as if the env var is absent. These env vars are documented as strict overrides where invalid values are fatal, and ignoring blank values can also defeat env precedence over a config-enabled poller.
   • Fix: read these strict vars as present-or-absent with std::env::var, trim only for parsing, and reject empty/whitespace values through the same invalid-value path.

Tests

2. Medium Missing caller-level coverage for trigger-poller env overrides (confidence 85) — crates/ironclaw_reborn_cli/src/runtime/mod.rs:265
   • Helper tests cover env/config precedence and the caller-level test covers config-only propagation, but no caller-level test proves IRONCLAW_TRIGGER_POLLER_* env overrides survive build_runtime_input_with_options() and reach RebornRuntimeInput. Per the repo's test-through-the-caller rule, this wiring gates runtime worker startup and should be covered at the call site.
   • Fix: add build_runtime_input_env_trigger_poller_overrides_config in runtime::tests, holding lock_trigger_env(), setting both env vars, calling build_runtime_input, and asserting enabled=true plus the env interval.

Conventions

3. Low Trigger-poller config docs describe a nonexistent CLI precedence layer (confidence 85) — crates/ironclaw_reborn_config/src/config_file.rs:259

   • The public doc comment says CLI flags override env vars, but this PR implements only env > config > compiled defaults and adds no trigger-poller CLI flags. That makes the exported config contract stale.
   • Fix: remove the CLI-flags sentence unless this PR wires an actual higher-precedence CLI layer.

4. Low Runtime boundary guard only scans one directory level (confidence 65) — crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs:243

   • The new boundary-test comment says it scans every .rs file under the runtime module, but std::fs::read_dir(&runtime_dir) only visits immediate children. A future nested runtime module could bypass the forbidden-import string guard.
   • Fix: make the scan recursive, or assert that runtime/ has no nested directories so the guardrail matches its documented coverage.

Validation

• Passed: cargo test -p ironclaw_architecture reborn_cli_binary_crate_stays_separate_from_v1_root
• Attempted: cargo test -p ironclaw_reborn_cli -p ironclaw_reborn_config, but the local machine ran out of disk space while compiling dependencies (No space left on device), before test execution completed.

[serrrfirat]

Thermo-Nuclear Code Quality Review — head ddc540f

I focused this pass on structural maintainability: file-size pressure, abstraction quality, spaghetti growth, and whether the new trigger-poller config path fits the existing Reborn config/runtime boundaries.

Result: 1 high-signal maintainability finding. I am not repeating the regular review's lower-level test/docs/boundary comments here.

Finding

1. Medium The strict trigger-poller env parser is built on the wrong abstraction — crates/ironclaw_reborn_cli/src/runtime/mod.rs:433 / crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:128

optional_nonempty_env() is an OAuth-style helper: trim, collapse blank to absent, and keep optional configuration easy. Exporting it with pub(super) so trigger_poller_settings() can consume strict operator controls makes the new module inherit the wrong model. For these env vars, “present but blank” is not “not configured”; it is an invalid override that should fail loudly. The helper hides that distinction, so the code now has to rely on comments and tests to remember which env vars are optional and which are strict.

Can we keep optional_nonempty_env private to the optional-config/OAuth path and give trigger poller its own small strict-env reader instead? A clean shape would be something like read_trigger_env(name) -> anyhow::Result<Option<String>> that preserves presence, trims only for parsing, and rejects empty/whitespace in one place. Then the enabled and interval branches express the real invariant directly, instead of sharing a helper whose name and behavior encode the opposite contract.

Structure Check

• runtime/mod.rs stays below 1k lines: 876 lines at base, 925 at head.
• New runtime/trigger_poller.rs is 612 lines, under the repo's new-file target; most of the size is focused tests, so I do not see a file-size blocker.
• The direct default → section → env merge shape matches nearby runner_settings; a heavier reusable config resolver would be over-abstracting for this PR.

[abbyshekit]

Code review — PR18.6: enable trigger poller via config + env

Multi-agent review (security · bugs · performance · tests · conventions) at ddc540f7. Diff-only mode.

Independent review of an open PR.

3 findings — 3 Low. Posted as a comment (advisory). Confidence ≥ 50, deduplicated across reviewers.

Sev	Conf	Reviewer	Location	Finding
Low	70%	conventions	reborn_dependency_boundaries.rs:237	Boundary scan over runtime/ is non-recursive, contradicting its own comment and diverging from the four recursive walkers in this same file
Low	55%	tests	trigger_poller.rs:128	Empty/whitespace env-var branch is untested (could silently regress into a fatal startup bail)
Low	50%	tests	mod.rs:798	Env-override precedence has no caller-level (build_runtime_input) test; only the config-file path is exercised through the caller

---

_{Generated by near-ai-code-review (5 parallel reviewer agents + intent analysis). Diff-only; confidence ≥ 50; ≤ 15 inline comments.}

[henrypark133]

Code Review Round 4 (8 reviewers, sonnet) — head fef95a1

Intent: R4 follow-up — address R2 + R3 review feedback. Extract shared runtime/test_env.rs to fix cross-module race; bug fixes; boundary coverage.

Prior round findings — ALL RESOLVED ✅:

R2 (my last round):

• ✅ R2 Med (82) cross-module race → new runtime/test_env.rs shared module; both mod.rs::tests and trigger_poller::tests use lock_trigger_env() + shared EnvGuard
• ✅ R2 Low (70) lowercase echo → now passes &raw (original case)
• ✅ R2 Low (75) at-cap acceptance → 3 *_at_cap_is_accepted tests added
• ✅ R2 Low (75) max_concurrent=0 → test added
• ✅ R2 Low (75) numeric ENABLED 1/0 → 2 tests added
• ✅ R2 Low (50) truncation >64-char → test added (80 x's, assert ellipsis + raw exclusion)
• ✅ R2 Nit (55) doc-comment internal path → reworded

R3 (non-me, head ddc540f):

• ✅ R3 Low (70) conventions non-recursive boundary scan → collect_runtime_rs now recursive
• ✅ R3 Low (55) tests empty/whitespace env branch → 3 tests added (empty + whitespace + interval-empty)
• ✅ R3 Low (50) tests env-override caller-level → 2 new caller tests in mod.rs

Stats: 5 findings (from 5 raw, 5 after dedup) across 4 files. Reviewers run: security, bugs, performance, tests, conventions, local-patterns, maintainability, pattern-refactor. Reviewers failed: none. Body-only: 0. security/performance/conventions/maintainability/pattern-refactor all clean.

Event: COMMENT — no Critical/High ≥75. Findings are test-coverage gaps + style nits.

Findings

Bugs / Tests (Medium — close before merge)

1. Medium Three at-cap tests reach env layer without acquiring TRIGGER_ENV_LOCK (crates/ironclaw_reborn_cli/src/runtime/trigger_poller.rs:529-570, confidence 80) — anchor: :529

   • *_at_cap_is_accepted tests pass config validation and proceed into env layer without lock or env-clear. Concurrent test setting INTERVAL_SECS=45 → at-cap test reads 45 instead of 3600 → flaky failure. Or ENABLED=yes → expect() panics.
   • Fix: add let _lock = lock_trigger_env(); _e/_i = EnvGuard::clear(...) at top of each.

2. Medium build_runtime_input invalid-env error propagation untested at caller boundary (crates/ironclaw_reborn_cli/src/runtime/mod.rs:262-265, confidence 75) — anchor: :262

   • trigger_poller_settings() is ?-propagated in build_runtime_input_with_options. Per .claude/rules/testing.md Test-Through-the-Caller, error path needs caller-level coverage. Existing caller tests cover happy-path + override-wins; none drives ENABLED=yes through build_runtime_input and asserts Err propagation.

Local patterns

3. Low collect_runtime_rs nested inside test body; sibling walkers are module-level (crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs:243-267, confidence 75) — anchor: :243

   • Only nested-fn pattern in 2500-line file. Sibling walkers (collect_forbidden_*, extract_pub_use_block) all at module level. Hoist to match pattern.

4. Nit Doc-comment on optional_nonempty_env cites private rust-path that won't rustdoc-link (crates/ironclaw_reborn_cli/src/runtime/mod.rs:433-445, confidence 55) — anchor: :438

   • Cites runtime::trigger_poller::strict_env_var — also private. Path neither rustdoc-linkable nor externally navigable. Replace with prose or drop.

5. Nit Redundant #![cfg(test)] inner attribute in test_env.rs (crates/ironclaw_reborn_cli/src/runtime/test_env.rs:1-8, confidence 50) — anchor: :1

   • mod.rs already gates with #[cfg(test)] mod test_env;. Inner attr is no-op + confusing about authoritative gate. Drop the inner attribute.

Other categories

• security ✅ clean — all R2 hardening preserved (truncate helper, original-case raw, caps)
• performance ✅ clean — config-only tests that skip lock all bail before env reads
• conventions ✅ clean — module split + visibility correct; no string fields means trigger_poller intentionally skipped in validate()'s secrets_guard sweep
• maintainability ✅ clean
• pattern-refactor ✅ empty

Mergeability

✅ All R2 + R3 findings RESOLVED. CI mergeable_state=clean.

Recommend before merge: close the 2 Mediums (#1 lock-missing in at-cap tests, #2 caller-level error-propagation test). Both are low-effort. The 3 Lows/Nits = follow-up.