Skip to content

Feat/webhook linear#230

Closed
twelvearrays wants to merge 25 commits into
AgentWrapper:mainfrom
twelvearrays:feat/webhook-linear
Closed

Feat/webhook linear#230
twelvearrays wants to merge 25 commits into
AgentWrapper:mainfrom
twelvearrays:feat/webhook-linear

Conversation

@twelvearrays

Copy link
Copy Markdown

No description provided.

Add protocol-agnostic McpInputSource interface to replace the Tracker
plugin for issue context resolution. Any MCP server (Linear, GitHub,
Notion, custom) can now drive agent spawning.

New types: McpInputSource, AgentContext, AgentContextComment, Resource.
Existing Tracker interface kept with @deprecated annotation for
backward compatibility.
Add 9th plugin slot: McpToolPlugin for injecting scoped MCP tool
surfaces into spawned agent sessions. Also adds:

- ReactionType union type with all known reaction types
- 'hardware-test-required' reaction for hardware-dependent plugins
- 'queue-for-hardware' action type for ReactionConfig
- McpEntry schema validated via Zod (requires plugin or name+url)
- McpServerConfig, HealthResult types

Config schema validates mcp array entries: rejects invalid URLs,
invalid scope values, and entries missing both plugin and name+url.
# Conflicts:
#	packages/core/src/index.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fdf28043c8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +11 to +16
const previousLabelIds = payload.updatedFrom?.labelIds;
if (previousLabelIds && previousLabelIds.includes(matchingLabel.id)) {
return false;
}

return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require labelIds change before firing label trigger

wasLabelAdded treats a missing updatedFrom.labelIds as a newly added label, so any non-label Issue update that still includes the label in data.labels will retrigger the automation path. In Linear webhooks, updatedFrom commonly omits fields that did not change, so this can repeatedly spawn/send actions whenever the dedup window expires even though no label transition happened.

Useful? React with 👍 / 👎.

Comment on lines +21 to +24
const previousState = payload.updatedFrom?.state;

if (!currentState || currentState.type !== "completed") return false;
if (previousState?.type === "completed") return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Gate completed-state trigger on an actual state transition

wasMovedToCompleted returns true whenever the current state is completed and updatedFrom.state is absent, which means later unrelated Issue updates on already-completed items can keep triggering test-generation. Because this check runs on every Issue/update webhook, completed issues can repeatedly enqueue test-gen runs after dedup timeout without any new state change.

Useful? React with 👍 / 👎.

