feat(flows-yaml): port CI events to YAML (ci_run trigger + pr-ci.yml + pr-opened.yml rewrite) #1097

Merged

reviewer merged 2 commits from feat/flows-yaml-ci-events into main 2026-05-11 00:22:21 +00:00

Conversation 1 Commits 2 Files changed 38 +2717 -286

claude-desktop commented 2026-05-11 00:01:00 +00:00

Collaborator

Copy link

Summary

Closes #1092. Restores the silent fix-ci + reviewer-request paths that PR #1090 surfaced as broken.

Surface ci_run (Forgejo status + action_run_*, GitHub workflow_run, GitLab pipeline) as a YAML trigger and replace the orphaned imperative handlers with flows.

Trigger surface

• New CiRunTrigger in flows-yaml/trigger-events.ts; "ci_run" added to FORGE_TRIGGER_KINDS
• webhook-adapter.ts:185 converts the wire-shaped ForgeEvent into CiRunTrigger (was dropping it on the floor)
• schema.ts parses on: { ci_run: [success, failure] }

Capability + ops

• New PostCiCapability in flows-yaml/types.ts; live + shadow bags wire it; LiveCapabilitiesDeps extended with postCi
• 7 new ops wrapping helpers from domain/workflow/post-ci.ts:
  repo_has_workflows, arm_review_fallback, cancel_review_fallback, find_open_pr_for_head, fetch_aggregate_ci_state, dispatch_fix_ci, decide_post_ci (composite — keeps decision tree atomic)
• main.ts wires the postCi capability slot via findOpenPRForHead + renderPrompt (fix-ci skill) + decidePostCiAction

Flows

• pr-opened.yml rewritten: drop the area:agents gate that silently skipped PR #1090; arm the 180 s CI fallback timer instead of dispatching the reviewer immediately
• New pr-ci.yml: cancel fallback → find PR → dispatch fix-ci (red) or aggregate-confirmed decide_post_ci (green)

Cleanup

• Delete orphaned handlePullRequestOpened + handleCheckSuiteCompleted from event-handlers.ts (no non-test callers)
• Delete their tests
• Regenerated flows.schema.json (22 ops now)

Test plan

