feat: add webhook-triggered pull request review workflow

[geoffjay]

feat: add webhook-triggered pull request review workflow

feat(workflows): add webhook-triggered pull request review workflow (closes #459)

Add webhook-pull-request-reviewer.yml — a new workflow that triggers
immediately when GitHub delivers a pull_request webhook event rather than
polling on a 60-second interval. Key features:

• webhook trigger type with optional HMAC-SHA256 secret verification
• guard clause: only processes labeled events with the review-agent label
• memory integration: queries shared reviewer memory before each review
• isolated git worktree: checks out the PR branch under /tmp/agentd-review-<pr_number>
  so the review does not disturb the main working tree, then cleans up afterward
• same label transition logic as the polling reviewer (merge-queue, needs-rework,
  needs-restack) with review-agent label removal on completion
• ships as enabled: false; requires GitHub webhook configuration before enabling

Update onboarding.md and pipeline-state-machine.md to document both the
polling and webhook reviewer variants.

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com

[geoffjay]

This change is part of the following stack:

• feat: add webhook-triggered pull request review workflow #747 ◀

_{Change managed by git-spice.}

[geoffjay]

Review: feat(workflows): webhook-triggered pull request review workflow

Stack position: issue-459 sits directly on main — no parent PR dependency.

The overall structure is good: the header comment is thorough, the enabled: false default is correct, and the prompt template covers all the important steps (memory query, stack check, worktree isolation, label transitions). One blocking issue prevents the workflow from being deployed at all.

---

Blocking — must fix

Workflow cannot be applied: 4 template variables are unknown to the validator

scheduler/api.rs calls validate_template() on registration and treats any
"Unknown template variable" warning as an error that rejects the
create_workflow request (returns HTTP 4xx):

// scheduler/api.rs:49-59
let warnings = validate_template(&req.prompt_template);
let errors: Vec<&String> = warnings
    .iter()
    .filter(|w| w.contains("Unknown template variable") || ...)
    .collect();
if !errors.is_empty() {
    return Err(ApiError::InvalidInput(...));
}

The new template uses four variables that are not in KNOWN_VARIABLES
(crates/orchestrator/src/scheduler/template.rs):

Template variable	Populated by parse_webhook_payload?	In KNOWN_VARIABLES?
{{github_event}}	Yes — metadata["github_event"]	No
{{action}}	Yes — metadata["action"] for PR/issue events	No
{{delivery_id}}	Yes — metadata["delivery_id"] always	No
{{pr_number}}	Yes — metadata["pr_number"] for PR events	No

Running agent apply .agentd/workflows/webhook-pull-request-reviewer.yml will
fail with:

Invalid prompt template: Unknown template variable '{{github_event}}';
  Unknown template variable '{{action}}'; Unknown template variable
  '{{delivery_id}}'; Unknown template variable '{{pr_number}}'

The architect analysis for issue #459 (stored in shared memory) explicitly
called this out as the required Rust change. This PR needs to extend
KNOWN_VARIABLES with the four webhook-specific metadata keys:

// scheduler/template.rs — add a new section:
// Metadata-backed (webhook triggers)
"github_event",
"action",
"delivery_id",
"pr_number",

The doc-comment table in template.rs should also be updated to document these
alongside the cron / delay / dispatch_result entries.

---

Non-blocking suggestions

gh pr checkout --worktree is not a valid flag

Step 3 of the prompt template:

gh pr checkout {{pr_number}} --repo geoffjay/agentd --worktree "$REVIEW_DIR" || \
  git worktree add "$REVIEW_DIR" "origin/$(gh pr view ...)"

gh pr checkout does not accept a --worktree flag — the primary command will
always fail, and the agent always falls through to the git worktree add
fallback. The fallback itself is correct. Consider dropping the primary attempt
and using only the git worktree add form:

BRANCH=$(gh pr view {{pr_number}} --repo geoffjay/agentd --json headRefName --jq '.headRefName')
git worktree add "$REVIEW_DIR" "origin/$BRANCH"

secret: "" placeholder is misleading

# secret: ""  # Set to a random string...

An empty string is not a valid HMAC secret — GitHub's webhook delivery will
fail signature verification if an empty value is used. The commented placeholder
would be less confusing as:

# secret: "replace-with-a-random-string"

All pull_request events trigger an agent invocation

The guard is entirely prompt-level ("if {{action}} is not labeled... skip").
Every pull_request event (closed, merged, synchronize, assigned, etc.) fires an
agent that reads the prompt, evaluates the guard, and exits — consuming an
invocation for each. The architect noted that adding an actions filter to
TriggerConfig::Webhook was the proper fix but was deferred to a follow-up.
Worth a tracking issue if one does not already exist.

---

Minor

• The agent orchestrator list-workflows command in the setup instructions — double-check
  this is the actual CLI subcommand name (I see agent wraps various services;
  if the command differs, operators will be confused on first run).
• The {{pr_number}} placeholder in the review step commands (gh pr diff {{pr_number}}) will render literally as {{pr_number}} if the webhook
  payload omits the PR number field. Adding a note to the header that the
  workflow requires a properly-formed pull_request payload would help
  operators debug unexpected failures.

[geoffjay]

Review: feat: add webhook-triggered pull request review workflow

Stack Position

This branch (issue-459) is stacked on main but is currently behind main — git-spice shows (needs restack). This is the primary issue to address before this PR can merge.

---

Needs Restack: template.rs Changes Already on Main

The diff includes additions to crates/orchestrator/src/scheduler/template.rs — specifically adding github_event, action, delivery_id, and pr_number to KNOWN_VARIABLES. These variables are already present on main (8 occurrences as of the current trunk), with a richer doc comment than this PR adds. After a rebase, the template.rs hunk will either conflict or vanish cleanly, leaving only the workflow YAML and any doc changes as the actual diff.

Please run git-spice branch restack (or git rebase main) before requesting re-review.

---

Workflow YAML Review (.agentd/workflows/webhook-pull-request-reviewer.yml)

The workflow itself is the real new content here and it looks solid overall.

Strengths:

• The guard clause at the top of prompt_template (if action not in [...]) prevents spurious fires on non-review-ready events
• Worktree isolation via /tmp/agentd-review-{{pr_number}} is the right pattern for review agents
• Memory integration steps (search before, remember after) are well-placed
• enabled: false is the correct default for a new workflow — forces a deliberate opt-in

Non-blocking observations:

1. Hardcoded repo name: The workflow contains --repo geoffjay/agentd in several gh invocations inside prompt_template. If this repo is ever forked or the workflow is reused, those will silently target the wrong repo. Consider templating this from a workflow variable or pulling it from the webhook payload (repository.full_name).

2. Missing --approve fallback path: The reviewer prompt instructs the agent to use gh pr review ... --approve on clean PRs. For first-party repos where the review agent cannot approve its own PRs, this will silently fail (or error). The prompt could add a fallback: attempt --approve; if it fails with "cannot approve own PR", fall back to --comment + add merge-queue label directly. This is a pattern already discovered in practice.

3. HMAC verification is optional: The secret field is present but the scheduler currently makes HMAC-SHA256 verification optional when no secret is configured. If the workflow is enabled without setting a secret, any HTTP client that knows the endpoint URL can trigger a review. Worth noting in the workflow's comment header so operators know to set the secret before enabling in production.

4. delivery_id usage: delivery_id is captured in the trigger variables but not referenced in the workflow steps. Either use it (e.g., for idempotency — skip if this delivery was already processed) or remove it from the trigger variable list to keep the interface minimal.

---

Summary

The branch needs a restack before it can be evaluated cleanly — the template.rs diff is stale relative to main. The workflow YAML itself is well-structured and ready to merge once the restack is done. Four non-blocking observations above; none require code changes before merging.

Adding needs-restack. Please restack and re-submit with --label review-agent.