Comment on lines +84 to +86
const res = await fetch(healthUrl, {
signal: AbortSignal.timeout(3000),
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Pass configured auth when probing MCP server health

UrlMcpToolPlugin.healthCheck probes the endpoint without any auth headers, but many MCP servers require credentials to return 2xx. In those setups this check marks otherwise-usable plugins unhealthy, and injectMcpConfig then excludes them from .mcp.json, causing agents to start without expected MCP tools despite valid runtime credentials being configured in env.

Useful? React with 👍 / 👎.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 5 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.


const testGenPromptFile = resolve(
process.env["TEST_GEN_PROMPT_FILE"] ?? resolve(__dirname, "../../test-gen-prompt.md"),
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong relative path breaks server startup

High Severity

The default path for test-gen-prompt.md uses "../../test-gen-prompt.md" relative to __dirname, but after compilation __dirname is the dist/ directory. Going up two levels from packages/webhook-linear/dist/ lands at packages/, not packages/webhook-linear/ where test-gen-prompt.md actually lives. The correct relative path is "../test-gen-prompt.md". Without the TEST_GEN_PROMPT_FILE env var set, loadConfig() will throw on startup, crashing the server.

Fix in Cursor Fix in Web

return false;
}

return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Label detection triggers on any issue update

Medium Severity

wasLabelAdded returns true whenever the target label is present on the issue and updatedFrom.labelIds is absent. Linear webhooks only include labelIds in updatedFrom when labels actually changed. So any non-label update (title edit, assignee change, etc.) to an issue that already has the trigger label will incorrectly be treated as a new label addition, causing spurious agent spawns.

Fix in Cursor Fix in Web

url?: string;
auth?: { type: "bearer"; token: string };
toolMap?: Record<string, unknown>;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated InputSourceConfig type across two files

Low Severity

InputSourceConfig is defined identically in both resolve-input-source.ts and types.ts. The file already imports ProjectConfig from ./types.js, so it can import InputSourceConfig from there as well instead of re-declaring it. Maintaining two copies risks them diverging silently.

Additional Locations (1)

Fix in Cursor Fix in Web

Triggered by project rule: BugBot Configuration

if (previousState?.type === "completed") return false;

return true;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completed-state detection triggers on any issue update

Medium Severity

wasMovedToCompleted has the same false-positive pattern as wasLabelAdded. When updatedFrom exists but doesn't include state (because a non-state field like title or description was edited), previousState is undefined, the previousState?.type === "completed" check passes, and the function returns true. Any edit to an already-completed issue (description tweak, label change, etc.) would incorrectly trigger a test-gen agent spawn.

Fix in Cursor Fix in Web

hardwareReaction = reaction;
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crashed health check silently treated as healthy

Medium Severity

When a plugin's healthCheck throws (promise rejects), the for loop skips it via continue on line 42, so the plugin is never added to failedPlugins. Since failedNames is built from failedPlugins, the crashing plugin passes the healthy filter on line 63 and gets written to .mcp.json as if it were healthy. A plugin that explicitly defines a healthCheck but crashes during the check ends up being treated identically to a plugin with no health check at all.

Fix in Cursor Fix in Web

harshitsinghbhandari pushed a commit to harshitsinghbhandari/agent-orchestrator that referenced this pull request Jun 24, 2026
* feat(session): support multiple PRs per session

A session can now own several pull requests (a root plus stacked
children) instead of being capped at one. The SQLite schema was already
1-session->many-PR (pr.url PK, session_id a plain FK), so this is a
behavioural change across the observe -> persist -> derive -> react
pipeline, not a migration.

- observe: the SCM observer discovers every open PR whose source branch
  matches a session branch or descends from it ("branch/..." stacking),
  attributing each to the owning session; the longest matching branch
  wins so a child session claims its own stacked PRs.
- derive: session status is a worst-wins aggregate over all owned PRs,
  with a stack model (B is a child of A iff B.target == A.source and A is
  open) exposed via prs[] on every session read DTO.
- react: per-PR reactions; a stacked child blocked by an open parent is
  exempt from the rebase/merge-conflict nudge (only the bottom of the
  stack is eligible), and the session completes only when no PR is open
  and at least one merged.
- tests: unit coverage across stack/status/observer/lifecycle, a
  real-SQLite ListPRFactsForSession test for the stacked-PR read path,
  and a functional end-to-end integration test driving the real store +
  lifecycle + observer through attribution, completion, and stacked-child
  nudge suppression.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(scm): ignore fork heads in PR attribution and persist discovered siblings before completion

Branch-prefix attribution now requires a discovered PR's head branch to live
in the project repo. A fork PR can reuse a session's branch name while its
commits live in the fork, so the previous code could auto-claim foreign work.
Carry head repo full_name from the REST list response and skip any PR whose
head repo is not the base repo.

discoverNewPRs also writes each newly discovered PR as an open baseline row
before the refresh/lifecycle pass runs. A session can own several PRs, and a
terminal observation triggers a completion check that reads all of the
session's PRs from the store. Without the early write, an open sibling found
in the same poll was not yet durable and the session could terminate while
that PR was still open.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(session): surface actionable signals from blocked stacked children; clarify worker prompt

Status aggregation previously dropped any open PR blocked by an open parent,
hiding actionable child signals (failing CI, draft, requested changes,
unresolved comments) behind the parent's status. A blocked child still cannot
merge, so its readiness signals (mergeable/approved/review-pending/open) stay
suppressed, but its problem signals now contribute to the worst-wins aggregate.
The all-blocked fallback is preserved so a session never goes dark.

The worker multi-PR prompt said independent PRs could branch off the base
branch as usual, which conflicts with branch-prefix attribution. Clarify that
a PR may target the base branch, but its source branch must stay under the
session branch namespace for AO to track it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant