feat(runtime): drain friction sidecar reports#1300
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Requesting changes: friction sidecars are created and drained, but the runtime metadata required by the card is not reliably propagated to either normal native-tool trigger runs or in-process LLMist gadgets.
Code Issues
Should Fix
- src/backends/secretBuilder.ts:51 -
injectAgentInputContextreads work item and PR URL/title fields only fromAgentInput, but the standard PM/GitHub trigger builders keep those values onTriggerResultand only copyworkItemId/prNumberintoagentInput. Native-toolReportFrictioncalls from normal webhook-triggered runs will therefore get IDs but missCASCADE_WORK_ITEM_URL,CASCADE_WORK_ITEM_TITLE,CASCADE_PR_URL, andCASCADE_PR_TITLE. - src/gadgets/pm/core/reportFriction.ts:177 - The in-process path only falls back to
SessionStatefor the sidecar path.buildReport()still reads project/run/work item/PR metadata fromprocess.env, but LLMist gadgets run in the worker process whereprojectSecretsare not applied to env. The queued report payload can be created withunknown-projectand missing run/work item/PR context, then the backend drain files that stale payload.
🕵️ codex · gpt-5.5 · run details
| projectSecrets.CASCADE_TRELLO_LABELS = JSON.stringify(trelloConfig.labels); | ||
| } | ||
|
|
||
| function injectAgentInputContext(projectSecrets: Record<string, string>, input: AgentInput): void { |
There was a problem hiding this comment.
This misses the normal webhook path for most of these fields. buildPMDispatchResult/buildGitHubPRDispatchResult keep workItemUrl, workItemTitle, prUrl, and prTitle on TriggerResult, while buildAgentInput only copies workItemId and prNumber; prepareAgentWorkItem only patches workItemId as well. As a result, native-tool runs triggered from PM/GitHub webhooks will not receive the URL/title env vars this function is meant to inject, so friction reports lose the agreed context.
There was a problem hiding this comment.
Fixed. Updated buildAgentInput in src/triggers/shared/result-builders.ts to accept an optional extraContext parameter carrying workItemUrl, workItemTitle, prUrl, and prTitle. Both buildPMDispatchResult and buildGitHubPRDispatchResult now pass these fields through, so they land in agentInput and get exported as the corresponding CASCADE_* env vars by injectAgentInputContext for subprocess engines.
| params.sidecarPath ?? process.env[FRICTION_SIDECAR_ENV_VAR] ?? DEFAULT_FRICTION_SIDECAR_PATH; | ||
| params.sidecarPath ?? | ||
| process.env[FRICTION_SIDECAR_ENV_VAR] ?? | ||
| getFrictionSidecarPath() ?? |
There was a problem hiding this comment.
This recovers only the sidecar path for in-process gadgets. In LLMist, projectSecrets are not exported into process.env, but projectFromEnv()/buildReport() still read project, run, work item, PR, engine, and model metadata from env. The unit test sets those env vars manually, but production LLMist ReportFriction calls will queue a report with unknown-project/missing context, and the later drain files that already-built payload.
There was a problem hiding this comment.
Fixed. Extended SessionState with runId, prNumber, prUrl, and prTitle fields (with corresponding module-level getters), and updated buildReport() in reportFriction.ts to fall back to getRunId(), getPrNumber(), getPrUrl(), getPrTitle(), getWorkItemId(), getWorkItemUrl(), and getWorkItemTitle() when the env vars are absent. The LLMist backend now passes prNumber/prUrl/prTitle from agentInput into createConfiguredBuilder, which stores them in SessionState via initSessionState. CASCADE_DASHBOARD_URL (global infra config) is still read from process.env directly.
…ate 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>
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Requesting changes: the native-tool sidecar path and drain flow are in place, but the LLMist in-process ReportFriction path still records incorrect project context in the queued report payload.
Code Issues
Should Fix
- src/gadgets/pm/core/reportFriction.ts:143 — The new SessionState fallbacks cover run/work-item/PR metadata, but the production in-process gadget still calls
reportFriction()withoutparams.project, soreportFriction()falls back toprojectFromEnv(). In LLMist, project secrets are not exported intoprocess.env; the wrapper atsrc/gadgets/pm/ReportFriction.tspasses only the model-supplied fields. That means a normal LLMistReportFrictioncall builds and queues a payload withcontext.project.id = "unknown-project", missing repo/name/PM type, and likely empty Trello/JIRA/Linear placement config. The backend drain later uses the realinput.projectonly to create the PM item; it does not rebuildevent.report.context, so the filed friction card keeps the stale project context. The current unit test avoids this by passingprojectmanually, which the production wrapper does not do.
🕵️ codex · gpt-5.5 · run details
| // are NOT exported to process.env — only subprocess engines (claude-code, opencode, codex) | ||
| // receive them as actual env vars. Fall back to SessionState for fields that are available | ||
| // there when the env var is absent. | ||
| const runId = process.env.CASCADE_RUN_ID ?? getRunId() ?? undefined; |
There was a problem hiding this comment.
This still leaves the project context on process.env. In production LLMist runs, ReportFriction calls reportFriction() without params.project, and projectSecrets are not exported into the worker process env. So projectFromEnv() returns unknown-project/empty PM config before this fallback logic builds the queued sidecar report. The later backend drain uses the real project only for materialization, not to rebuild event.report.context, so the filed friction card keeps the wrong project metadata. Please pass the execution project into the in-process gadget or store/use the full project context from SessionState, and cover the actual wrapper path in the test.
There was a problem hiding this comment.
Fixed. The root cause was that reportFriction() fell back to projectFromEnv() when params.project was absent, but in LLMist (in-process gadgets), projectSecrets are never exported to process.env — so projectFromEnv() would return id='unknown-project' with empty PM placement config.
Changes made:
SessionState— addedproject?: ProjectConfigtoInitSessionStateOptionsandSessionStateData, with a newgetProject()getter (both on the class and as a module-level wrapper).builderFactory.ts— addedproject?: ProjectConfigtoCreateBuilderOptionsand passed it through toinitSessionState.LlmistEngine— passesproject: input.projecttocreateConfiguredBuilder, so the full project config is stored in SessionState for the duration of the LLMist run.reportFriction()— now resolves project asparams.project ?? getProject() ?? projectFromEnv(). ThegetProject()fallback provides the real project (set by LLMist) before falling back to env-var reconstruction.- Test — added
'uses project from SessionState when params.project is absent (production ReportFriction wrapper path)'that callsreportFriction()withoutparams.project(exactly as the production wrapper does), confirms the report carries the correct project context, and verifiesmaterializeFrictionReportreceives the real project.
… 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>
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Requesting changes: the LLMist in-process path now has the sidecar/project fallback, but the PR URL is being stored in the existing “created PR” session slot and some friction runtime context remains env-only for LLMist.
Code Issues
Should Fix
- src/gadgets/sessionState.ts:147 —
prUrlis now initialized from the incoming PR context, but that same field is the historical CreatePR result field.LlmistEnginereadsgetSessionState().prUrlas authoritative PR evidence after the run, and trailing hints render it as “PR created”. A normal PR-triggered LLMist review/respond-to-ci run that never calls CreatePR will now return a PR result as if the run created one. Store incoming PR context separately from the CreatePR/recordPRCreationstate, or gate LLMist’s returned PR evidence onprCreated. - src/gadgets/pm/core/reportFriction.ts:144 — the LLMist fallback still covers only run/work-item/PR number/title/url and project.
context.agentandpr.branch/pr.headShastill read onlyprocess.env, but LLMist does not export projectSecrets into env. Native-tool reports getCASCADE_AGENT_TYPE,CASCADE_ENGINE_LABEL,CASCADE_MODEL,CASCADE_PR_BRANCH, andCASCADE_INITIAL_HEAD_SHA; in-process reports still file withAgent: unknown, no engine/model, and no branch/head SHA. Those values need the same SessionState treatment for in-process reports.
🕵️ codex · gpt-5.5 · run details
| initialHeadSha: initialHeadSha ?? null, | ||
| runId: runId ?? null, | ||
| prNumber: prNumber ?? null, | ||
| prUrl: prUrl ?? null, |
There was a problem hiding this comment.
This reuses state.prUrl for incoming PR context, but that field is also the CreatePR result slot: recordPRCreation() writes it, hintConfig renders it as PR created, and LlmistEngine returns it as authoritative PR evidence after the run. Because buildGitHubPRDispatchResult() now mirrors existing PR URLs into agentInput, any PR-triggered LLMist run starts with this field populated even if the agent never calls CreatePR. Please split incoming PR context from created-PR state, or make LLMist return PR evidence only when prCreated is true.
There was a problem hiding this comment.
Fixed. Gated prUrl in LlmistEngine on prCreated === true:
const sessionState = getSessionState();
const prUrl = sessionState.prCreated ? (sessionState.prUrl ?? undefined) : undefined;prCreated is only set to true by recordPRCreation() (called from the CreatePR gadget), so incoming PR context stored in prUrl via init() no longer leaks through as false "PR created" evidence. PR-triggered review/respond-to-ci runs that never call CreatePR will now correctly return prUrl: undefined. Updated the llmist.test.ts test to set prCreated: true in the mock alongside prUrl for the scenario where CreatePR was genuinely called.
| // are NOT exported to process.env — only subprocess engines (claude-code, opencode, codex) | ||
| // receive them as actual env vars. Fall back to SessionState for fields that are available | ||
| // there when the env var is absent. | ||
| const runId = process.env.CASCADE_RUN_ID ?? getRunId() ?? undefined; |
There was a problem hiding this comment.
This fallback is still incomplete for the LLMist path. The same function later reads CASCADE_AGENT_TYPE, CASCADE_ENGINE_LABEL, CASCADE_MODEL, CASCADE_PR_BRANCH, and CASCADE_INITIAL_HEAD_SHA directly from process.env, but LLMist does not export projectSecrets into env. Native-tool friction reports get those values; in-process reports still file as Agent: unknown with no engine/model and no PR branch/head SHA. Those fields need SessionState fallbacks too.
There was a problem hiding this comment.
Fixed. Added SessionState fallbacks for all five fields that were previously env-only:
CASCADE_AGENT_TYPE→getAgentType()CASCADE_ENGINE_LABEL→getEngineLabel()CASCADE_MODEL→getModel()CASCADE_PR_BRANCH→getPrBranch()CASCADE_INITIAL_HEAD_SHA→getInitialHeadSha()
Added engineLabel and model to InitSessionStateOptions/SessionStateData (and the corresponding module-level getters). LlmistEngine already passes engineLabel: this.definition.id and model flows through createConfiguredBuilder → initSessionState. buildReport() now uses process.env.CASCADE_X ?? getX() ?? fallback for all five fields. Updated backendMocks.ts and the reportFriction.test.ts "uses SessionState for run/work-item/PR metadata" test to assert agent.engine, agent.model, pr.branch, and pr.headSha are populated from SessionState.
…t 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>
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Requesting changes: the runtime sidecar/drain flow is in place, but URL/title metadata is still not reliably propagated for normal GitHub-triggered runs that do not use the shared dispatch builders.
Code Issues
Should Fix
- src/triggers/shared/result-builders.ts:65 - The new mirroring only covers callers that go through
buildPMDispatchResultorbuildGitHubPRDispatchResult. Several GitHub handlers still returnTriggerResultobjects manually; for examplePRReviewSubmittedTriggersetsprUrl,prTitle,workItemUrl, andworkItemTitleon the top-level result but leaves them out ofagentInput.prepareAgentWorkItem()only patchesworkItemId, whileaugmentProjectSecrets()andLlmistEngineboth read URL/title fields fromagentInput. A normalscm:pr-review-submittedrespond-to-review run that callsReportFrictionwill therefore still file withoutCASCADE_PR_URL,CASCADE_PR_TITLE,CASCADE_WORK_ITEM_URL, andCASCADE_WORK_ITEM_TITLEin native-tool mode, and without the same SessionState fields in LLMist. The same pattern appears inpr-opened,review-requested, andpr-comment-mentionfor at least PR URL/title. Please centralize the top-levelTriggerResultmetadata merge before agent execution, or update the manual GitHub handlers to put the metadata intoagentInputas well.
🕵️ codex · gpt-5.5 · run details
| triggerEvent, | ||
| ...(workItemId ? { workItemId } : {}), | ||
| ...(prNumber !== undefined ? { prNumber } : {}), | ||
| // Mirror URL/title fields into agentInput so injectAgentInputContext |
There was a problem hiding this comment.
This only fixes metadata propagation for callers that go through these builders. Several GitHub handlers still construct TriggerResult manually (for example pr-review-submitted.ts returns prUrl/prTitle/workItemUrl/workItemTitle only at the top level). Since prepareAgentWorkItem() only patches workItemId, and both injectAgentInputContext() and LlmistEngine read from agentInput, those normal PR-triggered runs still lose URL/title context for ReportFriction. Please centralize the top-level-to-agentInput merge or update the manual handlers too.
There was a problem hiding this comment.
Fixed. Centralized the top-level → agentInput metadata merge in prepareAgentWorkItem (src/triggers/shared/agent-work-items.ts).
After resolving workItemId, the function now merges any top-level TriggerResult fields (prUrl, prTitle, workItemUrl, workItemTitle) into agentInput when they are absent. This covers all manual GitHub handlers — PRReviewSubmittedTrigger, PROpenedTrigger, ReviewRequestedTrigger, PRCommentMentionTrigger — without modifying any of them individually. Fields already present in agentInput (set by handlers that go through buildGitHubPRDispatchResult) are not overwritten.
Added three new test cases in agent-work-items.test.ts:
- merges top-level prUrl/prTitle/workItemUrl/workItemTitle into agentInput when handler omits them
- does not overwrite agentInput fields already populated by the handler
- renamed the existing 'leaves agentInput untouched' test to clarify it applies when the TriggerResult also carries no URL/title fields
…ntWorkItem 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>
nhopeatall
left a comment
There was a problem hiding this comment.
LGTM - the runtime now creates and propagates the friction sidecar independently of finish hooks, drains pending reports on success and ordinary failure without changing the main result, and carries the required project/run/work-item/PR context through both native-tool env vars and LLMist SessionState. Targeted backend, core, and trigger tests passed locally.
🕵️ codex · gpt-5.5 · run details
Summary
Implements runtime support for friction report sidecars and context injection.
Card: https://trello.com/c/69ff6a906f4a39c5c1610874
Changes
CASCADE_FRICTION_SIDECAR_PATHinto native-tool runs, independent of finish hooks.SessionStatefor in-process gadgets.lists.frictionis available like JIRA/Linear status mappings.Verification
npx vitest run --project unit-backends tests/unit/backends/sidecarManager.test.ts tests/unit/backends/adapter.test.ts tests/unit/backends/secretBuilder.test.ts tests/unit/backends/secretOrchestrator.test.tsnpx vitest run --project unit-core tests/unit/agents/shared/builderFactory.test.ts tests/unit/gadgets/pm/core/reportFriction.test.ts tests/unit/cli/credential-scoping.test.tsnpm run typechecknpm run lint:fix && npm run lint(passes; reports existing warnings in unrelated tests)npm test🕵️ codex · gpt-5.5 · run details