feat: Make triggers opt-in and fix inject submission

[2witstudios]

Summary

• Triggers no longer auto-bind on spawn — users must explicitly opt in via pu spawn --trigger <name> or pu trigger assign <agent_id> <name> post-hoc
• New Request::AssignTrigger / Response::AssignTriggerResult protocol types with round-trip tests
• Switches inject_text from raw write_to_fd to write_chunked_submit, fixing the race where pasted trigger text + Enter was being swallowed by the TUI
• --trigger and --no-trigger flags are mutually exclusive (clap conflicts_with)
• pu trigger assign validates agent exists before updating manifest (returns NOT_FOUND error instead of silent no-op)

Test plan

• cargo check passes for pu-core, pu-engine, pu-cli
• All tests pass across all three crates
• cargo clippy --all-targets clean with -D warnings
• cargo fmt --check passes
• Manual: pu spawn "task" --trigger <name> binds the trigger
• Manual: pu spawn "task" without --trigger does NOT auto-bind
• Manual: pu trigger assign <agent_id> <name> binds to an existing idle agent
• Manual: pu trigger assign <agent_id> <bad-name> returns NOT_FOUND error
• Manual: trigger inject actions actually submit (not just paste) into the agent

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Warning

Rate limit exceeded

@2witstudios has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 30 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 82912d08-b30e-40f5-85d9-129d215d23aa

📥 Commits

Reviewing files that changed from the base of the PR and between e364e23 and f700027.

📒 Files selected for processing (3)

• crates/pu-cli/src/main.rs
• crates/pu-core/src/protocol.rs
• crates/pu-engine/src/engine.rs

📝 Walkthrough

Walkthrough

The PR extends spawn functionality with optional trigger binding. A new trigger parameter is added to spawn requests and threaded through the CLI, protocol, and engine layers. A new AssignTrigger request variant enables explicit trigger assignment to agents at the protocol level. The engine implements binding logic that validates trigger existence, checks sequence non-emptiness, and updates agent manifests with trigger metadata (name, state, indices, gate attempts).

Changes

Cohort / File(s)	Summary
Protocol Extensions crates/pu-core/src/protocol.rs	Adds Request::AssignTrigger variant with project_root, agent_id, trigger_name fields; adds optional trigger: Option<String> field to Request::Spawn; introduces Response::AssignTriggerResult variant with agent_id, trigger_name, sequence_len; updates round-trip tests for new variants.
Engine Implementation crates/pu-engine/src/engine.rs	Implements handle_assign_trigger method to locate, validate, and bind triggers to agents; adds trigger: Option<String> field to SpawnParams; routes Request::AssignTrigger to handler; propagates trigger through spawn paths; adjusts text injection to omit unconditional newline.
CLI Command Handlers crates/pu-cli/src/commands/spawn.rs, crates/pu-cli/src/commands/trigger.rs	Threads trigger: Option<String> parameter through spawn command; adds new run_assign function to handle trigger assignment requests with JSON output support.
CLI Main & Routing crates/pu-cli/src/main.rs	Adds --trigger long flag to Spawn command; introduces TriggerAction::Assign variant; routes Assign action to commands::trigger::run_assign; propagates trigger parameter through spawn handling.
CLI Output crates/pu-cli/src/output.rs	Adds match arm for Response::AssignTriggerResult to print: "Assigned trigger {trigger_name} to agent {agent_id} ({sequence_len} steps)".
Test Helpers crates/pu-engine/src/test_helpers.rs	Updates test spawn construction to include trigger: None field.

Sequence Diagram

sequenceDiagram
    actor User
    participant CLI
    participant Daemon
    participant Engine
    participant Manifest

    User->>CLI: spawn --trigger "trigger-name"
    CLI->>Daemon: Request::Spawn { trigger_name: Some(...) }
    Daemon->>Engine: handle_spawn
    Engine->>Manifest: spawn agent
    Manifest-->>Engine: agent_id
    Engine->>Engine: if trigger provided: bind_trigger
    Engine->>Manifest: update with trigger metadata
    Manifest-->>Engine: success
    Engine-->>Daemon: Response::SpawnResult
    Daemon-->>CLI: SpawnResult
    CLI-->>User: agent created with trigger bound

    User->>CLI: trigger assign --agent-id X --trigger-name Y
    CLI->>Daemon: Request::AssignTrigger
    Daemon->>Engine: handle_assign_trigger
    Engine->>Manifest: locate trigger by name
    Manifest-->>Engine: trigger data
    Engine->>Manifest: validate & update agent binding
    Manifest-->>Engine: success
    Engine-->>Daemon: Response::AssignTriggerResult
    Daemon-->>CLI: AssignTriggerResult
    CLI-->>User: trigger assigned

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: Event-driven trigger system for agent workflows #121: Modifies trigger-related spawn behavior and Request::Spawn handling across protocol and engine spawn paths with complementary trigger-binding semantics.
• feat: Prompt-based plan mode for spawned agents #122: Updates spawn request path to thread additional optional spawn parameters (plan_mode) through CLI, protocol, and engine layers following a similar pattern.
• feat: Add worktree support for scheduled agents with validation #90: Modifies Request::Spawn variant at the protocol level to add optional fields, establishing precedent for extensible spawn API design.

