Merge dev to main: trigger-shadowing fix + friction reporting#1302
Merged
Conversation
Co-authored-by: Cascade Bot <bot@cascade.dev>
Co-authored-by: Cascade Bot <bot@cascade.dev>
Co-authored-by: Cascade Bot <bot@cascade.dev>
* feat(pm): add report friction command * fix(friction): gitignore friction sidecar to prevent accidental staging Add .cascade/friction-reports.jsonl to .gitignore so the runtime artifact written by `cascade-tools pm report-friction` is never staged by stageAndCommit() or surfaced in `git status --porcelain`, which previously blocked the Finish gadget's uncommitted-changes check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Cascade Bot <bot@cascade.dev> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(agents): scope friction reporting capability * fix(agents): move debug PM capabilities to optional to honour integration gate `debug` explicitly declares `integrations.optional: [pm]`, so all PM capabilities must live in `capabilities.optional` so that `resolveEffectiveCapabilities` can gate them on PM availability. Previously pm:read/write/checklist and pm:friction were all in `capabilities.required`, which caused `buildExecutionPlan` to unconditionally include `ReportFriction` in `availableTools` and append the friction guidance even on projects with no PM integration. Move all four PM capabilities to `capabilities.optional`, matching the pattern used by `review.yaml` and `respond-to-review.yaml`. Update the loader test to reflect that `deriveIntegrations` now correctly derives PM as optional (not required) for the debug profile. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Cascade Bot <bot@cascade.dev> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Closes the prod regression on 2026-05-09 where `PROpenedTrigger`'s structured-skip-on-disabled shadowed `PRConflictDetectedTrigger` for `pull_request: opened` events. On `zbigniewsobiecki/ucho` PR #367 the resolve-conflicts agent never fired even though resolve-conflicts was enabled, because review-on-open was disabled at config and its non-null structured skip halted the registry's first-match-wins loop before the second matcher got a chance. **Root cause.** `TriggerRegistry.dispatch` (`src/triggers/registry.ts`) loops through handlers and stops at the first non-null result. Two distinct semantic classes were collapsed into the same return shape: - "I don't claim this event today" (trigger disabled-at-config) - "I claim this event but my preconditions failed" (wrong base, wrong author, attempt limit) Both produced a structured `{ agentType: null, skipReason: { ... } }` result. Only the second class should halt the registry; the first should continue iteration. Five production sites repeated the wrong pattern (centralized `checkTriggerEnablement` + four inline call sites in `pr-opened`, `pr-merged`, `pr-ready-to-merge`, `check-suite-success`). Six handlers consumed the also-wrong `gateTriggerEnabled` wrapper. **Fix.** - `checkTriggerEnablement` returns `skipResult: null` for disabled, not a structured skip. Operator visibility preserved via the existing INFO-level `Trigger disabled by config, skipping` log. - Inline disabled-skip sites in `pr-opened`, `pr-merged`, `pr-ready-to-merge`, `check-suite-success` flipped to `return null;`. - `gateTriggerEnabled` deleted (now redundant with `checkTriggerEnabled`). Six call sites migrated to `if (!(await checkTriggerEnabled(...))) return null;`. `dispatchRespondToCi` return type widened to `Promise<TriggerResult | null>` to match. - New end-to-end regression test `tests/unit/triggers/disabled-trigger-shadowing.test.ts` — wires PROpenedTrigger + PRConflictDetectedTrigger into a real registry, mocks the config-resolver to disable review and enable resolve-conflicts, dispatches a `pull_request: opened` event, asserts resolve-conflicts fires. Sanity-verified against pre-fix code: the test fails loudly with `expected null to be 'resolve-conflicts'`, proving it actually catches the bug class. - Trigger-event-consistency static guard extended to recognize `dispatchRespondToCi(` as a cross-file emission marker (since `check-suite-failure.ts` now gates on `scm:check-suite-failure` via `checkTriggerEnabled` directly and emits through the dispatch helper). **Affected matcher pairs.** `pull_request: opened` is the only known shadowing pair in the registry (PROpened ⊕ PRConflictDetected). PM matchers (Trello/JIRA/Linear) don't overlap on the same payload shape. Other handlers using the same disabled-skip pattern aren't actively biting anything but were fixed for symmetry — same bug class, same fix, one PR. Webhook decisionReasons shift from per-handler "Trigger pr-opened skipped: review trigger is disabled..." to the registry-level "No trigger matched" when no other handler claims. The per-handler INFO log is unchanged, so operators can still grep Loki for disabled-trigger evidence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(runtime): drain friction sidecar reports * fix(runtime): propagate url/title context to agentInput and SessionState for LLMist gadgets - Forward workItemUrl/workItemTitle/prUrl/prTitle into agentInput from buildGitHubPRDispatchResult and buildPMDispatchResult so that injectAgentInputContext can emit the corresponding CASCADE_* env vars for native-tool subprocess engines - Extend SessionState with runId, prNumber, prUrl, prTitle fields and expose module-level getters so in-process LLMist gadgets (e.g. ReportFriction) can read per-run metadata without process.env - Pass prNumber/prUrl/prTitle from agentInput into createConfiguredBuilder and initSessionState in the LLMist backend - Update buildReport() in reportFriction to fall back to SessionState getters when the env vars are absent (LLMist in-process path) - Update affected trigger tests for the new agentInput shape Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(friction): store project in SessionState so LLMist ReportFriction uses correct context In LLMist (in-process gadgets), projectSecrets are NOT exported into process.env. The production ReportFriction.ts wrapper calls reportFriction() without params.project, causing projectFromEnv() to build a report with id='unknown-project' and empty PM config. - Add `project?: ProjectConfig` to SessionState and InitSessionStateOptions - Pass `project: input.project` from LLMist engine to createConfiguredBuilder → initSessionState - In reportFriction(), fall back to getProject() from SessionState before projectFromEnv() - Add test covering the production wrapper path (no params.project, project in SessionState) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(friction): gate prUrl on prCreated; add SessionState agent context fallbacks - Gate LlmistEngine's returned prUrl on prCreated===true to prevent PR-triggered review/respond-to-ci runs from falsely reporting a "PR created" result when no CreatePR gadget was called - Add SessionState fallbacks for CASCADE_AGENT_TYPE, CASCADE_ENGINE_LABEL, CASCADE_MODEL, CASCADE_PR_BRANCH, CASCADE_INITIAL_HEAD_SHA in buildReport() so LLMist in-process friction reports include accurate agent/PR context instead of "Agent: unknown" with no engine/model/branch/headSha - Store engineLabel and model in InitSessionStateOptions and SessionStateData - Update tests to reflect new prCreated gate and new SessionState fields Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(triggers): centralize TriggerResult URL/title merge in prepareAgentWorkItem Several GitHub trigger handlers (PRReviewSubmittedTrigger, PROpenedTrigger, ReviewRequestedTrigger, PRCommentMentionTrigger) set prUrl/prTitle/workItemUrl/ workItemTitle on the top-level TriggerResult but not in agentInput. Since both injectAgentInputContext (secretBuilder.ts) and LlmistEngine read from agentInput, these native-tool and in-process runs were missing CASCADE_PR_URL, CASCADE_PR_TITLE, CASCADE_WORK_ITEM_URL, and CASCADE_WORK_ITEM_TITLE. Centralizes the merge in prepareAgentWorkItem: after resolving workItemId, any top-level TriggerResult URL/title fields absent from agentInput are merged in before agent execution. This covers all handlers without requiring each one to be updated individually. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Cascade Bot <bot@cascade.dev> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Cascade Bot <bot@cascade.dev>
…er-shadows-siblings
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
zbigniewsobiecki/uchoPR build(deps-dev): bump ajv from 8.17.1 to 8.18.0 #367's resolve-conflicts agent never fired because review-on-open was disabled at config and PROpenedTrigger's structured skip halted the registry's first-match-wins loop. Migrates 6 handlers off the now-removedgateTriggerEnabledwrapper, flips 4 inline disabled-skip sites toreturn null;, adds end-to-end regression net.cascade pm friction-reportcommand (feat(pm): add report friction command #1297) + agent capability scoping (feat(agents): scope friction reporting capability #1298) + runtime drainer (feat(runtime): drain friction sidecar reports #1300) + pm-wizard slot UI (feat(pm-wizard): add friction mapping slot #1295) + slot accessors (feat(pm): add friction slot accessors #1294) + ops docs (docs: document friction reporting operations #1301).Test plan
pm friction-reportexposed in agent toolset, sidecar drains on agent finish🤖 Generated with Claude Code