• All 7 new op tests pass
• New pr-ci.yml flow test covers success + failure + no-PR + pending-aggregate paths
• pr-opened.yml test rewritten for new flow shape; covers no-workflows shortcut + workflows-present arming + the area:agents gate removal (#1090 regression test)
• Full server suite: 3240 pass / 0 fail
• bun run qa clean (typecheck + biome + tests)
• CI green on this PR

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

## Summary Closes #1092. Restores the silent fix-ci + reviewer-request paths that PR #1090 surfaced as broken. Surface `ci_run` (Forgejo `status` + `action_run_*`, GitHub `workflow_run`, GitLab pipeline) as a YAML trigger and replace the orphaned imperative handlers with flows. ### Trigger surface - New `CiRunTrigger` in `flows-yaml/trigger-events.ts`; `"ci_run"` added to `FORGE_TRIGGER_KINDS` - `webhook-adapter.ts:185` converts the wire-shaped `ForgeEvent` into `CiRunTrigger` (was dropping it on the floor) - `schema.ts` parses `on: { ci_run: [success, failure] }` ### Capability + ops - New `PostCiCapability` in `flows-yaml/types.ts`; live + shadow bags wire it; `LiveCapabilitiesDeps` extended with `postCi` - 7 new ops wrapping helpers from `domain/workflow/post-ci.ts`: `repo_has_workflows`, `arm_review_fallback`, `cancel_review_fallback`, `find_open_pr_for_head`, `fetch_aggregate_ci_state`, `dispatch_fix_ci`, `decide_post_ci` (composite — keeps decision tree atomic) - `main.ts` wires the `postCi` capability slot via `findOpenPRForHead` + `renderPrompt` (fix-ci skill) + `decidePostCiAction` ### Flows - `pr-opened.yml` rewritten: drop the `area:agents` gate that silently skipped PR #1090; arm the 180 s CI fallback timer instead of dispatching the reviewer immediately - New `pr-ci.yml`: cancel fallback → find PR → dispatch fix-ci (red) or aggregate-confirmed `decide_post_ci` (green) ### Cleanup - Delete orphaned `handlePullRequestOpened` + `handleCheckSuiteCompleted` from `event-handlers.ts` (no non-test callers) - Delete their tests - Regenerated `flows.schema.json` (22 ops now) ## Test plan - [x] All 7 new op tests pass - [x] New `pr-ci.yml` flow test covers success + failure + no-PR + pending-aggregate paths - [x] `pr-opened.yml` test rewritten for new flow shape; covers no-workflows shortcut + workflows-present arming + the `area:agents` gate removal (#1090 regression test) - [x] Full server suite: 3240 pass / 0 fail - [x] `bun run qa` clean (typecheck + biome + tests) - [ ] CI green on this PR Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;

claude-desktop added 1 commit 2026-05-11 00:01:00 +00:00

feat(flows-yaml): port CI events to YAML (ci_run trigger + pr-ci.yml + pr-opened.yml rewrite) ... 19052c1499

Closes #1092.

Surface `ci_run` (Forgejo `status` + `action_run_*`, GitHub `workflow_run`,
GitLab pipeline) as a YAML trigger and replace the orphaned imperative
handlers with flows. Restores the silent fix-ci + reviewer-request paths
that PR #1090 surfaced as broken.

Trigger surface:
- new `CiRunTrigger` in `flows-yaml/trigger-events.ts`; `"ci_run"` added to
  `FORGE_TRIGGER_KINDS`
- `webhook-adapter.ts:185` converts the wire-shaped `ForgeEvent` into
  `CiRunTrigger` (was dropping it on the floor)
- `schema.ts` parses `on: { ci_run: [success, failure] }`

Capability + ops:
- new `PostCiCapability` in `flows-yaml/types.ts`; live + shadow bags
  wire it; `LiveCapabilitiesDeps` extended with `postCi`
- 7 new ops wrapping helpers from `domain/workflow/post-ci.ts`:
  `repo_has_workflows`, `arm_review_fallback`, `cancel_review_fallback`,
  `find_open_pr_for_head`, `fetch_aggregate_ci_state`, `dispatch_fix_ci`,
  `decide_post_ci` (composite — keeps decision tree atomic)
- `main.ts` wires the `postCi` capability slot via `findOpenPRForHead` +
  `renderPrompt` (fix-ci skill) + `decidePostCiAction`

Flows:
- `pr-opened.yml` rewritten: drop the `area:agents` gate that silently
  skipped PR #1090; arm the 180 s CI fallback timer instead of
  dispatching the reviewer immediately
- new `pr-ci.yml`: cancel fallback → find PR → dispatch fix-ci (red) or
  aggregate-confirmed `decide_post_ci` (green)

Cleanup:
- delete orphaned `handlePullRequestOpened` + `handleCheckSuiteCompleted`
  from `event-handlers.ts` (no non-test callers)
- delete their tests
- regenerated `flows.schema.json` (22 ops now)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

charles referenced this pull request from a commit 2026-05-11 00:11:39 +00:00

fix(flows-yaml): thread branch hints + null-headSha guard (review feedback for #1097)

charles added 1 commit 2026-05-11 00:11:39 +00:00

fix(flows-yaml): thread branch hints + null-headSha guard (review feedback for #1097) ... 6d4d698b90

- Gate `pr-opened.yml` flow-level `if:` on `event.pr.headSha != null` so
  events with no head SHA skip the flow entirely (legacy
  `handlePullRequestOpened` bailed early on `!pr.head?.sha`; the op-level
  zod schemas require a non-empty string).
- Thread `branches: readonly string[]` through `PostCiCapability.dispatchFixCi`
  + `decidePostCi` and the underlying `LiveCapabilitiesDeps`. `pr-ci.yml`
  forwards `event.branches` to both ops so a force-push race that breaks
  SHA→PR resolution still recovers via branch-name match (parity with
  legacy `handleCheckSuiteCompleted`).
- Update shadow caps, recording test helpers, op tests, op argsSchemas.
- Regenerate `flows.schema.json` (op schemas changed).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

claude-desktop requested review from reviewer 2026-05-11 00:19:24 +00:00

reviewer approved these changes 2026-05-11 00:22:06 +00:00

reviewer left a comment

Copy link

Review — PR #1097: feat(flows-yaml): port CI events to YAML

CI: ✅ green
Verdict: APPROVE

What I read

Full diff reviewed end-to-end: trigger surface, webhook adapter, 7 new ops + tests, both flow YAML files, capability wiring in main.ts, shadow/live capability builders, and all test helpers.

---

What's well done

webhook-adapter.ts — ci_run mapping is correct.
pending is dropped, error is collapsed onto failure, yielding a clean two-state (success | failure) surface for YAML authors. Test coverage for all four wire states is solid.

pr-opened.yml rewrite is the right fix for #1090.
The area:agents gate was the root cause — any PR without that label was silently skipped forever. The replacement !author_is('reviewer', 'design-reviewer') && event.pr.headSha != null is minimal and correct. The headSha null guard mirrors the legacy if (!sha) return; bail-out.

pr-ci.yml flow logic is sound.
cancel_review_fallback unconditionally fires first (right — you want to cancel the timer even when no PR is found). The aggregate-state guard on the success path mirrors the legacy handleCheckSuiteCompleted exactly. The "pending aggregate → no-op" path is tested.

All 7 new ops follow the established pattern cleanly — thin adapters delegating to the postCi capability, proper Zod schemas, deps: ["postCi"] as const. Op-level unit tests are thorough (schema rejection, happy path, branch-hint forwarding, null defaults).

Deletions are clean. handlePullRequestOpened + handleCheckSuiteCompleted + their tests are gone with no dangling callers. The isReviewerAgent import is correctly dropped from event-handlers.ts.

---

One thing to note (not a blocker)

Double findOpenPRForHead call per CI event.
In main.ts, both dispatchFixCi and decidePostCi capability impls silently ignore their _prNumber parameter and do a fresh findOpenPRForHead lookup:

// dispatch_fix_ci op → capability → main.ts adapter
dispatchFixCi: async (repo, _prNumber, headSha, runId, workflowId, branches) => {
    // _prNumber is discarded; re-fetches PR by SHA
    const pr = await findOpenPRForHead(repo, headSha, branchHints, token);

The flow already resolved the PR number in find_open_pr_for_head; the capability then does a second full API probe because domain/workflow/post-ci.ts::dispatchFixCi / decidePostCiAction need the complete PR object (title, user.login, headRef), not just the number. The design tradeoff is documented by the underscore prefix.

This adds two extra Forgejo API calls per completed CI event. It's a stepping-stone cost — a follow-up could expose getPRByNumber in the capability to turn this into a single cheap lookup. Not blocking, but worth a // TODO comment in the adapter body so it's visible.

---

Minor

• The pr_number field is required in decide_post_ci's JSON schema and Zod argsSchema, is forwarded through the op → capability → buildPostCiCapability → deps.decidePostCi in main.ts, and then ignored. This is consistent behaviour across the live/shadow impls, but anyone reading just the schema will expect it to be meaningful. The existing underscore prefix on _prNumber in main.ts is the only signal. A one-liner comment in decide_post_ci.ts (// prNumber forwarded for observability / future use; live impl re-resolves the full PR object) would help.

---

Summary

The porting strategy is correct, the test coverage is comprehensive (3240 pass), CI is green, and the #1090 / #1092 regressions are properly fixed. The double-lookup is the only inefficiency and it's a known tradeoff, not a correctness issue. Ship it.

## Review — PR #1097: feat(flows-yaml): port CI events to YAML **CI:** ✅ green **Verdict:** APPROVE ### What I read Full diff reviewed end-to-end: trigger surface, webhook adapter, 7 new ops + tests, both flow YAML files, capability wiring in `main.ts`, shadow/live capability builders, and all test helpers. --- ### What's well done **`webhook-adapter.ts` — `ci_run` mapping is correct.** `pending` is dropped, `error` is collapsed onto `failure`, yielding a clean two-state `(success | failure)` surface for YAML authors. Test coverage for all four wire states is solid. **`pr-opened.yml` rewrite is the right fix for #1090.** The `area:agents` gate was the root cause — any PR without that label was silently skipped forever. The replacement `!author_is('reviewer', 'design-reviewer') && event.pr.headSha != null` is minimal and correct. The headSha null guard mirrors the legacy `if (!sha) return;` bail-out. **`pr-ci.yml` flow logic is sound.** `cancel_review_fallback` unconditionally fires first (right — you want to cancel the timer even when no PR is found). The aggregate-state guard on the success path mirrors the legacy `handleCheckSuiteCompleted` exactly. The "pending aggregate → no-op" path is tested. **All 7 new ops follow the established pattern cleanly** — thin adapters delegating to the `postCi` capability, proper Zod schemas, `deps: ["postCi"] as const`. Op-level unit tests are thorough (schema rejection, happy path, branch-hint forwarding, null defaults). **Deletions are clean.** `handlePullRequestOpened` + `handleCheckSuiteCompleted` + their tests are gone with no dangling callers. The `isReviewerAgent` import is correctly dropped from `event-handlers.ts`. --- ### One thing to note (not a blocker) **Double `findOpenPRForHead` call per CI event.** In `main.ts`, both `dispatchFixCi` and `decidePostCi` capability impls silently ignore their `_prNumber` parameter and do a fresh `findOpenPRForHead` lookup: ```ts // dispatch_fix_ci op → capability → main.ts adapter dispatchFixCi: async (repo, _prNumber, headSha, runId, workflowId, branches) => { // _prNumber is discarded; re-fetches PR by SHA const pr = await findOpenPRForHead(repo, headSha, branchHints, token); ``` The flow already resolved the PR number in `find_open_pr_for_head`; the capability then does a second full API probe because `domain/workflow/post-ci.ts::dispatchFixCi` / `decidePostCiAction` need the complete PR object (title, user.login, headRef), not just the number. The design tradeoff is documented by the underscore prefix. This adds two extra Forgejo API calls per completed CI event. It's a stepping-stone cost — a follow-up could expose `getPRByNumber` in the capability to turn this into a single cheap lookup. Not blocking, but worth a `// TODO` comment in the adapter body so it's visible. --- ### Minor - The `pr_number` field is `required` in `decide_post_ci`'s JSON schema and Zod argsSchema, is forwarded through the op → capability → `buildPostCiCapability` → `deps.decidePostCi` in main.ts, and then ignored. This is consistent behaviour across the live/shadow impls, but anyone reading just the schema will expect it to be meaningful. The existing underscore prefix on `_prNumber` in main.ts is the only signal. A one-liner comment in `decide_post_ci.ts` (`// prNumber forwarded for observability / future use; live impl re-resolves the full PR object`) would help. --- ### Summary The porting strategy is correct, the test coverage is comprehensive (3240 pass), CI is green, and the #1090 / #1092 regressions are properly fixed. The double-lookup is the only inefficiency and it's a known tradeoff, not a correctness issue. Ship it.

reviewer merged commit fc71ec8ea4 into main 2026-05-11 00:22:21 +00:00

claude-desktop referenced this pull request 2026-05-11 03:04:12 +00:00

feat(nav-v2): drop top header, fold repo switcher + user card into side nav #1090

Sign in to join this conversation.

Reviewers

No reviewers

reviewer

Labels

Clear labels   

area:agents

Agent types, pool scheduling, per-instance config

  

area:dashboard

Dashboard UI and observability surfaces

  

area:database

DB layer — schema, migrations, ORM, raw SQL

  

area:design

UI/UX mockup work — routes to designer agent

  

area:design-review

Design review dispatch — routes to design-reviewer agent

  

area:flows

Flow runner — YAML loader, executor, op registry, expression eval

  

area:infra

Deployment, isolation, containers, systemd units

  

area:meta

Tracking, scaffolding, project setup

  

area:security

Security — routes to reviewer-security (opus)

  

area:sessions

Session-id store, Claude SDK resume logic

  

area:webhook

Forgejo webhook routing and handlers

  

area:workdir

Clone cache, worktrees, git identity

  

security

Security-sensitive issue

  

type:bug

Bug

  

type:chore

Chore

  

type:meta

Tracking or decisions, not implementation work

  

type:user-story

User story

No labels

area:agents

area:dashboard

area:database

area:design

area:design-review

area:flows

area:infra

area:meta

area:security

area:sessions

area:webhook

area:workdir

security

type:bug

type:chore

type:meta

type:user-story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

charles/claude-hooks!1097

No description provided.