Poem

🐰 A trigger here, a trigger there,
Binding agents with utmost care,
Through spawn and assign, we weave it all,
Manifest dancing to trigger's call! 🎯

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 69.23% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the two main objectives: making triggers opt-in (adding --trigger flag and new assign command) and fixing inject submission (write_chunked_submit change).
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/trigger-opt-in

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (1)

crates/pu-cli/src/output.rs (1)

256-267: Add a smoke test for this new response arm.

This branch is the only new human-output path in the file, but there isn’t a matching print_response smoke test alongside the other response variants below. A small AssignTriggerResult case would keep this match arm from regressing unnoticed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/pu-cli/src/output.rs` around lines 256 - 267, Add a smoke test that
covers the new Response::AssignTriggerResult arm by calling the same test helper
used for other response variants (the print_response test harness) and passing a
Response::AssignTriggerResult with example agent_id, trigger_name, and
sequence_len; assert that the stdout contains the expected human-readable string
(e.g., trigger_name and agent_id) so the match arm in print_response (or the
Response::AssignTriggerResult handling in output.rs) is exercised and cannot
regress unnoticed.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/pu-cli/src/main.rs`:
- Around line 65-70: The two CLI flags are allowed together but should be
mutually exclusive; update the struct field attributes for `no_trigger` and
`trigger` (the `no_trigger: bool` and `trigger: Option<String>` fields in
main.rs) to use clap's conflicts_with so the parser rejects combinations—e.g.
add `conflicts_with = "no_trigger"` to the `trigger` arg (or symmetric
`conflicts_with = "trigger"` on `no_trigger`) so supplying both `--trigger` and
`--no-trigger` fails at parse time.

In `@crates/pu-core/src/protocol.rs`:
- Around line 132-136: The protocol enum has new variants AssignTrigger and
AssignTriggerResult but PROTOCOL_VERSION was not incremented; update the
PROTOCOL_VERSION constant to the next integer and adjust the associated version
assertion test expectation to match the new value (update the test that asserts
PROTOCOL_VERSION to the new number). Ensure both the constant named
PROTOCOL_VERSION and the test that checks the protocol version are changed in
the same commit so the runtime and tests remain consistent.

In `@crates/pu-engine/src/engine.rs`:
- Around line 1166-1200: The current branch that handles an explicit trigger
(when trigger_param is Some) silently logs and proceeds with (None, None) if the
named trigger is missing or has an empty sequence; instead, make this path
return an error consistent with handle_assign_trigger so that `--trigger <name>`
failures are surfaced. Locate the block that computes (trigger_name,
trigger_total) using trigger_param/no_trigger/project_root and change the match
arms for None and Some(_) where total == 0 to return an Err with a clear message
(e.g., "trigger '<name>' not found" or "trigger '<name>' has empty sequence")
rather than logging and falling back; ensure the calling function’s
signature/flow propagates that Result error (or convert to the same error path
used by handle_assign_trigger) so the CLI command fails on invalid trigger
names.
- Around line 246-253: Add Request::AssignTrigger to the project registration
switch so projects modified by trigger assignment are registered for scanning:
update the match that registers projects (the register_project branch) to
include the Request::AssignTrigger arm (same way as other project-scoped
requests), ensuring that when handle_assign_trigger(&project_root, &agent_id,
&trigger_name) is dispatched the project_root is also passed into the project
registration logic so scheduler_tick() will scan the project after persistently
assigning a trigger.
- Around line 1589-1606: Clippy flags uninlined format args in the
Response::Error messages—replace inline-captured identifiers in the format!
calls with explicit format placeholders and arguments; for example, change
format!("trigger lookup failed: {e}") to format!("trigger lookup failed: {}", e)
and ensure any uses like format!("trigger '{}' not found", trigger_name) or
format!("trigger '{}' has empty sequence", trigger_name) use "{}" placeholders
with trigger_name passed as an argument (references: Response::Error
construction, trigger_name, variable e, and sequence_len derived from
trigger.sequence.len()).

---

Nitpick comments:
In `@crates/pu-cli/src/output.rs`:
- Around line 256-267: Add a smoke test that covers the new
Response::AssignTriggerResult arm by calling the same test helper used for other
response variants (the print_response test harness) and passing a
Response::AssignTriggerResult with example agent_id, trigger_name, and
sequence_len; assert that the stdout contains the expected human-readable string
(e.g., trigger_name and agent_id) so the match arm in print_response (or the
Response::AssignTriggerResult handling in output.rs) is exercised and cannot
regress unnoticed.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0ed27be0-583b-4cb9-b77f-c13e563f7e7e

📥 Commits

Reviewing files that changed from the base of the PR and between 2a876e0 and e364e23.

📒 Files selected for processing (7)

• crates/pu-cli/src/commands/spawn.rs
• crates/pu-cli/src/commands/trigger.rs
• crates/pu-cli/src/main.rs
• crates/pu-cli/src/output.rs
• crates/pu-core/src/protocol.rs
• crates/pu-engine/src/engine.rs
• crates/pu-engine/src/test_helpers.rs