feat(iris): autonomous marketing agent with Slack ship-it loop - #640
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Comp AI code review complete — 1 issue found. Commit |
Contributor
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| export async function irisControllerRun( | ||
| payload: IrisWorkflowPayload | ||
| ): Promise<IrisControllerResult> { | ||
| "use workflow"; |
| executionId: string; | ||
| claimToken: string; | ||
| }): Promise<{ claimed: boolean }> { | ||
| "use step"; |
| organizationId: string; | ||
| ownerToken: string; | ||
| }): Promise<{ acquired: boolean; fencingToken: number | null }> { | ||
| "use step"; |
| organizationId: string; | ||
| ownerToken: string; | ||
| }): Promise<boolean> { | ||
| "use step"; |
| organizationId: string; | ||
| ownerToken: string; | ||
| }): Promise<void> { | ||
| "use step"; |
| export async function loadIrisMandate( | ||
| organizationId: string | ||
| ): Promise<IrisMandateContext> { | ||
| "use step"; |
| export async function gatherIrisContext( | ||
| organizationId: string | ||
| ): Promise<IrisGatherResult> { | ||
| "use step"; |
Member
Author
|
@greptile-apps review |
![comp-ai-code-review[bot]](https://avatars.githubusercontent.com/in/2105518?s=60&v=4){kind=small}
There was a problem hiding this comment.
No blocking issues found across the changed files.
1 other commit review still in progress for this PR — findings may follow.
Commit 641f02b · Posted by Comp AI Code Reviews.
Contributor
Greptile SummaryIris adds an autonomous signal-to-content workflow with durable planning, execution, Slack approval, scheduling, billing, and a dashboard management surface.
Confidence Score: 4/5The PR is not yet safe to merge because schedule reconciliation can disable or orphan recurring wakes, and task execution remains able to outlive its controller lease. Pause can complete after an ambiguous schedule deletion while resume trusts the retained ID without checking QStash, and a controller validates its lease only before a potentially long-running task rather than before committing task and run side effects. Files Needing Attention: apps/dashboard/src/lib/iris/wake-schedule.ts, apps/dashboard/src/lib/orpc/routers/iris.ts, apps/dashboard/src/workflows/iris-controller.ts Important Files Changed
Sequence DiagramsequenceDiagram
participant Sources as GitHub / Linear
participant Iris as Iris Controller
participant DB as Postgres
participant Models as Planner / Content Models
participant Slack
participant User
Sources->>DB: Record deduplicated signals
Iris->>DB: Acquire controller lease and claim signals
Iris->>Models: Plan validated task DAG
Iris->>Models: Generate content artifacts
Iris->>DB: Persist run, tasks, costs, and drafts
Iris->>Slack: Send Ship / Skip card
User->>Slack: Choose Ship or Skip
Slack->>DB: Atomically record decision
Slack->>DB: Publish approved post
Reviews (4): Last reviewed commit: "fix(iris): schedule rollback logging, si..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
40 issues found and verified against the latest diff
Confidence score: 2/5
- In
packages/ai/src/autonomy/leases.tsandpackages/db/src/schema.ts, lease loss/takeover is not reliably enforced (ignored renewal failure, deterministicownerToken, and no fencing checks), so duplicate or stale controllers can continue executing and writing side effects concurrently. Stop work immediately on failed renewal and requirefencingTokenon all mutating/external actions to prevent split-brain behavior. - Cost controls have concrete gaps across
packages/ai/src/autonomy/planner.ts,packages/ai/src/autonomy/gate.ts, andapps/dashboard/src/lib/iris/history.ts: failed/partial runs can dropcostCents, and gating can admit calls before reserving remaining budget. Persist accumulated cost on every failure path and enforce/reserve budget before each billable step to keep daily limits trustworthy. - Failure handling in
apps/dashboard/src/workflows/iris-controller.tsandpackages/ai/src/autonomy/ship.tscan permanently lose work: coalesced signals can be removed after exceptions, and a publish failure can become unrecoverable once approval is pre-recorded. Make status transitions and publish/approval updates atomic (or compensating) so retries can safely recover pending work. apps/dashboard/src/lib/iris/wake-schedule.tsuses mismatched URLs for signing vs verification whenWORKFLOW_BASE_URLdiffers, which can cause wake deliveries to fail entirely. Resolve and use one canonical destination URL for both QStash signing and route verification.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/ai/src/autonomy/leases.ts">
<violation number="1" location="packages/ai/src/autonomy/leases.ts:32">
P2: The fencing token repeats after every successful release, so it cannot order lease generations or reject stale writers. Preserve a monotonic counter when releasing/expiring the lease instead of deleting the row, or store the counter separately.</violation>
<violation number="2" location="packages/ai/src/autonomy/leases.ts:48">
P1: Duplicate starts of one execution can both acquire this lease and run Iris concurrently: `ownerToken` is deterministic per `executionId`, and this same-owner branch treats a second acquisition as valid. Use an attempt-unique owner token or distinguish an idempotent replay from a currently running attempt; otherwise both runs can generate tasks, spend budget, and publish results.</violation>
<violation number="3" location="packages/ai/src/autonomy/leases.ts:110">
P1: A controller continues running after its lease is lost because callers ignore this `false` result. Stop processing immediately on renewal failure (and fence subsequent writes), or an expired controller and its replacement can both generate content and update the same signals.</violation>
</file>
<file name="packages/ai/src/autonomy/planner.ts">
<violation number="1" location="packages/ai/src/autonomy/planner.ts:191">
P1: Invalid planner output consumes one or two model calls but records no cost or usage. Propagate accumulated `costCents` through this failure path so failed planning attempts count toward billing and the daily budget.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/wake-schedule.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/wake-schedule.ts:47">
P1: Wake deliveries fail signature verification whenever `WORKFLOW_BASE_URL` differs from the app URL, because QStash signs the app-URL destination while the route verifies the workflow URL. Use the same resolved URL for schedule destination and verification.</violation>
<violation number="2" location="apps/dashboard/src/lib/iris/wake-schedule.ts:70">
P2: A database failure after QStash accepts this schedule leaves an untracked recurring delivery, and retries create additional schedules. Compensate by deleting `scheduleId` when `persistScheduleId` fails, or make remote creation and persisted ownership recoverable.</violation>
<violation number="3" location="apps/dashboard/src/lib/iris/wake-schedule.ts:104">
P1: Pausing can orphan a live QStash schedule when deletion fails: the error is logged, then its ID is cleared. Keep the ID until QStash confirms deletion so a later pause/resume can retry cleanup.</violation>
</file>
<file name="packages/ai/src/autonomy/gate.ts">
<violation number="1" location="packages/ai/src/autonomy/gate.ts:47">
P1: A run admitted with one action slot remaining can still start every task in its plan, so `maxActionsPerDay` is not a ceiling. Reserve/check an action slot before each `startAction` (or restrict planning to remaining capacity).</violation>
<violation number="2" location="packages/ai/src/autonomy/gate.ts:54">
P1: A run can spend past `maxCostCentsPerDay`: e.g. 99¢ recorded against a 100¢ limit still admits planner/task calls, and their costs are only written at finalization. Enforce/reserve the remaining budget before each billable call, including planning.</violation>
</file>
<file name="apps/dashboard/src/workflows/iris-controller.ts">
<violation number="1" location="apps/dashboard/src/workflows/iris-controller.ts:82">
P2: Failures before the later `try` leave the controller lease held until its 30-minute TTL, and failures after creating a gate no-op can also leave a planning run open. Wrap all post-acquisition work in cleanup that releases the lease and closes any created run.</violation>
<violation number="2" location="apps/dashboard/src/workflows/iris-controller.ts:165">
P1: A planner/persistence/task exception permanently removes non-primary coalesced signals from future runs. Restore them to `pending` on failure, or defer the status transition until the run reaches a terminal outcome.</violation>
<violation number="3" location="apps/dashboard/src/workflows/iris-controller.ts:347">
P2: Partial task failures are shown as completed goals/runs whenever one task succeeded, hiding the failed and canceled work from completion state. Mark the goal/run completed only when all planned tasks succeed; otherwise use the appropriate failed or canceled terminal state.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/history.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/history.ts:47">
P1: A failure after a billed task leaves this run's accumulated cost at zero, so later gates undercount daily spend and can exceed the mandate cost ceiling. Preserve and persist the controller's accumulated `costCents` on this failure path (and ideally reconcile task state) instead of only changing run status.</violation>
</file>
<file name="packages/ai/src/autonomy/ship.ts">
<violation number="1" location="packages/ai/src/autonomy/ship.ts:95">
P1: A failed post publish becomes permanently unrecoverable: the `shipped` approval is persisted first, then retries treat it as already decided and never call `shipIrisPost`. Make approval recording and publishing one durable transition, or retry publishing when the existing action is `shipped`.</violation>
</file>
<file name="packages/db/src/schema.ts">
<violation number="1" location="packages/db/src/schema.ts:1944">
P1: A lease takeover does not fence a stale controller: `fencingToken` is never checked before task/action mutations or external capability execution. Persist and require the acquired token in those mutations (or otherwise reject stale owners) so an expired workflow cannot continue alongside its replacement.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-granola.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-granola.ts:32">
P1: Enabled Granola workspaces are polled but permanently produce no signals, so meeting-note activity never reaches Iris. Implement note retrieval and map notes into `IrisPollItem`s (or omit this source until that capability exists).</violation>
</file>
<file name="packages/ai/src/autonomy/capabilities.ts">
<violation number="1" location="packages/ai/src/autonomy/capabilities.ts:368">
P1: For organizations with multiple GitHub integrations, an announcement for one repository can clone and use an unrelated repository for its image. Select the integration identified by the signal context (or skip image generation when no single repository can be resolved).</violation>
<violation number="2" location="packages/ai/src/autonomy/capabilities.ts:518">
P2: An image is embedded and can be auto-published when quality review fails, despite the image never receiving a QC verdict. Default failed reviews to rejection so unavailable or malformed reviews fail closed.</violation>
<violation number="3" location="packages/ai/src/autonomy/capabilities.ts:649">
P1: Successful blog-image generations are billed twice: once as `marketing_assets` and again in the Iris run total. Keep image costs in the run budget, but use one billing path for this usage.</violation>
</file>
<file name="packages/ai/src/autonomy/run-store.ts">
<violation number="1" location="packages/ai/src/autonomy/run-store.ts:273">
P1: A retry re-executes an existing `executing` action, so a crash after paid generation but before post persistence can spend again and bypass the intended retry reuse. Treat in-progress actions as recoverable/unknown or persist an invocation result before allowing another execution.</violation>
</file>
<file name="apps/dashboard/src/utils/iris-signal-summary.ts">
<violation number="1" location="apps/dashboard/src/utils/iris-signal-summary.ts:25">
P1: Push-driven plans and drafts lose all change context: every same-sized push to a repo becomes only `N commits`. Include a bounded commit subject/excerpt so Iris can distinguish announcement-worthy work from typo or maintenance commits.</violation>
</file>
<file name="packages/ai/src/constants/autonomy-capabilities.ts">
<violation number="1" location="packages/ai/src/constants/autonomy-capabilities.ts:3">
P1: Tasks declaring an unsupported capability version are accepted and executed using the current implementation, so catalog versioning is not fail-closed. Validate each task's `(capabilityName, capabilityVersion)` against `IRIS_CAPABILITY_CATALOG` before persisting or executing it.</violation>
</file>
<file name="apps/dashboard/src/lib/webhooks/github.ts">
<violation number="1" location="apps/dashboard/src/lib/webhooks/github.ts:486">
P1: A transient signal-store failure returns 200 and marks the merged-PR delivery processed, permanently dropping that Iris signal. Propagate recording failures (or avoid delivery dedupe until recording succeeds) so GitHub retries it.</violation>
</file>
<file name="packages/ai/src/schemas/slack-delivery.ts">
<violation number="1" location="packages/ai/src/schemas/slack-delivery.ts:5">
P1: A successful response without `ts` is accepted and marks the outbox delivered, so a malformed/incomplete Slack response cannot be retried and later approval updates have no message timestamp. Require non-empty `channel` and `ts` when `ok` is true.</violation>
</file>
<file name="packages/ai/src/integrations/slack-post.ts">
<violation number="1" location="packages/ai/src/integrations/slack-post.ts:56">
P1: Organizations with multiple configured Slack workspaces can post or update through an arbitrary workspace. A later interaction can resolve a different integration than the card's originating `teamId`, so `chat.update` uses the wrong bot token and the approval succeeds without updating its card; select and persist/propagate the originating integration or team instead of using the first query result.</violation>
</file>
<file name="packages/ai/src/autonomy/deliver.ts">
<violation number="1" location="packages/ai/src/autonomy/deliver.ts:215">
P1: Pausing Iris can still send a card when cancellation wins between the initial select and this unconditional state update. Claim the row with `status = "pending"` and skip posting when that conditional update affects no row.</violation>
<violation number="2" location="packages/ai/src/autonomy/deliver.ts:285">
P1: Transient Slack failures can remain pending indefinitely during quiet wakes because no scheduler invokes `deliverPendingOutbox` after `nextAttemptAt`. Run the delivery sweep on each wake or enqueue a retry trigger when this state is recorded.</violation>
</file>
<file name="packages/ai/src/autonomy/outbox.ts">
<violation number="1" location="packages/ai/src/autonomy/outbox.ts:31">
P1: Pausing Iris during its final task can still post the completed run to Slack: this creates a deliverable row after the pause handler has already canceled existing rows. Re-check mission activity immediately before enqueue/delivery (and make delivery honor cancellation) so pause is a real stop boundary.</violation>
</file>
<file name="packages/ai/src/utils/autonomy-topo.ts">
<violation number="1" location="packages/ai/src/utils/autonomy-topo.ts:21">
P2: A self-dependent node is reported as acyclic, so callers relying on `cycleDetected` can persist or execute a task whose prerequisite can never complete. Retain the self-edge when constructing in-degrees so Kahn's traversal flags it as a cycle.</violation>
</file>
<file name="apps/agent/agent/lib/utils/iris-interaction.ts">
<violation number="1" location="apps/agent/agent/lib/utils/iris-interaction.ts:193">
P2: Near-simultaneous Slack clicks can lose a persisted decision, leaving the card/audit trail incomplete; conflicting Ship/Skip clicks can also leave a skipped approval for a post that the other callback published. Persist the append with row locking or optimistic compare-and-swap before rebuilding the card.</violation>
</file>
<file name="packages/ai/src/prompts/iris-content.ts">
<violation number="1" location="packages/ai/src/prompts/iris-content.ts:48">
P1: A malicious signal can survive into planner-generated `topic`, `angle`, or `audience` and become an instruction in the content prompt, outside `<source-context>`. Sanitize or delimit these task fields before interpolation so the untrusted-signal boundary remains intact across the planner-to-content handoff.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/mandate.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/mandate.ts:19">
P1: An additional active mandate can be selected arbitrarily here, so Iris may plan and spend against that mandate while the dashboard pauses/displays its named Iris mandate. Scope this lookup to `IRIS_MANDATE_NAME` (or pass the intended mandate ID) before limiting.</violation>
</file>
<file name="apps/dashboard/src/components/iris/iris-running-state.tsx">
<violation number="1" location="apps/dashboard/src/components/iris/iris-running-state.tsx:43">
P2: A revoked mandate is shown as paused and the Resume button reactivates it. Handle `revoked` as a distinct terminal state so this screen cannot silently revive a revoked mission.</violation>
</file>
<file name="packages/ai/src/utils/iris-billing.ts">
<violation number="1" location="packages/ai/src/utils/iris-billing.ts:36">
P2: Transient Autumn failures leave completed Iris runs permanently unmetered: this branch logs then returns success, so `finalizeIrisRun` has no retry path after `completeRun`. Persist a retryable, run-id-idempotent billing event (or propagate the failure with an idempotency mechanism) instead of swallowing it.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-linear.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-linear.ts:70">
P2: Busy Linear workspaces record only the first 25 completed issues per poll; remaining pages are ignored and can fall outside the lookback window before becoming signals. Paginate while `pageInfo.hasNextPage` (with an explicit completion-time ordering if a bounded cap is still required), or make the truncation an intentional surfaced limit.</violation>
</file>
<file name="packages/ai/src/utils/image-post-service.ts">
<violation number="1" location="packages/ai/src/utils/image-post-service.ts:283">
P2: A retry after the post insert succeeds but before the collection update will leave `completedPostCount` permanently low: the replay sees the existing `postId` and returns without incrementing. Keep the insert and conditional count update in one transaction, as `createPostRecord` does.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-github.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-github.ts:101">
P2: Prereleases can trigger a `github.release.published` signal only through polling, while their webhook action is ignored by `resolveIrisGithubSignalKind`. Exclude `release.prerelease` here (or model prereleases consistently) so polling and webhooks converge and prereleases do not produce an unintended ship-it loop.</violation>
<violation number="2" location="packages/ai/src/utils/iris-poll-github.ts:242">
P2: One repository API/token failure makes the whole wake record no source signals, including successful repositories and Linear results. Isolate repository failures into an empty/skipped result so a stale or revoked connection cannot suppress all polling.</violation>
</file>
<file name="packages/ai/src/utils/iris-image-markers.ts">
<violation number="1" location="packages/ai/src/utils/iris-image-markers.ts:7">
P2: A malformed placeholder can consume and remove intervening blog text because the description capture permits line breaks. Restrict marker descriptions to a single line so only standalone placeholder lines are resolved.</violation>
</file>
<file name="apps/dashboard/scripts/autonomy-smoke.ts">
<violation number="1" location="apps/dashboard/scripts/autonomy-smoke.ts:136">
P2: Claim acquisition failures only print `false`, so a broken claim path can still make `smoke:autonomy` exit 0. Assert expected results for acquisition, contention, release, and takeover rather than treating logs as checks.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }); | ||
| } | ||
|
|
||
| return renewed; |
Contributor
There was a problem hiding this comment.
P1: A controller continues running after its lease is lost because callers ignore this false result. Stop processing immediately on renewal failure (and fence subsequent writes), or an expired controller and its replacement can both generate content and update the same signals.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/leases.ts, line 110:
<comment>A controller continues running after its lease is lost because callers ignore this `false` result. Stop processing immediately on renewal failure (and fence subsequent writes), or an expired controller and its replacement can both generate content and update the same signals.</comment>
<file context>
@@ -0,0 +1,136 @@
+ });
+ }
+
+ return renewed;
+});
+
</file context>
|
|
||
| const output = validation.output; | ||
| if (!output) { | ||
| return yield* Effect.fail( |
Contributor
There was a problem hiding this comment.
P1: Invalid planner output consumes one or two model calls but records no cost or usage. Propagate accumulated costCents through this failure path so failed planning attempts count toward billing and the daily budget.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/planner.ts, line 191:
<comment>Invalid planner output consumes one or two model calls but records no cost or usage. Propagate accumulated `costCents` through this failure path so failed planning attempts count toward billing and the daily budget.</comment>
<file context>
@@ -0,0 +1,209 @@
+
+ const output = validation.output;
+ if (!output) {
+ return yield* Effect.fail(
+ new IrisPlannerError({
+ message: "Planner output failed validation after the repair attempt",
</file context>
| try: async () => { | ||
| const client = getQstashClient(); | ||
| const result = await client.schedules.create({ | ||
| destination: `${getAppUrl()}${IRIS_WAKE_ROUTE_PATH}`, |
Contributor
There was a problem hiding this comment.
P1: Wake deliveries fail signature verification whenever WORKFLOW_BASE_URL differs from the app URL, because QStash signs the app-URL destination while the route verifies the workflow URL. Use the same resolved URL for schedule destination and verification.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/lib/iris/wake-schedule.ts, line 47:
<comment>Wake deliveries fail signature verification whenever `WORKFLOW_BASE_URL` differs from the app URL, because QStash signs the app-URL destination while the route verifies the workflow URL. Use the same resolved URL for schedule destination and verification.</comment>
<file context>
@@ -0,0 +1,115 @@
+ try: async () => {
+ const client = getQstashClient();
+ const result = await client.schedules.create({
+ destination: `${getAppUrl()}${IRIS_WAKE_ROUTE_PATH}`,
+ cron: IRIS_WAKE_CRON,
+ body: JSON.stringify({ organizationId, trigger: "wake" }),
</file context>
| }; | ||
| } | ||
|
|
||
| if (costCentsInLast24h >= mandate.policy.maxCostCentsPerDay) { |
Contributor
There was a problem hiding this comment.
P1: A run can spend past maxCostCentsPerDay: e.g. 99¢ recorded against a 100¢ limit still admits planner/task calls, and their costs are only written at finalization. Enforce/reserve the remaining budget before each billable call, including planning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/gate.ts, line 54:
<comment>A run can spend past `maxCostCentsPerDay`: e.g. 99¢ recorded against a 100¢ limit still admits planner/task calls, and their costs are only written at finalization. Enforce/reserve the remaining budget before each billable call, including planning.</comment>
<file context>
@@ -0,0 +1,140 @@
+ };
+ }
+
+ if (costCentsInLast24h >= mandate.policy.maxCostCentsPerDay) {
+ return {
+ proceed: false,
</file context>
| const runId = created.runId; | ||
|
|
||
| try { | ||
| const coalesced = await coalesceIrisSignals({ |
Contributor
There was a problem hiding this comment.
P1: A planner/persistence/task exception permanently removes non-primary coalesced signals from future runs. Restore them to pending on failure, or defer the status transition until the run reaches a terminal outcome.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/workflows/iris-controller.ts, line 165:
<comment>A planner/persistence/task exception permanently removes non-primary coalesced signals from future runs. Restore them to `pending` on failure, or defer the status transition until the run reaches a terminal outcome.</comment>
<file context>
@@ -0,0 +1,398 @@
+ const runId = created.runId;
+
+ try {
+ const coalesced = await coalesceIrisSignals({
+ organizationId,
+ signalIds: gathered.pendingSignalIds,
</file context>
| return response.data | ||
| .filter( | ||
| (release) => | ||
| !release.draft && isWithinWindow(release.published_at ?? null, since) |
Contributor
There was a problem hiding this comment.
P2: Prereleases can trigger a github.release.published signal only through polling, while their webhook action is ignored by resolveIrisGithubSignalKind. Exclude release.prerelease here (or model prereleases consistently) so polling and webhooks converge and prereleases do not produce an unintended ship-it loop.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/utils/iris-poll-github.ts, line 101:
<comment>Prereleases can trigger a `github.release.published` signal only through polling, while their webhook action is ignored by `resolveIrisGithubSignalKind`. Exclude `release.prerelease` here (or model prereleases consistently) so polling and webhooks converge and prereleases do not produce an unintended ship-it loop.</comment>
<file context>
@@ -0,0 +1,252 @@
+ return response.data
+ .filter(
+ (release) =>
+ !release.draft && isWithinWindow(release.published_at ?? null, since)
+ )
+ .map(
</file context>
| return { status: "controller_busy", runId: null, reason: null }; | ||
| } | ||
|
|
||
| const context = await loadIrisMandate(organizationId); |
Contributor
There was a problem hiding this comment.
P2: Failures before the later try leave the controller lease held until its 30-minute TTL, and failures after creating a gate no-op can also leave a planning run open. Wrap all post-acquisition work in cleanup that releases the lease and closes any created run.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/workflows/iris-controller.ts, line 82:
<comment>Failures before the later `try` leave the controller lease held until its 30-minute TTL, and failures after creating a gate no-op can also leave a planning run open. Wrap all post-acquisition work in cleanup that releases the lease and closes any created run.</comment>
<file context>
@@ -0,0 +1,398 @@
+ return { status: "controller_busy", runId: null, reason: null };
+ }
+
+ const context = await loadIrisMandate(organizationId);
+ const mandate = context.mandate;
+ if (!mandate) {
</file context>
| }); | ||
| } | ||
|
|
||
| const runStatus = succeededCount > 0 ? "completed" : "failed"; |
Contributor
There was a problem hiding this comment.
P2: Partial task failures are shown as completed goals/runs whenever one task succeeded, hiding the failed and canceled work from completion state. Mark the goal/run completed only when all planned tasks succeed; otherwise use the appropriate failed or canceled terminal state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/workflows/iris-controller.ts, line 347:
<comment>Partial task failures are shown as completed goals/runs whenever one task succeeded, hiding the failed and canceled work from completion state. Mark the goal/run completed only when all planned tasks succeed; otherwise use the appropriate failed or canceled terminal state.</comment>
<file context>
@@ -0,0 +1,398 @@
+ });
+ }
+
+ const runStatus = succeededCount > 0 ? "completed" : "failed";
+
+ await finalizeIrisRun({
</file context>
| } from "@notra/ai/types/autonomy-capabilities"; | ||
|
|
||
| const IRIS_IMAGE_MARKER_PATTERN = | ||
| /^[^\S\r\n]*\[iris-image:([^\]]*)\][^\S\r\n]*$/gm; |
Contributor
There was a problem hiding this comment.
P2: A malformed placeholder can consume and remove intervening blog text because the description capture permits line breaks. Restrict marker descriptions to a single line so only standalone placeholder lines are resolved.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/utils/iris-image-markers.ts, line 7:
<comment>A malformed placeholder can consume and remove intervening blog text because the description capture permits line breaks. Restrict marker descriptions to a single line so only standalone placeholder lines are resolved.</comment>
<file context>
@@ -0,0 +1,72 @@
+} from "@notra/ai/types/autonomy-capabilities";
+
+const IRIS_IMAGE_MARKER_PATTERN =
+ /^[^\S\r\n]*\[iris-image:([^\]]*)\][^\S\r\n]*$/gm;
+const COLLAPSE_BLANK_LINES_PATTERN = /\n{3,}/g;
+
</file context>
| ttlSeconds: 60, | ||
| organizationId, | ||
| }); | ||
| console.log(`owner A acquires: ${first.claimed}`); |
Contributor
There was a problem hiding this comment.
P2: Claim acquisition failures only print false, so a broken claim path can still make smoke:autonomy exit 0. Assert expected results for acquisition, contention, release, and takeover rather than treating logs as checks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/scripts/autonomy-smoke.ts, line 136:
<comment>Claim acquisition failures only print `false`, so a broken claim path can still make `smoke:autonomy` exit 0. Assert expected results for acquisition, contention, release, and takeover rather than treating logs as checks.</comment>
<file context>
@@ -0,0 +1,197 @@
+ ttlSeconds: 60,
+ organizationId,
+ });
+ console.log(`owner A acquires: ${first.claimed}`);
+
+ const contender = await acquireClaim({
</file context>
Contributor
There was a problem hiding this comment.
37 issues found and verified against the latest diff
Confidence score: 2/5
- In
apps/dashboard/src/workflows/steps/iris-steps.tsandpackages/ai/src/autonomy/leases.ts, lease/fencing handling looks unsafe: expired controllers can keep writing, renewal failures are ignored, and token reuse after cleanup can let stale and current holders look identical, risking duplicate or conflicting finalization—propagate/enforce fencing tokens on every write, fail closed on renewal loss, and keep lease counters monotonic. - In
apps/dashboard/src/lib/iris/mandate.ts, mandate lookup is not scoped to Iris (IRIS_MANDATE_NAME), so pausing Iris may still leave this controller active when another org mandate exists, creating cross-mandate interference—scope the query/check to the Iris mandate only. - In
packages/ai/src/autonomy/planner.tsandapps/dashboard/src/workflows/steps/iris-steps.ts, accounting and guardrails can drift: failed repair paths may consume model calls without billing/spend tracking, a single plan can overshoot daily caps, and image generation may be double-charged—carry usage through error paths, reserve/re-check budget per task/model call, and unify image billing to one path. - In
packages/ai/src/autonomy/signals.tsandpackages/ai/src/utils/iris-poll-granola.ts, signal intake/durability is fragile: coalesced secondary signals can be dropped on failure and enabled Granola sources poll without producing notes, causing missed automation triggers—defer signal removal until durable completion and implement Granola note retrieval/mapping before treating it as active input.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/dashboard/src/workflows/steps/iris-steps.ts">
<violation number="1" location="apps/dashboard/src/workflows/steps/iris-steps.ts:112">
P1: An expired controller can continue generating and finalizing after another controller acquires this lease. The returned fencing token is never applied to downstream writes, and renewal failure is ignored; carry/enforce the token (or stop immediately on failed renewal) before task, run, signal, and outbox mutations.</violation>
<violation number="2" location="apps/dashboard/src/workflows/steps/iris-steps.ts:236">
P1: Daily action and spend limits can be exceeded by a single plan because this gate checks only the pre-plan totals. Reserve budget for the plan or re-check before each task and before model work.</violation>
<violation number="3" location="apps/dashboard/src/workflows/steps/iris-steps.ts:485">
P2: A task that spends on generation then fails while saving reports zero cost, so the run record and billing omit consumed credits. Preserve partial capability cost on failure or meter it before the post-save step.</violation>
<violation number="4" location="apps/dashboard/src/workflows/steps/iris-steps.ts:592">
P1: Blog runs with accepted images charge the image generation twice: once as `marketing_assets` and again in the Iris run total. Bill these image costs through only one of those paths.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/mandate.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/mandate.ts:19">
P1: Pausing Iris can still leave this controller active when another mandate is active for the organization. Scope this lookup to `IRIS_MANDATE_NAME`, matching the router, so the controller loads and checks only Iris's mission.</violation>
</file>
<file name="packages/ai/src/autonomy/planner.ts">
<violation number="1" location="packages/ai/src/autonomy/planner.ts:66">
P1: Invalid model output consumes up to two planner calls but is neither billed nor counted toward the daily spend gate when repair also fails. Carry accrued usage through `IrisPlannerError` and finalize/track the failed run with that cost.</violation>
<violation number="2" location="packages/ai/src/autonomy/planner.ts:129">
P1: Plans can persist a capability version the runtime does not implement, then execute the name-matched implementation anyway. Validate every task's name and exact version against `input.capabilityCatalog` before accepting the plan, including rejecting mandate-allowed capabilities absent from that catalog.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-granola.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-granola.ts:45">
P1: Enabled Granola workspaces are polled but permanently yield no signals, so their notes never reach Iris despite the source being registered in the autonomous polling loop. Implement note retrieval and item mapping before enrolling this source, or omit it until that capability exists.</violation>
</file>
<file name="packages/ai/src/autonomy/leases.ts">
<violation number="1" location="packages/ai/src/autonomy/leases.ts:32">
P1: Fencing tokens reset to `1` after normal release or expiry cleanup, so old and new holders can receive the same token and downstream fencing cannot distinguish them. Preserve the lease counter on release/cleanup or allocate tokens from a monotonic sequence.</violation>
</file>
<file name="packages/ai/src/autonomy/signals.ts">
<violation number="1" location="packages/ai/src/autonomy/signals.ts:188">
P1: A failure after coalescing can permanently drop secondary signals from future runs. They are removed from `listPendingSignals` before the run is durable, and the controller's error path does not reset them; defer this state transition until successful completion or restore these rows on failure.</violation>
</file>
<file name="apps/dashboard/scripts/autonomy-smoke.ts">
<violation number="1" location="apps/dashboard/scripts/autonomy-smoke.ts:19">
P2: This smoke script is coupled to one specific user account, so it fails for other developers/CI even when autonomy logic is healthy. Making the target email configurable keeps the smoke test portable across org datasets.</violation>
<violation number="2" location="apps/dashboard/scripts/autonomy-smoke.ts:105">
P2: The contract negative test is non-blocking, so the smoke run can report success even if cyclic plans start validating. Turning this into an assertion keeps this script useful as a regression gate.</violation>
<violation number="3" location="apps/dashboard/scripts/autonomy-smoke.ts:136">
P1: Claim-flow checks are currently observational, so this smoke test can pass despite claim acquisition/release regressions. Converting expected `claimed` states into throws would make failures fail-fast instead of silent.</violation>
</file>
<file name="packages/ai/src/autonomy/run-store.ts">
<violation number="1" location="packages/ai/src/autonomy/run-store.ts:273">
P1: Workflow retries with an existing `executing` action run the capability again, repeating model/image generation after a crash between execution and `finishAction`. Return a non-runnable outcome for active actions (or atomically claim/take over only stale actions) instead of treating every non-succeeded duplicate as executable.</violation>
</file>
<file name="apps/dashboard/src/lib/webhooks/github.ts">
<violation number="1" location="apps/dashboard/src/lib/webhooks/github.ts:486">
P1: Iris signal ingestion can fail silently while the webhook is still treated as fully processed, so a transient signal-recording/start-run error can permanently skip marketing for that GitHub event. This is caused by awaiting `dispatchIrisGithubSignal` (which catches and suppresses errors) before marking the delivery processed.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/wake-schedule.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/wake-schedule.ts:47">
P1: The QStash wake schedule is created with `getAppUrl()` as the destination, but the receiving route (`apps/dashboard/src/app/api/workflows/iris/route.ts`) verifies the QStash signature against `${getBaseUrl()}${IRIS_WAKE_ROUTE_PATH}`. `getBaseUrl()` returns `getConfiguredWorkflowUrl() ?? getConfiguredAppUrl()` (`packages/ai/src/qstash/triggers.ts`), so whenever a separate `WORKFLOW_BASE_URL` is configured the two URLs differ. QStash signs the delivery URL as part of the signature, so the `Receiver.verify` call in `verifyQstashSignature` would fail against the mismatched URL and every wake delivery would be rejected with 401 — silently breaking the autonomous polling loop. Every other scheduled workflow route (e.g. `/api/workflows/schedule`) uses `getBaseUrl()` consistently for both the destination and the verification URL. Align both sides on the same helper.</violation>
<violation number="2" location="apps/dashboard/src/lib/iris/wake-schedule.ts:70">
P2: A database failure after QStash creates the schedule leaks an active schedule because its ID is never persisted or cleaned up. Compensate by deleting `scheduleId` when `persistScheduleId` fails (or make creation/persistence recoverable) before surfacing the error.</violation>
</file>
<file name="apps/dashboard/src/components/iris/iris-running-state.tsx">
<violation number="1" location="apps/dashboard/src/components/iris/iris-running-state.tsx:41">
P2: A revoked mandate is presented as paused and can be reactivated through Resume. Handle `revoked` separately and avoid wiring it to `onResume`, so revocation remains a distinct terminal state.</violation>
</file>
<file name="packages/ai/src/autonomy/poll.ts">
<violation number="1" location="packages/ai/src/autonomy/poll.ts:64">
P2: A failure in any one integration drops signals from every other source for this wake. Isolating source failures would allow GitHub/Linear results to be recorded while the failed source is reported as skipped; otherwise an outage lasting past the lookback window permanently misses activity.</violation>
</file>
<file name="packages/ai/src/autonomy/capabilities.ts">
<violation number="1" location="packages/ai/src/autonomy/capabilities.ts:368">
P2: Image generation can run against the wrong repository in organizations with multiple GitHub integrations, because repository selection picks the first enabled record instead of the repo tied to current signals. That makes image context nondeterministic and can produce unrelated visuals.</violation>
<violation number="2" location="packages/ai/src/autonomy/capabilities.ts:518">
P2: Image review failures still allow generated images to be accepted and saved, so unreviewed images can be embedded in blog posts when the QC model call fails. This comes from defaulting `accept` to true before checking whether `reviewIrisImage` succeeded.</violation>
</file>
<file name="packages/db/src/schema.ts">
<violation number="1" location="packages/db/src/schema.ts:1831">
P2: Action idempotency is currently global, which can make one organization reuse another organization's existing action when keys match. Including organization_id in the unique constraint keeps idempotency isolation aligned with tenant boundaries.</violation>
<violation number="2" location="packages/db/src/schema.ts:1899">
P1: Outbox dedupe is globally scoped, so one organization's dedupe key can block another organization's message with the same destination/key pair. Scoping this unique index to organization_id keeps dedupe behavior tenant-safe and matches organization-scoped outbox workflows.</violation>
</file>
<file name="apps/dashboard/src/workflows/iris-controller.ts">
<violation number="1" location="apps/dashboard/src/workflows/iris-controller.ts:60">
P1: Duplicate deliveries with the same executionId are not actually deduplicated here, which can start multiple controller runs for the same execution. The claim owner token is deterministic from executionId, and the claim API treats same-owner re-acquire as success.</violation>
</file>
<file name="packages/ai/src/autonomy/deliver.ts">
<violation number="1" location="packages/ai/src/autonomy/deliver.ts:307">
P1: Messages that are torn mid-delivery can get stranded forever. `deliverOutboxMessage` sets the outbox row to `attempting` before posting, but the delivery query in `loadDeliverableRows` only ever re-selects rows with status `pending`. If the process dies after marking `attempting` but before the row resolves to `delivered` / `failed` / `pending` (retry), that message is never re-delivered because no cleanup path considers the `attempting` status. Consider making the picker also target stale `attempting` rows (e.g. treating `attempting` older than some threshold as retryable) so an interrupted delivery is retried instead of permanently stranded.</violation>
</file>
<file name="packages/ai/src/autonomy/ship.ts">
<violation number="1" location="packages/ai/src/autonomy/ship.ts:203">
P2: This path can return `status: "shipped"` even when no publish update happened (race between read and conditional update). Checking update result (e.g., `returning`/row count) and falling back to `already_published` keeps API status aligned with DB state.</violation>
</file>
<file name="apps/agent/agent/lib/utils/iris-interaction.ts">
<violation number="1" location="apps/agent/agent/lib/utils/iris-interaction.ts:202">
P2: Slack can show a post as shipped/skipped even when the outbox record no longer exists, which makes moderation state inaccurate for users. This happens because the non-recorded path logs but still updates the card using the click outcome; returning early for the missing-outbox case avoids false confirmations.</violation>
<violation number="2" location="apps/agent/agent/lib/utils/iris-interaction.ts:217">
P2: If publishing the post fails (the `shipIrisPost` DB update throws `IrisShipError`), `recordIrisApproval` has already persisted a `shipped` approval into the outbox payload before `shipIrisPost` runs. The error propagates and aborts `runIrisInteraction` before `buildDecisionContent`/`updateDecisionCard`, so: (1) the card is left unchanged on the first click; and (2) any later interaction on the same card (even a Skip on another artifact) rebuilds the card from `card.payload.approvals` via `buildDecisionContent` and will show "Shipped by <user>" even though the post is still `draft`. A retry click won't help either, because on the second click `recordIrisApproval` returns `recorded:false` (the approval already exists), so the `shipped` branch is skipped and the post is never actually published. Consider publishing/shipping first and only recording the approval on success, or making the publish independently retryable, or reconciling the card against the real post status rather than the approval record.</violation>
</file>
<file name="packages/ai/src/utils/iris-markdown.ts">
<violation number="1" location="packages/ai/src/utils/iris-markdown.ts:29">
P2: Title extraction can return an empty string when the first non-empty line is only markdown syntax, which drops the fallback title. The sanitized first line needs a non-empty check before returning.</violation>
</file>
<file name="apps/dashboard/src/utils/iris-github-signal.ts">
<violation number="1" location="apps/dashboard/src/utils/iris-github-signal.ts:82">
P2: Push signals are recorded with processing time instead of commit time, so delayed/retried deliveries can appear out of order in Iris timelines and time-window logic. This happens because `resolveIrisGithubOccurredAt` only checks `publishedAt` and `mergedAt` and never reads push commit timestamps.</violation>
</file>
<file name="packages/ai/src/prompts/iris-content.ts">
<violation number="1" location="packages/ai/src/prompts/iris-content.ts:48">
P2: Content prompts can be steered by injected markup/instructions in planner-provided `topic`, `angle`, or `audience`, because those fields are interpolated raw into the XML-like context block. Sanitizing these fields the same way as signal summaries would preserve structure and reduce cross-step prompt injection risk.</violation>
</file>
<file name="apps/agent/agent/channels/slack.ts">
<violation number="1" location="apps/agent/agent/channels/slack.ts:112">
P2: The Ship/Skip handler swallows every failure (only logs a warning and returns normally), so when recording the approval or updating the Slack card fails, the click is acked by Slack with no retry, the user gets no feedback, and the card stays stale in the pending state even though the decision was already persisted. Consider rethrowing to trigger Slack's retry, or posting a fallback message to the thread on failure.</violation>
</file>
<file name="packages/ai/src/utils/iris-image-markers.ts">
<violation number="1" location="packages/ai/src/utils/iris-image-markers.ts:72">
P2: Blog image markdown can render incorrectly when alt text contains markdown control chars, because `buildIrisImageMarkdown` interpolates unescaped `altText` directly into `![...]()`. Escaping bracket/backslash chars in alt text (and wrapping URL in `<...>`) keeps generated posts stable for valid but tricky text.</violation>
</file>
<file name="packages/ai/src/schemas/autonomy/mandate.ts">
<violation number="1" location="packages/ai/src/schemas/autonomy/mandate.ts:26">
P2: Destination policy currently has no effect: a mandate excluding Slack will still enqueue/deliver Slack notifications because delivery hard-codes Slack and never checks `allowedDestinations`. Enforce this allowlist when creating/delivering outbox messages, or remove the field until destination selection supports it.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-linear.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-linear.ts:84">
P2: A second completion of the same Linear issue can be dropped, so Iris may miss later completed-work signals. The dedupe discriminator uses only `issue.id`; including completion timestamp keeps dedupe event-scoped instead of issue-scoped.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-github.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-github.ts:176">
P2: Recent pushes can be ordered behind older work when a commit was authored long before it was pushed, because `occurredAt` uses `head.commit.author?.date`. Using committer timestamp first keeps polling chronology aligned with when changes actually landed.</violation>
</file>
<file name="apps/dashboard/src/schemas/workflows/iris.ts">
<violation number="1" location="apps/dashboard/src/schemas/workflows/iris.ts:16">
P2: The wake-delivery schema currently allows `manual`/`signal`/`repair`, so this endpoint can run non-wake trigger paths even though it always uses a wake execution id. Constraining this field to literal `"wake"` keeps the route contract and downstream run semantics consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }): Promise<{ acquired: boolean; fencingToken: number | null }> { | ||
| "use step"; | ||
| return await Effect.runPromise( | ||
| acquireControllerLease({ |
Contributor
There was a problem hiding this comment.
P1: An expired controller can continue generating and finalizing after another controller acquires this lease. The returned fencing token is never applied to downstream writes, and renewal failure is ignored; carry/enforce the token (or stop immediately on failed renewal) before task, run, signal, and outbox mutations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/workflows/steps/iris-steps.ts, line 112:
<comment>An expired controller can continue generating and finalizing after another controller acquires this lease. The returned fencing token is never applied to downstream writes, and renewal failure is ignored; carry/enforce the token (or stop immediately on failed renewal) before task, run, signal, and outbox mutations.</comment>
<file context>
@@ -0,0 +1,665 @@
+}): Promise<{ acquired: boolean; fencingToken: number | null }> {
+ "use step";
+ return await Effect.runPromise(
+ acquireControllerLease({
+ leaseName: buildControllerLeaseName(input.organizationId),
+ ownerToken: input.ownerToken,
</file context>
| .where( | ||
| and( | ||
| eq(autonomyMandates.organizationId, organizationId), | ||
| eq(autonomyMandates.status, "active") |
Contributor
There was a problem hiding this comment.
P1: Pausing Iris can still leave this controller active when another mandate is active for the organization. Scope this lookup to IRIS_MANDATE_NAME, matching the router, so the controller loads and checks only Iris's mission.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/lib/iris/mandate.ts, line 19:
<comment>Pausing Iris can still leave this controller active when another mandate is active for the organization. Scope this lookup to `IRIS_MANDATE_NAME`, matching the router, so the controller loads and checks only Iris's mission.</comment>
<file context>
@@ -0,0 +1,91 @@
+ .where(
+ and(
+ eq(autonomyMandates.organizationId, organizationId),
+ eq(autonomyMandates.status, "active")
+ )
+ )
</file context>
| }), | ||
| }), | ||
| catch: (cause) => | ||
| new IrisPlannerError({ |
Contributor
There was a problem hiding this comment.
P1: Invalid model output consumes up to two planner calls but is neither billed nor counted toward the daily spend gate when repair also fails. Carry accrued usage through IrisPlannerError and finalize/track the failed run with that cost.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/planner.ts, line 66:
<comment>Invalid model output consumes up to two planner calls but is neither billed nor counted toward the daily spend gate when repair also fails. Carry accrued usage through `IrisPlannerError` and finalize/track the failed run with that cost.</comment>
<file context>
@@ -0,0 +1,209 @@
+ }),
+ }),
+ catch: (cause) =>
+ new IrisPlannerError({
+ message: "The planner model call failed",
+ violations: [],
</file context>
| } | ||
|
|
||
| const errors = [ | ||
| ...validatePlannerOutputAgainstMandate(parsed.data, input.mandate), |
Contributor
There was a problem hiding this comment.
P1: Plans can persist a capability version the runtime does not implement, then execute the name-matched implementation anyway. Validate every task's name and exact version against input.capabilityCatalog before accepting the plan, including rejecting mandate-allowed capabilities absent from that catalog.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/planner.ts, line 129:
<comment>Plans can persist a capability version the runtime does not implement, then execute the name-matched implementation anyway. Validate every task's name and exact version against `input.capabilityCatalog` before accepting the plan, including rejecting mandate-allowed capabilities absent from that catalog.</comment>
<file context>
@@ -0,0 +1,209 @@
+ }
+
+ const errors = [
+ ...validatePlannerOutputAgainstMandate(parsed.data, input.mandate),
+ ...collectTaskParamErrors(parsed.data),
+ ];
</file context>
| }): Promise<{ proceed: boolean; reason: string }> { | ||
| "use step"; | ||
| return await Effect.runPromise( | ||
| evaluateGate({ |
Contributor
There was a problem hiding this comment.
P1: Daily action and spend limits can be exceeded by a single plan because this gate checks only the pre-plan totals. Reserve budget for the plan or re-check before each task and before model work.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/workflows/steps/iris-steps.ts, line 236:
<comment>Daily action and spend limits can be exceeded by a single plan because this gate checks only the pre-plan totals. Reserve budget for the plan or re-check before each task and before model work.</comment>
<file context>
@@ -0,0 +1,665 @@
+}): Promise<{ proceed: boolean; reason: string }> {
+ "use step";
+ return await Effect.runPromise(
+ evaluateGate({
+ mandate: input.mandate,
+ pendingSignalCount: input.pendingSignalCount,
</file context>
| (approval.recorded || approval.existingAction === "shipped"); | ||
|
|
||
| if (shouldPublish) { | ||
| const result = yield* shipIrisPost({ |
Contributor
There was a problem hiding this comment.
P2: If publishing the post fails (the shipIrisPost DB update throws IrisShipError), recordIrisApproval has already persisted a shipped approval into the outbox payload before shipIrisPost runs. The error propagates and aborts runIrisInteraction before buildDecisionContent/updateDecisionCard, so: (1) the card is left unchanged on the first click; and (2) any later interaction on the same card (even a Skip on another artifact) rebuilds the card from card.payload.approvals via buildDecisionContent and will show "Shipped by " even though the post is still draft. A retry click won't help either, because on the second click recordIrisApproval returns recorded:false (the approval already exists), so the shipped branch is skipped and the post is never actually published. Consider publishing/shipping first and only recording the approval on success, or making the publish independently retryable, or reconciling the card against the real post status rather than the approval record.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/lib/utils/iris-interaction.ts, line 213:
<comment>If publishing the post fails (the `shipIrisPost` DB update throws `IrisShipError`), `recordIrisApproval` has already persisted a `shipped` approval into the outbox payload before `shipIrisPost` runs. The error propagates and aborts `runIrisInteraction` before `buildDecisionContent`/`updateDecisionCard`, so: (1) the card is left unchanged on the first click; and (2) any later interaction on the same card (even a Skip on another artifact) rebuilds the card from `card.payload.approvals` via `buildDecisionContent` and will show "Shipped by <user>" even though the post is still `draft`. A retry click won't help either, because on the second click `recordIrisApproval` returns `recorded:false` (the approval already exists), so the `shipped` branch is skipped and the post is never actually published. Consider publishing/shipping first and only recording the approval on success, or making the publish independently retryable, or reconciling the card against the real post status rather than the approval record.</comment>
<file context>
@@ -0,0 +1,271 @@
+ }
+
+ if (approval.recorded && outcome === "shipped") {
+ const result = yield* shipIrisPost({
+ organizationId: parsed.organizationId,
+ postId: parsed.postId,
</file context>
| dedupeHash: computeSignalDedupeHash( | ||
| SIGNAL_SOURCE_LINEAR, | ||
| SIGNAL_KIND_LINEAR_ISSUE_COMPLETED, | ||
| issue.id |
Contributor
There was a problem hiding this comment.
P2: A second completion of the same Linear issue can be dropped, so Iris may miss later completed-work signals. The dedupe discriminator uses only issue.id; including completion timestamp keeps dedupe event-scoped instead of issue-scoped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/utils/iris-poll-linear.ts, line 84:
<comment>A second completion of the same Linear issue can be dropped, so Iris may miss later completed-work signals. The dedupe discriminator uses only `issue.id`; including completion timestamp keeps dedupe event-scoped instead of issue-scoped.</comment>
<file context>
@@ -0,0 +1,138 @@
+ dedupeHash: computeSignalDedupeHash(
+ SIGNAL_SOURCE_LINEAR,
+ SIGNAL_KIND_LINEAR_ISSUE_COMPLETED,
+ issue.id
+ ),
+ sourceEventId: issue.id,
</file context>
| kind: SIGNAL_KIND_GITHUB_PUSH, | ||
| dedupeHash: buildGithubPushSignalHash(repository.id, head.sha), | ||
| sourceEventId: null, | ||
| occurredAt: new Date(head.commit.author?.date ?? Date.now()), |
Contributor
There was a problem hiding this comment.
P2: Recent pushes can be ordered behind older work when a commit was authored long before it was pushed, because occurredAt uses head.commit.author?.date. Using committer timestamp first keeps polling chronology aligned with when changes actually landed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/utils/iris-poll-github.ts, line 176:
<comment>Recent pushes can be ordered behind older work when a commit was authored long before it was pushed, because `occurredAt` uses `head.commit.author?.date`. Using committer timestamp first keeps polling chronology aligned with when changes actually landed.</comment>
<file context>
@@ -0,0 +1,252 @@
+ kind: SIGNAL_KIND_GITHUB_PUSH,
+ dedupeHash: buildGithubPushSignalHash(repository.id, head.sha),
+ sourceEventId: null,
+ occurredAt: new Date(head.commit.author?.date ?? Date.now()),
+ title: `${commits.length} commits on ${branch} in ${repositoryName}`,
+ url: head.html_url,
</file context>
| }); | ||
|
|
||
| for (const integration of integrations) { | ||
| const repository = integration.repositories.at(0); |
Contributor
There was a problem hiding this comment.
P2: Image generation can run against the wrong repository in organizations with multiple GitHub integrations, because repository selection picks the first enabled record instead of the repo tied to current signals. That makes image context nondeterministic and can produce unrelated visuals.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/capabilities.ts, line 368:
<comment>Image generation can run against the wrong repository in organizations with multiple GitHub integrations, because repository selection picks the first enabled record instead of the repo tied to current signals. That makes image context nondeterministic and can produce unrelated visuals.</comment>
<file context>
@@ -0,0 +1,980 @@
+ });
+
+ for (const integration of integrations) {
+ const repository = integration.repositories.at(0);
+ if (
+ !(integration.enabled && repository?.enabled) ||
</file context>
|
|
||
| export const irisWakeDeliverySchema = z.object({ | ||
| organizationId: z.string().trim().min(1), | ||
| trigger: irisTriggerSchema.default("wake"), |
Contributor
There was a problem hiding this comment.
P2: The wake-delivery schema currently allows manual/signal/repair, so this endpoint can run non-wake trigger paths even though it always uses a wake execution id. Constraining this field to literal "wake" keeps the route contract and downstream run semantics consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/schemas/workflows/iris.ts, line 16:
<comment>The wake-delivery schema currently allows `manual`/`signal`/`repair`, so this endpoint can run non-wake trigger paths even though it always uses a wake execution id. Constraining this field to literal `"wake"` keeps the route contract and downstream run semantics consistent.</comment>
<file context>
@@ -0,0 +1,18 @@
+
+export const irisWakeDeliverySchema = z.object({
+ organizationId: z.string().trim().min(1),
+ trigger: irisTriggerSchema.default("wake"),
+});
+export type IrisWakeDelivery = z.infer<typeof irisWakeDeliverySchema>;
</file context>
Contributor
There was a problem hiding this comment.
37 issues found and verified against the latest diff
Confidence score: 2/5
- In
apps/dashboard/src/workflows/steps/iris-steps.tsandpackages/ai/src/autonomy/leases.ts, lease/fencing handling looks unsafe: expired controllers can keep writing, renewal failures are ignored, and token reuse after cleanup can let stale and current holders look identical, risking duplicate or conflicting finalization—propagate/enforce fencing tokens on every write, fail closed on renewal loss, and keep lease counters monotonic. - In
apps/dashboard/src/lib/iris/mandate.ts, mandate lookup is not scoped to Iris (IRIS_MANDATE_NAME), so pausing Iris may still leave this controller active when another org mandate exists, creating cross-mandate interference—scope the query/check to the Iris mandate only. - In
packages/ai/src/autonomy/planner.tsandapps/dashboard/src/workflows/steps/iris-steps.ts, accounting and guardrails can drift: failed repair paths may consume model calls without billing/spend tracking, a single plan can overshoot daily caps, and image generation may be double-charged—carry usage through error paths, reserve/re-check budget per task/model call, and unify image billing to one path. - In
packages/ai/src/autonomy/signals.tsandpackages/ai/src/utils/iris-poll-granola.ts, signal intake/durability is fragile: coalesced secondary signals can be dropped on failure and enabled Granola sources poll without producing notes, causing missed automation triggers—defer signal removal until durable completion and implement Granola note retrieval/mapping before treating it as active input.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/dashboard/src/workflows/steps/iris-steps.ts">
<violation number="1" location="apps/dashboard/src/workflows/steps/iris-steps.ts:112">
P1: An expired controller can continue generating and finalizing after another controller acquires this lease. The returned fencing token is never applied to downstream writes, and renewal failure is ignored; carry/enforce the token (or stop immediately on failed renewal) before task, run, signal, and outbox mutations.</violation>
<violation number="2" location="apps/dashboard/src/workflows/steps/iris-steps.ts:236">
P1: Daily action and spend limits can be exceeded by a single plan because this gate checks only the pre-plan totals. Reserve budget for the plan or re-check before each task and before model work.</violation>
<violation number="3" location="apps/dashboard/src/workflows/steps/iris-steps.ts:485">
P2: A task that spends on generation then fails while saving reports zero cost, so the run record and billing omit consumed credits. Preserve partial capability cost on failure or meter it before the post-save step.</violation>
<violation number="4" location="apps/dashboard/src/workflows/steps/iris-steps.ts:592">
P1: Blog runs with accepted images charge the image generation twice: once as `marketing_assets` and again in the Iris run total. Bill these image costs through only one of those paths.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/mandate.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/mandate.ts:19">
P1: Pausing Iris can still leave this controller active when another mandate is active for the organization. Scope this lookup to `IRIS_MANDATE_NAME`, matching the router, so the controller loads and checks only Iris's mission.</violation>
</file>
<file name="packages/ai/src/autonomy/planner.ts">
<violation number="1" location="packages/ai/src/autonomy/planner.ts:66">
P1: Invalid model output consumes up to two planner calls but is neither billed nor counted toward the daily spend gate when repair also fails. Carry accrued usage through `IrisPlannerError` and finalize/track the failed run with that cost.</violation>
<violation number="2" location="packages/ai/src/autonomy/planner.ts:129">
P1: Plans can persist a capability version the runtime does not implement, then execute the name-matched implementation anyway. Validate every task's name and exact version against `input.capabilityCatalog` before accepting the plan, including rejecting mandate-allowed capabilities absent from that catalog.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-granola.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-granola.ts:45">
P1: Enabled Granola workspaces are polled but permanently yield no signals, so their notes never reach Iris despite the source being registered in the autonomous polling loop. Implement note retrieval and item mapping before enrolling this source, or omit it until that capability exists.</violation>
</file>
<file name="packages/ai/src/autonomy/leases.ts">
<violation number="1" location="packages/ai/src/autonomy/leases.ts:32">
P1: Fencing tokens reset to `1` after normal release or expiry cleanup, so old and new holders can receive the same token and downstream fencing cannot distinguish them. Preserve the lease counter on release/cleanup or allocate tokens from a monotonic sequence.</violation>
</file>
<file name="packages/ai/src/autonomy/signals.ts">
<violation number="1" location="packages/ai/src/autonomy/signals.ts:188">
P1: A failure after coalescing can permanently drop secondary signals from future runs. They are removed from `listPendingSignals` before the run is durable, and the controller's error path does not reset them; defer this state transition until successful completion or restore these rows on failure.</violation>
</file>
<file name="apps/dashboard/scripts/autonomy-smoke.ts">
<violation number="1" location="apps/dashboard/scripts/autonomy-smoke.ts:19">
P2: This smoke script is coupled to one specific user account, so it fails for other developers/CI even when autonomy logic is healthy. Making the target email configurable keeps the smoke test portable across org datasets.</violation>
<violation number="2" location="apps/dashboard/scripts/autonomy-smoke.ts:105">
P2: The contract negative test is non-blocking, so the smoke run can report success even if cyclic plans start validating. Turning this into an assertion keeps this script useful as a regression gate.</violation>
<violation number="3" location="apps/dashboard/scripts/autonomy-smoke.ts:136">
P1: Claim-flow checks are currently observational, so this smoke test can pass despite claim acquisition/release regressions. Converting expected `claimed` states into throws would make failures fail-fast instead of silent.</violation>
</file>
<file name="packages/ai/src/autonomy/run-store.ts">
<violation number="1" location="packages/ai/src/autonomy/run-store.ts:273">
P1: Workflow retries with an existing `executing` action run the capability again, repeating model/image generation after a crash between execution and `finishAction`. Return a non-runnable outcome for active actions (or atomically claim/take over only stale actions) instead of treating every non-succeeded duplicate as executable.</violation>
</file>
<file name="apps/dashboard/src/lib/webhooks/github.ts">
<violation number="1" location="apps/dashboard/src/lib/webhooks/github.ts:486">
P1: Iris signal ingestion can fail silently while the webhook is still treated as fully processed, so a transient signal-recording/start-run error can permanently skip marketing for that GitHub event. This is caused by awaiting `dispatchIrisGithubSignal` (which catches and suppresses errors) before marking the delivery processed.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/wake-schedule.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/wake-schedule.ts:47">
P1: The QStash wake schedule is created with `getAppUrl()` as the destination, but the receiving route (`apps/dashboard/src/app/api/workflows/iris/route.ts`) verifies the QStash signature against `${getBaseUrl()}${IRIS_WAKE_ROUTE_PATH}`. `getBaseUrl()` returns `getConfiguredWorkflowUrl() ?? getConfiguredAppUrl()` (`packages/ai/src/qstash/triggers.ts`), so whenever a separate `WORKFLOW_BASE_URL` is configured the two URLs differ. QStash signs the delivery URL as part of the signature, so the `Receiver.verify` call in `verifyQstashSignature` would fail against the mismatched URL and every wake delivery would be rejected with 401 — silently breaking the autonomous polling loop. Every other scheduled workflow route (e.g. `/api/workflows/schedule`) uses `getBaseUrl()` consistently for both the destination and the verification URL. Align both sides on the same helper.</violation>
<violation number="2" location="apps/dashboard/src/lib/iris/wake-schedule.ts:70">
P2: A database failure after QStash creates the schedule leaks an active schedule because its ID is never persisted or cleaned up. Compensate by deleting `scheduleId` when `persistScheduleId` fails (or make creation/persistence recoverable) before surfacing the error.</violation>
</file>
<file name="apps/dashboard/src/components/iris/iris-running-state.tsx">
<violation number="1" location="apps/dashboard/src/components/iris/iris-running-state.tsx:41">
P2: A revoked mandate is presented as paused and can be reactivated through Resume. Handle `revoked` separately and avoid wiring it to `onResume`, so revocation remains a distinct terminal state.</violation>
</file>
<file name="packages/ai/src/autonomy/poll.ts">
<violation number="1" location="packages/ai/src/autonomy/poll.ts:64">
P2: A failure in any one integration drops signals from every other source for this wake. Isolating source failures would allow GitHub/Linear results to be recorded while the failed source is reported as skipped; otherwise an outage lasting past the lookback window permanently misses activity.</violation>
</file>
<file name="packages/ai/src/autonomy/capabilities.ts">
<violation number="1" location="packages/ai/src/autonomy/capabilities.ts:368">
P2: Image generation can run against the wrong repository in organizations with multiple GitHub integrations, because repository selection picks the first enabled record instead of the repo tied to current signals. That makes image context nondeterministic and can produce unrelated visuals.</violation>
<violation number="2" location="packages/ai/src/autonomy/capabilities.ts:518">
P2: Image review failures still allow generated images to be accepted and saved, so unreviewed images can be embedded in blog posts when the QC model call fails. This comes from defaulting `accept` to true before checking whether `reviewIrisImage` succeeded.</violation>
</file>
<file name="packages/db/src/schema.ts">
<violation number="1" location="packages/db/src/schema.ts:1831">
P2: Action idempotency is currently global, which can make one organization reuse another organization's existing action when keys match. Including organization_id in the unique constraint keeps idempotency isolation aligned with tenant boundaries.</violation>
<violation number="2" location="packages/db/src/schema.ts:1899">
P1: Outbox dedupe is globally scoped, so one organization's dedupe key can block another organization's message with the same destination/key pair. Scoping this unique index to organization_id keeps dedupe behavior tenant-safe and matches organization-scoped outbox workflows.</violation>
</file>
<file name="apps/dashboard/src/workflows/iris-controller.ts">
<violation number="1" location="apps/dashboard/src/workflows/iris-controller.ts:60">
P1: Duplicate deliveries with the same executionId are not actually deduplicated here, which can start multiple controller runs for the same execution. The claim owner token is deterministic from executionId, and the claim API treats same-owner re-acquire as success.</violation>
</file>
<file name="packages/ai/src/autonomy/deliver.ts">
<violation number="1" location="packages/ai/src/autonomy/deliver.ts:307">
P1: Messages that are torn mid-delivery can get stranded forever. `deliverOutboxMessage` sets the outbox row to `attempting` before posting, but the delivery query in `loadDeliverableRows` only ever re-selects rows with status `pending`. If the process dies after marking `attempting` but before the row resolves to `delivered` / `failed` / `pending` (retry), that message is never re-delivered because no cleanup path considers the `attempting` status. Consider making the picker also target stale `attempting` rows (e.g. treating `attempting` older than some threshold as retryable) so an interrupted delivery is retried instead of permanently stranded.</violation>
</file>
<file name="packages/ai/src/autonomy/ship.ts">
<violation number="1" location="packages/ai/src/autonomy/ship.ts:203">
P2: This path can return `status: "shipped"` even when no publish update happened (race between read and conditional update). Checking update result (e.g., `returning`/row count) and falling back to `already_published` keeps API status aligned with DB state.</violation>
</file>
<file name="apps/agent/agent/lib/utils/iris-interaction.ts">
<violation number="1" location="apps/agent/agent/lib/utils/iris-interaction.ts:202">
P2: Slack can show a post as shipped/skipped even when the outbox record no longer exists, which makes moderation state inaccurate for users. This happens because the non-recorded path logs but still updates the card using the click outcome; returning early for the missing-outbox case avoids false confirmations.</violation>
<violation number="2" location="apps/agent/agent/lib/utils/iris-interaction.ts:217">
P2: If publishing the post fails (the `shipIrisPost` DB update throws `IrisShipError`), `recordIrisApproval` has already persisted a `shipped` approval into the outbox payload before `shipIrisPost` runs. The error propagates and aborts `runIrisInteraction` before `buildDecisionContent`/`updateDecisionCard`, so: (1) the card is left unchanged on the first click; and (2) any later interaction on the same card (even a Skip on another artifact) rebuilds the card from `card.payload.approvals` via `buildDecisionContent` and will show "Shipped by <user>" even though the post is still `draft`. A retry click won't help either, because on the second click `recordIrisApproval` returns `recorded:false` (the approval already exists), so the `shipped` branch is skipped and the post is never actually published. Consider publishing/shipping first and only recording the approval on success, or making the publish independently retryable, or reconciling the card against the real post status rather than the approval record.</violation>
</file>
<file name="packages/ai/src/utils/iris-markdown.ts">
<violation number="1" location="packages/ai/src/utils/iris-markdown.ts:29">
P2: Title extraction can return an empty string when the first non-empty line is only markdown syntax, which drops the fallback title. The sanitized first line needs a non-empty check before returning.</violation>
</file>
<file name="apps/dashboard/src/utils/iris-github-signal.ts">
<violation number="1" location="apps/dashboard/src/utils/iris-github-signal.ts:82">
P2: Push signals are recorded with processing time instead of commit time, so delayed/retried deliveries can appear out of order in Iris timelines and time-window logic. This happens because `resolveIrisGithubOccurredAt` only checks `publishedAt` and `mergedAt` and never reads push commit timestamps.</violation>
</file>
<file name="packages/ai/src/prompts/iris-content.ts">
<violation number="1" location="packages/ai/src/prompts/iris-content.ts:48">
P2: Content prompts can be steered by injected markup/instructions in planner-provided `topic`, `angle`, or `audience`, because those fields are interpolated raw into the XML-like context block. Sanitizing these fields the same way as signal summaries would preserve structure and reduce cross-step prompt injection risk.</violation>
</file>
<file name="apps/agent/agent/channels/slack.ts">
<violation number="1" location="apps/agent/agent/channels/slack.ts:112">
P2: The Ship/Skip handler swallows every failure (only logs a warning and returns normally), so when recording the approval or updating the Slack card fails, the click is acked by Slack with no retry, the user gets no feedback, and the card stays stale in the pending state even though the decision was already persisted. Consider rethrowing to trigger Slack's retry, or posting a fallback message to the thread on failure.</violation>
</file>
<file name="packages/ai/src/utils/iris-image-markers.ts">
<violation number="1" location="packages/ai/src/utils/iris-image-markers.ts:72">
P2: Blog image markdown can render incorrectly when alt text contains markdown control chars, because `buildIrisImageMarkdown` interpolates unescaped `altText` directly into `![...]()`. Escaping bracket/backslash chars in alt text (and wrapping URL in `<...>`) keeps generated posts stable for valid but tricky text.</violation>
</file>
<file name="packages/ai/src/schemas/autonomy/mandate.ts">
<violation number="1" location="packages/ai/src/schemas/autonomy/mandate.ts:26">
P2: Destination policy currently has no effect: a mandate excluding Slack will still enqueue/deliver Slack notifications because delivery hard-codes Slack and never checks `allowedDestinations`. Enforce this allowlist when creating/delivering outbox messages, or remove the field until destination selection supports it.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-linear.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-linear.ts:84">
P2: A second completion of the same Linear issue can be dropped, so Iris may miss later completed-work signals. The dedupe discriminator uses only `issue.id`; including completion timestamp keeps dedupe event-scoped instead of issue-scoped.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-github.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-github.ts:176">
P2: Recent pushes can be ordered behind older work when a commit was authored long before it was pushed, because `occurredAt` uses `head.commit.author?.date`. Using committer timestamp first keeps polling chronology aligned with when changes actually landed.</violation>
</file>
<file name="apps/dashboard/src/schemas/workflows/iris.ts">
<violation number="1" location="apps/dashboard/src/schemas/workflows/iris.ts:16">
P2: The wake-delivery schema currently allows `manual`/`signal`/`repair`, so this endpoint can run non-wake trigger paths even though it always uses a wake execution id. Constraining this field to literal `"wake"` keeps the route contract and downstream run semantics consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }): Promise<{ acquired: boolean; fencingToken: number | null }> { | ||
| "use step"; | ||
| return await Effect.runPromise( | ||
| acquireControllerLease({ |
Contributor
There was a problem hiding this comment.
P1: An expired controller can continue generating and finalizing after another controller acquires this lease. The returned fencing token is never applied to downstream writes, and renewal failure is ignored; carry/enforce the token (or stop immediately on failed renewal) before task, run, signal, and outbox mutations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/workflows/steps/iris-steps.ts, line 112:
<comment>An expired controller can continue generating and finalizing after another controller acquires this lease. The returned fencing token is never applied to downstream writes, and renewal failure is ignored; carry/enforce the token (or stop immediately on failed renewal) before task, run, signal, and outbox mutations.</comment>
<file context>
@@ -0,0 +1,665 @@
+}): Promise<{ acquired: boolean; fencingToken: number | null }> {
+ "use step";
+ return await Effect.runPromise(
+ acquireControllerLease({
+ leaseName: buildControllerLeaseName(input.organizationId),
+ ownerToken: input.ownerToken,
</file context>
| .where( | ||
| and( | ||
| eq(autonomyMandates.organizationId, organizationId), | ||
| eq(autonomyMandates.status, "active") |
Contributor
There was a problem hiding this comment.
P1: Pausing Iris can still leave this controller active when another mandate is active for the organization. Scope this lookup to IRIS_MANDATE_NAME, matching the router, so the controller loads and checks only Iris's mission.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/lib/iris/mandate.ts, line 19:
<comment>Pausing Iris can still leave this controller active when another mandate is active for the organization. Scope this lookup to `IRIS_MANDATE_NAME`, matching the router, so the controller loads and checks only Iris's mission.</comment>
<file context>
@@ -0,0 +1,91 @@
+ .where(
+ and(
+ eq(autonomyMandates.organizationId, organizationId),
+ eq(autonomyMandates.status, "active")
+ )
+ )
</file context>
| }), | ||
| }), | ||
| catch: (cause) => | ||
| new IrisPlannerError({ |
Contributor
There was a problem hiding this comment.
P1: Invalid model output consumes up to two planner calls but is neither billed nor counted toward the daily spend gate when repair also fails. Carry accrued usage through IrisPlannerError and finalize/track the failed run with that cost.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/planner.ts, line 66:
<comment>Invalid model output consumes up to two planner calls but is neither billed nor counted toward the daily spend gate when repair also fails. Carry accrued usage through `IrisPlannerError` and finalize/track the failed run with that cost.</comment>
<file context>
@@ -0,0 +1,209 @@
+ }),
+ }),
+ catch: (cause) =>
+ new IrisPlannerError({
+ message: "The planner model call failed",
+ violations: [],
</file context>
| } | ||
|
|
||
| const errors = [ | ||
| ...validatePlannerOutputAgainstMandate(parsed.data, input.mandate), |
Contributor
There was a problem hiding this comment.
P1: Plans can persist a capability version the runtime does not implement, then execute the name-matched implementation anyway. Validate every task's name and exact version against input.capabilityCatalog before accepting the plan, including rejecting mandate-allowed capabilities absent from that catalog.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/planner.ts, line 129:
<comment>Plans can persist a capability version the runtime does not implement, then execute the name-matched implementation anyway. Validate every task's name and exact version against `input.capabilityCatalog` before accepting the plan, including rejecting mandate-allowed capabilities absent from that catalog.</comment>
<file context>
@@ -0,0 +1,209 @@
+ }
+
+ const errors = [
+ ...validatePlannerOutputAgainstMandate(parsed.data, input.mandate),
+ ...collectTaskParamErrors(parsed.data),
+ ];
</file context>
| }): Promise<{ proceed: boolean; reason: string }> { | ||
| "use step"; | ||
| return await Effect.runPromise( | ||
| evaluateGate({ |
Contributor
There was a problem hiding this comment.
P1: Daily action and spend limits can be exceeded by a single plan because this gate checks only the pre-plan totals. Reserve budget for the plan or re-check before each task and before model work.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/workflows/steps/iris-steps.ts, line 236:
<comment>Daily action and spend limits can be exceeded by a single plan because this gate checks only the pre-plan totals. Reserve budget for the plan or re-check before each task and before model work.</comment>
<file context>
@@ -0,0 +1,665 @@
+}): Promise<{ proceed: boolean; reason: string }> {
+ "use step";
+ return await Effect.runPromise(
+ evaluateGate({
+ mandate: input.mandate,
+ pendingSignalCount: input.pendingSignalCount,
</file context>
| (approval.recorded || approval.existingAction === "shipped"); | ||
|
|
||
| if (shouldPublish) { | ||
| const result = yield* shipIrisPost({ |
Contributor
There was a problem hiding this comment.
P2: If publishing the post fails (the shipIrisPost DB update throws IrisShipError), recordIrisApproval has already persisted a shipped approval into the outbox payload before shipIrisPost runs. The error propagates and aborts runIrisInteraction before buildDecisionContent/updateDecisionCard, so: (1) the card is left unchanged on the first click; and (2) any later interaction on the same card (even a Skip on another artifact) rebuilds the card from card.payload.approvals via buildDecisionContent and will show "Shipped by " even though the post is still draft. A retry click won't help either, because on the second click recordIrisApproval returns recorded:false (the approval already exists), so the shipped branch is skipped and the post is never actually published. Consider publishing/shipping first and only recording the approval on success, or making the publish independently retryable, or reconciling the card against the real post status rather than the approval record.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/lib/utils/iris-interaction.ts, line 213:
<comment>If publishing the post fails (the `shipIrisPost` DB update throws `IrisShipError`), `recordIrisApproval` has already persisted a `shipped` approval into the outbox payload before `shipIrisPost` runs. The error propagates and aborts `runIrisInteraction` before `buildDecisionContent`/`updateDecisionCard`, so: (1) the card is left unchanged on the first click; and (2) any later interaction on the same card (even a Skip on another artifact) rebuilds the card from `card.payload.approvals` via `buildDecisionContent` and will show "Shipped by <user>" even though the post is still `draft`. A retry click won't help either, because on the second click `recordIrisApproval` returns `recorded:false` (the approval already exists), so the `shipped` branch is skipped and the post is never actually published. Consider publishing/shipping first and only recording the approval on success, or making the publish independently retryable, or reconciling the card against the real post status rather than the approval record.</comment>
<file context>
@@ -0,0 +1,271 @@
+ }
+
+ if (approval.recorded && outcome === "shipped") {
+ const result = yield* shipIrisPost({
+ organizationId: parsed.organizationId,
+ postId: parsed.postId,
</file context>
| dedupeHash: computeSignalDedupeHash( | ||
| SIGNAL_SOURCE_LINEAR, | ||
| SIGNAL_KIND_LINEAR_ISSUE_COMPLETED, | ||
| issue.id |
Contributor
There was a problem hiding this comment.
P2: A second completion of the same Linear issue can be dropped, so Iris may miss later completed-work signals. The dedupe discriminator uses only issue.id; including completion timestamp keeps dedupe event-scoped instead of issue-scoped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/utils/iris-poll-linear.ts, line 84:
<comment>A second completion of the same Linear issue can be dropped, so Iris may miss later completed-work signals. The dedupe discriminator uses only `issue.id`; including completion timestamp keeps dedupe event-scoped instead of issue-scoped.</comment>
<file context>
@@ -0,0 +1,138 @@
+ dedupeHash: computeSignalDedupeHash(
+ SIGNAL_SOURCE_LINEAR,
+ SIGNAL_KIND_LINEAR_ISSUE_COMPLETED,
+ issue.id
+ ),
+ sourceEventId: issue.id,
</file context>
| kind: SIGNAL_KIND_GITHUB_PUSH, | ||
| dedupeHash: buildGithubPushSignalHash(repository.id, head.sha), | ||
| sourceEventId: null, | ||
| occurredAt: new Date(head.commit.author?.date ?? Date.now()), |
Contributor
There was a problem hiding this comment.
P2: Recent pushes can be ordered behind older work when a commit was authored long before it was pushed, because occurredAt uses head.commit.author?.date. Using committer timestamp first keeps polling chronology aligned with when changes actually landed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/utils/iris-poll-github.ts, line 176:
<comment>Recent pushes can be ordered behind older work when a commit was authored long before it was pushed, because `occurredAt` uses `head.commit.author?.date`. Using committer timestamp first keeps polling chronology aligned with when changes actually landed.</comment>
<file context>
@@ -0,0 +1,252 @@
+ kind: SIGNAL_KIND_GITHUB_PUSH,
+ dedupeHash: buildGithubPushSignalHash(repository.id, head.sha),
+ sourceEventId: null,
+ occurredAt: new Date(head.commit.author?.date ?? Date.now()),
+ title: `${commits.length} commits on ${branch} in ${repositoryName}`,
+ url: head.html_url,
</file context>
| }); | ||
|
|
||
| for (const integration of integrations) { | ||
| const repository = integration.repositories.at(0); |
Contributor
There was a problem hiding this comment.
P2: Image generation can run against the wrong repository in organizations with multiple GitHub integrations, because repository selection picks the first enabled record instead of the repo tied to current signals. That makes image context nondeterministic and can produce unrelated visuals.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/capabilities.ts, line 368:
<comment>Image generation can run against the wrong repository in organizations with multiple GitHub integrations, because repository selection picks the first enabled record instead of the repo tied to current signals. That makes image context nondeterministic and can produce unrelated visuals.</comment>
<file context>
@@ -0,0 +1,980 @@
+ });
+
+ for (const integration of integrations) {
+ const repository = integration.repositories.at(0);
+ if (
+ !(integration.enabled && repository?.enabled) ||
</file context>
|
|
||
| export const irisWakeDeliverySchema = z.object({ | ||
| organizationId: z.string().trim().min(1), | ||
| trigger: irisTriggerSchema.default("wake"), |
Contributor
There was a problem hiding this comment.
P2: The wake-delivery schema currently allows manual/signal/repair, so this endpoint can run non-wake trigger paths even though it always uses a wake execution id. Constraining this field to literal "wake" keeps the route contract and downstream run semantics consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/schemas/workflows/iris.ts, line 16:
<comment>The wake-delivery schema currently allows `manual`/`signal`/`repair`, so this endpoint can run non-wake trigger paths even though it always uses a wake execution id. Constraining this field to literal `"wake"` keeps the route contract and downstream run semantics consistent.</comment>
<file context>
@@ -0,0 +1,18 @@
+
+export const irisWakeDeliverySchema = z.object({
+ organizationId: z.string().trim().min(1),
+ trigger: irisTriggerSchema.default("wake"),
+});
+export type IrisWakeDelivery = z.infer<typeof irisWakeDeliverySchema>;
</file context>
There was a problem hiding this comment.
3 issues found across 2 files.
Prompt for AI agents (all issues)
Check whether each issue below is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it, and use sub-agents to handle independent issues in parallel.
<file name="apps/agent/agent/lib/utils/iris-interaction.ts">
<issue n="1" at="apps/agent/agent/lib/utils/iris-interaction.ts:109-208" severity="MEDIUM">Slack interaction handler publishes posts without per-user authorization — handleIrisInteraction processes "Ship it" / "Skip" button clicks from Slack and, for "Ship it", calls shipIrisPost which flips a post's status from draft to published — a privileged content-publishing operation. The only authorization performed is an organization-mismatch check (installation.organizationId === parsed.organizationId), which verifies that the Slack workspace (teamId) maps to the same organization embedded in the button value. There is NO check that the clicking Slack user (action.user.id / action.user.username) is an authorized member of that organization or has publish rights. In the dashboard, every equivalent operation is gated by assertOrganizationAccess + authorizedProcedure (see apps/dashboard/src/lib/orpc/routers/content.ts and iris.ts runNow), verifying the authenticated dashboard user has access to the organization. The Slack path bypasses this entirely, treating Slack workspace membership as equivalent to organization publish authorization. Slack workspaces routinely include members who should not be able to publish content on behalf of the org (guests, contractors, multi-channel guests, new hires). Any such member who can see the Iris notification card (posted to the notification channel) can click "Ship it" and publish draft content publicly. Note also that allowedChannelIds (the per-integration channel allowlist used in onSlackMessage via isChannelAllowed) is NOT consulted on the interaction path — onSlackInteraction dispatches directly to handleIrisInteraction with no channel check, so the channel restriction does not mitigate this. The request itself is Slack-signature-verified (SLACK_AGENT_SIGNING_SECRET in slack-credentials.ts), so the actor is a genuine workspace member, but that is a weaker boundary than the dashboard's per-user organization access check. Evidence: apps/agent/agent/channels/slack.ts onSlackInteraction (lines 109-112) calls handleIrisInteraction directly; handleIrisInteraction -> runIrisInteraction only checks parsed && teamId and installation?.organizationId !== parsed.organizationId (lines ~175, ~183); recordIrisApproval/shipIrisPost use parsed.organizationId (server-derived from the button) and input.actorId is only stored as audit metadata, never checked for authorization. Fix: Before recording the approval / shipping, verify that the Slack user (action.user.id) is authorized for the target organization. Options: (1) resolve the Slack user to a Notra user via the organization's Slack integration and run the same assertOrganizationAccess-style check used by the dashboard; (2) maintain an allowlist of Slack user IDs permitted to approve/ship Iris artifacts and reject others; (3) at minimum, apply isChannelAllowed on the interaction channel so interactions from non-allowed channels are rejected. Do not rely on Slack workspace membership alone as the authorization boundary for publishing content.</issue>
<issue n="2" at="apps/agent/agent/lib/utils/iris-interaction.ts:192-208" severity="MEDIUM">Non-atomic read-modify-write of outbox approvals can flip a ship/skip decision (lost update) — recordIrisApproval (packages/ai/src/autonomy/ship.ts) performs a non-atomic read-modify-write on the approvals array stored in the autonomyOutbox.payload JSONB column. It SELECTs the current payload, parses the approvals array, checks for an existing entry for the postId, and if none exists appends a new approval and UPDATEs the whole payload with `{ ...basePayload, approvals: appended }`. There is no transaction, no SELECT ... FOR UPDATE, and no optimistic concurrency control (no version/etag check). If two Slack users click buttons on the same outbox card concurrently (e.g., one clicks Skip and one clicks Ship), both interactions can read the approvals array before either write lands. Both then append and overwrite the full payload; the second writer wins and the first approval is silently lost. In a Skip-then-Ship (or Ship-then-Skip) race this does not merely lose audit data — it can flip the recorded decision outcome: the shouldPublish logic in iris-interaction.ts (outcome === 'shipped' && (approval.recorded || approval.existingAction === 'shipped')) will publish the post based on whichever approval survives the race, which may not be the decision that was actually intended/first. The post-publish itself is made idempotent at the DB layer (shipIrisPost updates WHERE status = 'draft'), but the decision-affecting race on the approvals array is not protected. Evidence: packages/ai/src/autonomy/ship.ts recordIrisApproval — separate SELECT (lines ~24-44) then UPDATE (lines ~63-78) with no transaction; payload column is jsonb with no version column (packages/db/src/schema.ts line 1885). Fix: Wrap the read-modify-write of the approvals array in a serializable transaction (or SELECT ... FOR UPDATE on the outbox row) so concurrent approvals serialize correctly. Alternatively, store approvals in a separate table with a unique constraint on (outboxId, postId) so duplicate approvals are rejected at the DB layer and the existing-entry check is atomic with the insert. Ensure the ship/skip decision is determined by the committed approvals state, not by an in-memory copy read before the write.</issue>
</file>
<file name="apps/dashboard/src/workflows/iris-controller.ts">
<issue n="3" at="apps/dashboard/src/workflows/iris-controller.ts:160-427" severity="MEDIUM">Controller lease not released if createIrisRun throws after the gate passes — In irisControllerRun, the lease is acquired before the gate check. When the gate proceeds, createIrisRun is called (line ~160) to create the actual run record, and only afterward does a try block begin (line ~168) whose catch releases the lease and closes the open run. If createIrisRun throws (DB error, transient failure), execution escapes the function without entering the try, so releaseIrisLease is never called and markIrisSignalsProcessed is never called. The lease will eventually expire via its TTL and the signals remain pending (which is arguably correct for a retry), but the lease is held longer than necessary and there is no closeOpenIrisRun cleanup for the not-yet-created run. Contrast with the gate-blocked and plan-rejected paths, which explicitly release the lease. The no_active_mandate path also releases explicitly. This is a minor resource-leak / cleanup-gap, not a security issue: the lease TTL bounds the leak and a duplicate execution claim prevents concurrent controllers. But it is an inconsistency in cleanup handling. Fix: Move createIrisRun inside the try block, or wrap the entire post-gate body (including createIrisRun) in the try/catch so that any throw after the lease is acquired releases the lease. Alternatively, use a finally block to release the lease on every exit path (excluding the intentional leaseLost path where another controller now owns the lease).</issue>
</file>
1 other commit review still in progress for this PR — findings may follow.
Commit 8a6cf96 · Posted by Comp AI Code Reviews.
Comment on lines
+109
to
+208
| ? `Shipped by ${decidedBy}.` | ||
| : `Skipped by ${decidedBy}.`, | ||
| blocks: [], | ||
| }; | ||
| return fallback; | ||
| } | ||
|
|
||
| const shippedPostIds = new Set( | ||
| input.approvals | ||
| .filter((approval) => approval.action === "shipped") | ||
| .map((approval) => approval.postId) | ||
| ); | ||
|
|
||
| return buildShippedBlocks({ | ||
| organizationId: input.value.organizationId, | ||
| organizationSlug: card.organizationSlug, | ||
| outboxId: card.row.id, | ||
| runId: card.payload.runId, | ||
| headline: card.payload.headline, | ||
| signalCount: card.payload.signalCount, | ||
| trigger: card.payload.trigger, | ||
| artifacts: card.payload.artifacts.map((artifact) => | ||
| shippedPostIds.has(artifact.postId) | ||
| ? { ...artifact, status: "published" } | ||
| : artifact | ||
| ), | ||
| decisions: | ||
| decisions.length > 0 | ||
| ? decisions | ||
| : [ | ||
| { | ||
| postId: input.value.postId, | ||
| decidedBy, | ||
| outcome: decidedOutcome, | ||
| }, | ||
| ], | ||
| }); | ||
| } | ||
| ); | ||
|
|
||
| const runIrisInteraction = Effect.fn("agent.iris.interaction")( | ||
| function* (input: { | ||
| actionId: string; | ||
| value: string | undefined; | ||
| messageTs: string | undefined; | ||
| channelId: string; | ||
| teamId: string | undefined; | ||
| actorId: string; | ||
| actorName: string | null; | ||
| }) { | ||
| const parsed = parseIrisInteractionValue(input.value); | ||
| const teamId = input.teamId; | ||
|
|
||
| if (!(parsed && teamId)) { | ||
| yield* Effect.logWarning("Iris interaction payload was not usable").pipe( | ||
| Effect.annotateLogs({ actionId: input.actionId }) | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const installation = yield* Effect.tryPromise({ | ||
| try: () => resolveSlackInstallation(teamId), | ||
| catch: (cause) => | ||
| new IrisInteractionError({ | ||
| operation: "resolveInstallation", | ||
| cause, | ||
| }), | ||
| }); | ||
|
|
||
| if (installation?.organizationId !== parsed.organizationId) { | ||
| yield* Effect.logWarning( | ||
| "Iris interaction rejected for an organization mismatch" | ||
| ).pipe( | ||
| Effect.annotateLogs({ | ||
| teamId, | ||
| actionOrganizationId: parsed.organizationId, | ||
| }) | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const outcome: IrisDecisionOutcome = | ||
| input.actionId === IRIS_SHIP_ACTION_ID ? "shipped" : "skipped"; | ||
|
|
||
| const approval = yield* recordIrisApproval({ | ||
| organizationId: parsed.organizationId, | ||
| outboxId: parsed.outboxId, | ||
| postId: parsed.postId, | ||
| action: outcome, | ||
| slackUserId: input.actorId, | ||
| slackUserName: input.actorName, | ||
| }); | ||
|
|
||
| if (!approval.recorded) { | ||
| yield* Effect.logInfo("Iris decision already recorded").pipe( | ||
| Effect.annotateLogs({ | ||
| organizationId: parsed.organizationId, | ||
| postId: parsed.postId, | ||
| existingAction: approval.existingAction ?? "none", | ||
| }) |
There was a problem hiding this comment.
MEDIUM: Slack interaction handler publishes posts without per-user authorization
handleIrisInteraction processes "Ship it" / "Skip" button clicks from Slack and, for "Ship it", calls shipIrisPost which flips a post's status from draft to published — a privileged content-publishing operation. The only authorization performed is an organization-mismatch check (installation.organizationId === parsed.organizationId), which verifies that the Slack workspace (teamId) maps to the same organization embedded in the button value. There is NO check that the clicking Slack user (action.user.id / action.user.username) is an authorized member of that organization or has publish rights.
In the dashboard, every equivalent operation is gated by assertOrganizationAccess + authorizedProcedure (see apps/dashboard/src/lib/orpc/routers/content.ts and iris.ts runNow), verifying the authenticated dashboard user has access to the organization. The Slack path bypasses this entirely, treating Slack workspace membership as equivalent to organization publish authorization. Slack workspaces routinely include members who should not be able to publish content on behalf of the org (guests, contractors, multi-channel guests, new hires). Any such member who can see the Iris notification card (posted to the notification channel) can click "Ship it" and publish draft content publicly.
Note also that allowedChannelIds (the per-integration channel allowlist used in onSlackMessage via isChannelAllowed) is NOT consulted on the interaction path — onSlackInteraction dispatches directly to handleIrisInteraction with no channel check, so the channel restriction does not mitigate this. The request itself is Slack-signature-verified (SLACK_AGENT_SIGNING_SECRET in slack-credentials.ts), so the actor is a genuine workspace member, but that is a weaker boundary than the dashboard's per-user organization access check.
Evidence: apps/agent/agent/channels/slack.ts onSlackInteraction (lines 109-112) calls handleIrisInteraction directly; handleIrisInteraction -> runIrisInteraction only checks parsed && teamId and installation?.organizationId !== parsed.organizationId (lines ~175, ~183); recordIrisApproval/shipIrisPost use parsed.organizationId (server-derived from the button) and input.actorId is only stored as audit metadata, never checked for authorization.
Suggestion: Before recording the approval / shipping, verify that the Slack user (action.user.id) is authorized for the target organization. Options: (1) resolve the Slack user to a Notra user via the organization's Slack integration and run the same assertOrganizationAccess-style check used by the dashboard; (2) maintain an allowlist of Slack user IDs permitted to approve/ship Iris artifacts and reject others; (3) at minimum, apply isChannelAllowed on the interaction channel so interactions from non-allowed channels are rejected. Do not rely on Slack workspace membership alone as the authorization boundary for publishing content.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="apps/agent/agent/lib/utils/iris-interaction.ts:109-208" severity="MEDIUM">Slack interaction handler publishes posts without per-user authorization — handleIrisInteraction processes "Ship it" / "Skip" button clicks from Slack and, for "Ship it", calls shipIrisPost which flips a post's status from draft to published — a privileged content-publishing operation. The only authorization performed is an organization-mismatch check (installation.organizationId === parsed.organizationId), which verifies that the Slack workspace (teamId) maps to the same organization embedded in the button value. There is NO check that the clicking Slack user (action.user.id / action.user.username) is an authorized member of that organization or has publish rights. In the dashboard, every equivalent operation is gated by assertOrganizationAccess + authorizedProcedure (see apps/dashboard/src/lib/orpc/routers/content.ts and iris.ts runNow), verifying the authenticated dashboard user has access to the organization. The Slack path bypasses this entirely, treating Slack workspace membership as equivalent to organization publish authorization. Slack workspaces routinely include members who should not be able to publish content on behalf of the org (guests, contractors, multi-channel guests, new hires). Any such member who can see the Iris notification card (posted to the notification channel) can click "Ship it" and publish draft content publicly. Note also that allowedChannelIds (the per-integration channel allowlist used in onSlackMessage via isChannelAllowed) is NOT consulted on the interaction path — onSlackInteraction dispatches directly to handleIrisInteraction with no channel check, so the channel restriction does not mitigate this. The request itself is Slack-signature-verified (SLACK_AGENT_SIGNING_SECRET in slack-credentials.ts), so the actor is a genuine workspace member, but that is a weaker boundary than the dashboard's per-user organization access check. Evidence: apps/agent/agent/channels/slack.ts onSlackInteraction (lines 109-112) calls handleIrisInteraction directly; handleIrisInteraction -> runIrisInteraction only checks parsed && teamId and installation?.organizationId !== parsed.organizationId (lines ~175, ~183); recordIrisApproval/shipIrisPost use parsed.organizationId (server-derived from the button) and input.actorId is only stored as audit metadata, never checked for authorization. Fix: Before recording the approval / shipping, verify that the Slack user (action.user.id) is authorized for the target organization. Options: (1) resolve the Slack user to a Notra user via the organization's Slack integration and run the same assertOrganizationAccess-style check used by the dashboard; (2) maintain an allowlist of Slack user IDs permitted to approve/ship Iris artifacts and reject others; (3) at minimum, apply isChannelAllowed on the interaction channel so interactions from non-allowed channels are rejected. Do not rely on Slack workspace membership alone as the authorization boundary for publishing content.</issue>
Commit 8a6cf96.
Comment on lines
+192
to
+208
|
|
||
| const approval = yield* recordIrisApproval({ | ||
| organizationId: parsed.organizationId, | ||
| outboxId: parsed.outboxId, | ||
| postId: parsed.postId, | ||
| action: outcome, | ||
| slackUserId: input.actorId, | ||
| slackUserName: input.actorName, | ||
| }); | ||
|
|
||
| if (!approval.recorded) { | ||
| yield* Effect.logInfo("Iris decision already recorded").pipe( | ||
| Effect.annotateLogs({ | ||
| organizationId: parsed.organizationId, | ||
| postId: parsed.postId, | ||
| existingAction: approval.existingAction ?? "none", | ||
| }) |
There was a problem hiding this comment.
MEDIUM: Non-atomic read-modify-write of outbox approvals can flip a ship/skip decision (lost update)
recordIrisApproval (packages/ai/src/autonomy/ship.ts) performs a non-atomic read-modify-write on the approvals array stored in the autonomyOutbox.payload JSONB column. It SELECTs the current payload, parses the approvals array, checks for an existing entry for the postId, and if none exists appends a new approval and UPDATEs the whole payload with { ...basePayload, approvals: appended }. There is no transaction, no SELECT ... FOR UPDATE, and no optimistic concurrency control (no version/etag check).
If two Slack users click buttons on the same outbox card concurrently (e.g., one clicks Skip and one clicks Ship), both interactions can read the approvals array before either write lands. Both then append and overwrite the full payload; the second writer wins and the first approval is silently lost. In a Skip-then-Ship (or Ship-then-Skip) race this does not merely lose audit data — it can flip the recorded decision outcome: the shouldPublish logic in iris-interaction.ts (outcome === 'shipped' && (approval.recorded || approval.existingAction === 'shipped')) will publish the post based on whichever approval survives the race, which may not be the decision that was actually intended/first. The post-publish itself is made idempotent at the DB layer (shipIrisPost updates WHERE status = 'draft'), but the decision-affecting race on the approvals array is not protected.
Evidence: packages/ai/src/autonomy/ship.ts recordIrisApproval — separate SELECT (lines ~24-44) then UPDATE (lines ~63-78) with no transaction; payload column is jsonb with no version column (packages/db/src/schema.ts line 1885).
Suggestion: Wrap the read-modify-write of the approvals array in a serializable transaction (or SELECT ... FOR UPDATE on the outbox row) so concurrent approvals serialize correctly. Alternatively, store approvals in a separate table with a unique constraint on (outboxId, postId) so duplicate approvals are rejected at the DB layer and the existing-entry check is atomic with the insert. Ensure the ship/skip decision is determined by the committed approvals state, not by an in-memory copy read before the write.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="apps/agent/agent/lib/utils/iris-interaction.ts:192-208" severity="MEDIUM">Non-atomic read-modify-write of outbox approvals can flip a ship/skip decision (lost update) — recordIrisApproval (packages/ai/src/autonomy/ship.ts) performs a non-atomic read-modify-write on the approvals array stored in the autonomyOutbox.payload JSONB column. It SELECTs the current payload, parses the approvals array, checks for an existing entry for the postId, and if none exists appends a new approval and UPDATEs the whole payload with `{ ...basePayload, approvals: appended }`. There is no transaction, no SELECT ... FOR UPDATE, and no optimistic concurrency control (no version/etag check). If two Slack users click buttons on the same outbox card concurrently (e.g., one clicks Skip and one clicks Ship), both interactions can read the approvals array before either write lands. Both then append and overwrite the full payload; the second writer wins and the first approval is silently lost. In a Skip-then-Ship (or Ship-then-Skip) race this does not merely lose audit data — it can flip the recorded decision outcome: the shouldPublish logic in iris-interaction.ts (outcome === 'shipped' && (approval.recorded || approval.existingAction === 'shipped')) will publish the post based on whichever approval survives the race, which may not be the decision that was actually intended/first. The post-publish itself is made idempotent at the DB layer (shipIrisPost updates WHERE status = 'draft'), but the decision-affecting race on the approvals array is not protected. Evidence: packages/ai/src/autonomy/ship.ts recordIrisApproval — separate SELECT (lines ~24-44) then UPDATE (lines ~63-78) with no transaction; payload column is jsonb with no version column (packages/db/src/schema.ts line 1885). Fix: Wrap the read-modify-write of the approvals array in a serializable transaction (or SELECT ... FOR UPDATE on the outbox row) so concurrent approvals serialize correctly. Alternatively, store approvals in a separate table with a unique constraint on (outboxId, postId) so duplicate approvals are rejected at the DB layer and the existing-entry check is atomic with the insert. Ensure the ship/skip decision is determined by the committed approvals state, not by an in-memory copy read before the write.</issue>
Commit 8a6cf96.
Comment on lines
+160
to
+427
| const created = await createIrisRun({ | ||
| organizationId, | ||
| mandateId: mandate.id, | ||
| mandateVersion: mandate.version, | ||
| trigger, | ||
| }); | ||
| const runId = created.runId; | ||
|
|
||
| try { | ||
| const coalesced = await coalesceIrisSignals({ | ||
| organizationId, | ||
| signalIds: gathered.pendingSignalIds, | ||
| }); | ||
|
|
||
| const plan = await planIrisRun({ | ||
| runId, | ||
| mandate, | ||
| signalSummaries: coalesced.summaries, | ||
| recentActionSummaries: gathered.recentActionSummaries, | ||
| }); | ||
|
|
||
| const output = plan.output; | ||
|
|
||
| if (plan.status === "rejected" || output === null) { | ||
| await finalizeIrisRun({ | ||
| organizationId, | ||
| runId, | ||
| status: "failed", | ||
| costCents: plan.costCents, | ||
| goalId: null, | ||
| goalStatus: null, | ||
| }); | ||
| await markIrisSignalsProcessed({ | ||
| organizationId, | ||
| signalIds: coalesced.signalIds, | ||
| }); | ||
| await releaseIrisLease({ organizationId, ownerToken: leaseToken }); | ||
| return { | ||
| status: "plan_rejected", | ||
| runId, | ||
| reason: plan.violations.join("; ") || "The plan was rejected", | ||
| }; | ||
| } | ||
|
|
||
| if (output.decision === "no_op") { | ||
| await finalizeIrisRun({ | ||
| organizationId, | ||
| runId, | ||
| status: "completed", | ||
| costCents: plan.costCents, | ||
| goalId: null, | ||
| goalStatus: null, | ||
| }); | ||
| await markIrisSignalsProcessed({ | ||
| organizationId, | ||
| signalIds: coalesced.signalIds, | ||
| }); | ||
| await releaseIrisLease({ organizationId, ownerToken: leaseToken }); | ||
| return { status: "no_op", runId, reason: output.reason }; | ||
| } | ||
|
|
||
| if (output.decision === "escalate") { | ||
| await finalizeIrisRun({ | ||
| organizationId, | ||
| runId, | ||
| status: "completed", | ||
| costCents: plan.costCents, | ||
| goalId: null, | ||
| goalStatus: null, | ||
| }); | ||
| await publishIrisOutbox({ | ||
| organizationId, | ||
| runId, | ||
| payload: { | ||
| kind: "no_op", | ||
| runId, | ||
| headline: `Iris needs a decision from you: ${output.reason}`, | ||
| signalCount: coalesced.signalIds.length, | ||
| artifacts: [], | ||
| trigger, | ||
| organizationSlug: context.organizationSlug, | ||
| }, | ||
| }); | ||
| await markIrisSignalsProcessed({ | ||
| organizationId, | ||
| signalIds: coalesced.signalIds, | ||
| }); | ||
| await releaseIrisLease({ organizationId, ownerToken: leaseToken }); | ||
| return { status: "escalated", runId, reason: output.reason }; | ||
| } | ||
|
|
||
| const goal = output.goal; | ||
| if (!goal) { | ||
| await finalizeIrisRun({ | ||
| organizationId, | ||
| runId, | ||
| status: "failed", | ||
| costCents: plan.costCents, | ||
| goalId: null, | ||
| goalStatus: null, | ||
| }); | ||
| await markIrisSignalsProcessed({ | ||
| organizationId, | ||
| signalIds: coalesced.signalIds, | ||
| }); | ||
| await releaseIrisLease({ organizationId, ownerToken: leaseToken }); | ||
| return { | ||
| status: "plan_rejected", | ||
| runId, | ||
| reason: "The plan did not include a goal", | ||
| }; | ||
| } | ||
|
|
||
| const persisted = await persistIrisPlan({ | ||
| organizationId, | ||
| mandateId: mandate.id, | ||
| runId, | ||
| goal, | ||
| tasks: output.tasks, | ||
| originSignalIds: coalesced.signalIds, | ||
| }); | ||
|
|
||
| const collection = await ensureIrisRunCollection({ | ||
| organizationId, | ||
| runId, | ||
| }); | ||
|
|
||
| const canceledTaskIds = new Set<string>(); | ||
| const artifacts: IrisOutboxArtifact[] = []; | ||
| let succeededCount = 0; | ||
| let costCents = plan.costCents; | ||
|
|
||
| let mandateRevoked = false; | ||
| let leaseLost = false; | ||
|
|
||
| for (const task of persisted.tasks) { | ||
| if (canceledTaskIds.has(task.taskId)) { | ||
| continue; | ||
| } | ||
|
|
||
| const leaseHeld = await renewIrisLease({ | ||
| organizationId, | ||
| ownerToken: leaseToken, | ||
| }); | ||
| if (!leaseHeld) { | ||
| leaseLost = true; | ||
| await cancelIrisTasks({ | ||
| taskIds: collectRemainingTaskIds( | ||
| persisted.tasks, | ||
| canceledTaskIds, | ||
| task.taskId | ||
| ), | ||
| reason: "The controller lease was lost", | ||
| }); | ||
| break; | ||
| } | ||
|
|
||
| const stillActive = await isIrisMandateActive(organizationId); | ||
| if (!stillActive) { | ||
| mandateRevoked = true; | ||
| await cancelIrisTasks({ | ||
| taskIds: collectRemainingTaskIds( | ||
| persisted.tasks, | ||
| canceledTaskIds, | ||
| task.taskId | ||
| ), | ||
| reason: "Iris was paused", | ||
| }); | ||
| break; | ||
| } | ||
|
|
||
| const outcome = await runIrisTask({ | ||
| organizationId, | ||
| runId, | ||
| mandate, | ||
| collectionId: collection.collectionId, | ||
| task, | ||
| signalContext: { | ||
| primarySignal: coalesced.primarySignal, | ||
| summaries: coalesced.summaries, | ||
| }, | ||
| }); | ||
|
|
||
| costCents += outcome.costCents; | ||
|
|
||
| if (outcome.status === "succeeded") { | ||
| succeededCount += 1; | ||
| artifacts.push(...outcome.artifacts); | ||
| continue; | ||
| } | ||
|
|
||
| const blocked = collectDependentTaskIds( | ||
| persisted.tasks, | ||
| task.taskId | ||
| ).filter((taskId) => !canceledTaskIds.has(taskId)); | ||
|
|
||
| for (const taskId of blocked) { | ||
| canceledTaskIds.add(taskId); | ||
| } | ||
|
|
||
| await cancelIrisTasks({ | ||
| taskIds: blocked, | ||
| reason: `Blocked by failed task ${task.localId}`, | ||
| }); | ||
| } | ||
|
|
||
| if (leaseLost) { | ||
| await finalizeIrisRun({ | ||
| organizationId, | ||
| runId, | ||
| status: "failed", | ||
| costCents, | ||
| goalId: persisted.goalId, | ||
| goalStatus: "abandoned", | ||
| }); | ||
| console.warn( | ||
| `[${LOG_PREFIX}] Lost the controller lease for org ${organizationId}, stopping run ${runId}` | ||
| ); | ||
| return { status: "failed", runId, reason: LEASE_LOST_REASON }; | ||
| } | ||
|
|
||
| const runStatus = succeededCount > 0 ? "completed" : "failed"; | ||
|
|
||
| await finalizeIrisRun({ | ||
| organizationId, | ||
| runId, | ||
| status: runStatus, | ||
| costCents, | ||
| goalId: persisted.goalId, | ||
| goalStatus: succeededCount > 0 ? "completed" : "abandoned", | ||
| }); | ||
|
|
||
| if (mandateRevoked) { | ||
| await markIrisSignalsProcessed({ | ||
| organizationId, | ||
| signalIds: coalesced.signalIds, | ||
| }); | ||
| await releaseIrisLease({ organizationId, ownerToken: leaseToken }); | ||
| return { status: runStatus, runId, reason: "Iris was paused" }; | ||
| } | ||
|
|
||
| await publishIrisOutbox({ | ||
| organizationId, | ||
| runId, | ||
| payload: { | ||
| kind: "run_summary", | ||
| runId, | ||
| headline: buildIrisHeadline( | ||
| artifacts.length, | ||
| coalesced.signalIds.length | ||
| ), | ||
| signalCount: coalesced.signalIds.length, | ||
| artifacts, | ||
| trigger, | ||
| organizationSlug: context.organizationSlug, | ||
| }, | ||
| }); | ||
|
|
||
| await markIrisSignalsProcessed({ | ||
| organizationId, | ||
| signalIds: coalesced.signalIds, | ||
| }); | ||
|
|
||
| await releaseIrisLease({ organizationId, ownerToken: leaseToken }); | ||
| await reapIrisExpiredClaims(); | ||
|
|
||
| return { status: runStatus, runId, reason: null }; | ||
| } catch (error) { |
There was a problem hiding this comment.
MEDIUM: Controller lease not released if createIrisRun throws after the gate passes
In irisControllerRun, the lease is acquired before the gate check. When the gate proceeds, createIrisRun is called (line ~160) to create the actual run record, and only afterward does a try block begin (line ~168) whose catch releases the lease and closes the open run. If createIrisRun throws (DB error, transient failure), execution escapes the function without entering the try, so releaseIrisLease is never called and markIrisSignalsProcessed is never called. The lease will eventually expire via its TTL and the signals remain pending (which is arguably correct for a retry), but the lease is held longer than necessary and there is no closeOpenIrisRun cleanup for the not-yet-created run. Contrast with the gate-blocked and plan-rejected paths, which explicitly release the lease. The no_active_mandate path also releases explicitly.
This is a minor resource-leak / cleanup-gap, not a security issue: the lease TTL bounds the leak and a duplicate execution claim prevents concurrent controllers. But it is an inconsistency in cleanup handling.
Suggestion: Move createIrisRun inside the try block, or wrap the entire post-gate body (including createIrisRun) in the try/catch so that any throw after the lease is acquired releases the lease. Alternatively, use a finally block to release the lease on every exit path (excluding the intentional leaseLost path where another controller now owns the lease).
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="apps/dashboard/src/workflows/iris-controller.ts:160-427" severity="MEDIUM">Controller lease not released if createIrisRun throws after the gate passes — In irisControllerRun, the lease is acquired before the gate check. When the gate proceeds, createIrisRun is called (line ~160) to create the actual run record, and only afterward does a try block begin (line ~168) whose catch releases the lease and closes the open run. If createIrisRun throws (DB error, transient failure), execution escapes the function without entering the try, so releaseIrisLease is never called and markIrisSignalsProcessed is never called. The lease will eventually expire via its TTL and the signals remain pending (which is arguably correct for a retry), but the lease is held longer than necessary and there is no closeOpenIrisRun cleanup for the not-yet-created run. Contrast with the gate-blocked and plan-rejected paths, which explicitly release the lease. The no_active_mandate path also releases explicitly. This is a minor resource-leak / cleanup-gap, not a security issue: the lease TTL bounds the leak and a duplicate execution claim prevents concurrent controllers. But it is an inconsistency in cleanup handling. Fix: Move createIrisRun inside the try block, or wrap the entire post-gate body (including createIrisRun) in the try/catch so that any throw after the lease is acquired releases the lease. Alternatively, use a finally block to release the lease on every exit path (excluding the intentional leaseLost path where another controller now owns the lease).</issue>
Commit 8a6cf96.
Contributor
There was a problem hiding this comment.
40 issues found across 104 files
Confidence score: 2/5
- In
packages/ai/src/autonomy/claims.tsandpackages/ai/src/autonomy/leases.ts, duplicate deliveries for the sameexecutionIdcan both acquire/run because deterministic owner-token reacquisition is treated as valid, which can create duplicate marketing jobs and reentrant execution side effects — require unique per-attempt ownership (or otherwise reject same-token reacquire while lease is active). - Pause/cancel protections have race windows in
packages/ai/src/autonomy/deliver.tsandapps/dashboard/src/lib/orpc/routers/iris.ts, so Iris can still publish a Slack card after a successful pause/kill-switch response, undermining operator control — atomically claim onlypendingrows and recheck mandate activity immediately before publish/summarize. - Signal reliability looks fragile in
packages/ai/src/autonomy/signals.tsandapps/dashboard/src/lib/webhooks/github.ts: non-primary signals can be dropped after coalescing failures, and webhook dispatch errors are acknowledged as success, causing silent event loss and missed planning triggers — move state transitions to post-success and propagate/retry dispatch failures. - User-action safety needs tightening in
apps/agent/agent/lib/utils/iris-interaction.tsandpackages/ai/src/autonomy/ship.ts, where any interactive Slack user may trigger publishing and concurrent decisions can lose a competing approval/skip, leading to unauthorized or incorrect publish outcomes — add per-user authorization checks and make decision writes first-writer-wins atomically.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/ai/src/autonomy/claims.ts">
<violation number="1" location="packages/ai/src/autonomy/claims.ts:51">
P1: Duplicate Iris deliveries for one `executionId` pass this claim and can create/run separate marketing jobs. The controller’s owner token is deterministic, so same-token reacquisition must not be treated as an acquired claim; only expired claims should be replaceable.</violation>
</file>
<file name="packages/ai/src/autonomy/signals.ts">
<violation number="1" location="packages/ai/src/autonomy/signals.ts:188">
P1: Any failure after coalescing permanently drops non-primary signals from future runs. This transition happens before planning/publishing, but retries load only `pending` rows; defer it until successful processing or restore these rows to `pending` in every failure path.</violation>
</file>
<file name="packages/ai/src/autonomy/deliver.ts">
<violation number="1" location="packages/ai/src/autonomy/deliver.ts:215">
P1: Pausing Iris can still publish a card if cancellation lands after `loadDeliverableRows` and before this unconditional state transition. Claim the row atomically with a `pending` status predicate and skip delivery when no row was claimed, so a canceled outbox item cannot be resurrected.</violation>
</file>
<file name="packages/ai/src/autonomy/leases.ts">
<violation number="1" location="packages/ai/src/autonomy/leases.ts:48">
P1: Duplicate deliveries for one `executionId` can both run Iris: this reentrant owner check accepts the same deterministic lease token while the first lease is still active. Use a per-workflow-attempt owner token, or otherwise reject a second active acquisition, so the lease remains exclusive.</violation>
</file>
<file name="apps/dashboard/src/lib/orpc/routers/iris.ts">
<violation number="1" location="apps/dashboard/src/lib/orpc/routers/iris.ts:272">
P1: Pausing during the final in-flight task can still post a new Slack card after the kill switch returns success. Recheck mandate activity immediately before publishing, and suppress/cancel the summary when it was paused.</violation>
<violation number="2" location="apps/dashboard/src/lib/orpc/routers/iris.ts:279">
P2: A transient QStash deletion failure leaves the wake schedule running indefinitely while Iris is paused, despite the successful pause response. Surface the failure for retry or enqueue a durable deletion retry instead of only logging it.</violation>
</file>
<file name="apps/agent/agent/lib/utils/iris-interaction.ts">
<violation number="1" location="apps/agent/agent/lib/utils/iris-interaction.ts:178">
P1: Restricted workspaces can ship or skip Iris posts from a channel outside the configured allowlist. Check `isChannelAllowed(installation, input.channelId)` together with the organization check before recording the decision.</violation>
<violation number="2" location="apps/agent/agent/lib/utils/iris-interaction.ts:193">
P1: A Slack user who can interact with this card can currently trigger draft publishing without a per-user authorization check. This path validates workspace/org alignment, then records approval and may call `shipIrisPost`, but it never verifies that `action.user.id` is allowed to publish for the organization. Consider enforcing the same organization access check used by dashboard actions (or a Slack approver allowlist) before recording the decision or shipping the post.</violation>
<violation number="3" location="apps/agent/agent/lib/utils/iris-interaction.ts:193">
P2: Simultaneous decisions on different card artifacts can erase one approval and re-enable its action in the refreshed card. Persist approvals atomically (or use optimistic concurrency/retry) instead of read-modify-writing the entire JSON payload.</violation>
<violation number="4" location="apps/agent/agent/lib/utils/iris-interaction.ts:216">
P1: Pausing Iris still permits Ship It on cards delivered before the pause, so the kill switch does not stop this remaining publish side effect. Reject Ship It unless the organization's Iris mandate is still active.</violation>
</file>
<file name="apps/dashboard/src/lib/webhooks/github.ts">
<violation number="1" location="apps/dashboard/src/lib/webhooks/github.ts:486">
P1: A failed Iris signal write is acknowledged as a successful webhook: `dispatchIrisGithubSignal` swallows the error, then this handler marks the GitHub delivery processed and returns 200. Propagate dispatch failures (and log this webhook as failed) so GitHub can retry instead of permanently dropping the signal.</violation>
</file>
<file name="packages/ai/src/autonomy/ship.ts">
<violation number="1" location="packages/ai/src/autonomy/ship.ts:56">
P2: An invalid outbox payload is treated as having no decisions, so a Ship click can still publish from a corrupted or incompatible card record. Fail this operation on parse failure rather than falling back to an empty approval list.</violation>
<violation number="2" location="packages/ai/src/autonomy/ship.ts:95">
P1: Concurrent Slack decisions can lose an approval and let Ship It publish despite a competing Skip. Make the check-and-append first-writer-wins atomically, e.g. lock the outbox row in a transaction or use a conditional update.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/wake-schedule.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/wake-schedule.ts:46">
P2: A DB write failure after QStash accepts this request, or two simultaneous starts, leaves one or more live schedules without a tracked ID. Those orphan schedules continue waking Iris after pause and can cause duplicate deliveries; make creation idempotent and compensate by deleting the created schedule if persistence fails.</violation>
<violation number="2" location="apps/dashboard/src/lib/iris/wake-schedule.ts:47">
P1: Wake requests are rejected as Unauthorized when `WORKFLOW_BASE_URL` differs from the app URL, because QStash signs the destination URL but the route verifies a different URL. Use the same workflow base URL for the schedule destination and signature verification.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-granola.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-granola.ts:32">
P1: Organizations with enabled Granola workspaces will always see this source skipped and never get meeting-note signals, so those notes cannot trigger planning. Implement note retrieval/mapping for the poll window (or omit this source until it is supported) rather than returning an empty successful result.</violation>
</file>
<file name="packages/ai/src/autonomy/capabilities.ts">
<violation number="1" location="packages/ai/src/autonomy/capabilities.ts:518">
P1: Images are embedded when the vision review fails, so malformed review responses or review outages bypass the rejection gate. Treat review failure as a rejection (or fail the task) to preserve the advertised quality-check boundary.</violation>
<violation number="2" location="packages/ai/src/autonomy/capabilities.ts:649">
P1: Each accepted blog image is charged twice: once as `marketing_assets` here and again in the Iris run total. Omit the per-image Autumn tracking here, since the run-level charge already includes this usage.</violation>
<violation number="3" location="packages/ai/src/autonomy/capabilities.ts:747">
P1: A save failure after generation makes the run report and bill zero task cost even though the model call has already consumed tokens. Preserve accrued cost through failed task outcomes so failed persistence does not create unbilled generation.</violation>
</file>
<file name="packages/ai/src/autonomy/planner.ts">
<violation number="1" location="packages/ai/src/autonomy/planner.ts:66">
P1: Rejected planner drafts consume model tokens but are recorded and billed as zero. Preserve accumulated `costCents` through the failure path so invalid-output/repair-exhaustion runs update the run cost and Autumn usage.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/mandate.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/mandate.ts:18">
P1: Pausing Iris can still leave its controller running when another mandate is active for the organization. Scope this lookup to `IRIS_MANDATE_NAME`, matching the Iris router's mandate lookup, so signals and wake runs cannot select an arbitrary active mandate.</violation>
</file>
<file name="packages/ai/src/autonomy/validate-plan.ts">
<violation number="1" location="packages/ai/src/autonomy/validate-plan.ts:34">
P1: A plan declaring an unsupported capability version is accepted and then executed as the current implementation, breaking the versioned task contract on catalog upgrades or model drift. Validate each task's `(capabilityName, capabilityVersion)` against the supplied capability catalog before persisting the plan.</violation>
</file>
<file name="packages/ai/src/autonomy/gate.ts">
<violation number="1" location="packages/ai/src/autonomy/gate.ts:127">
P1: Recently incurred cost from long-running runs is omitted because the rolling window uses `startedAt`. Filter finalized costs by `completedAt` so a run completed within the last 24 hours counts toward the daily ceiling.</violation>
</file>
<file name="apps/dashboard/src/workflows/iris-controller.ts">
<violation number="1" location="apps/dashboard/src/workflows/iris-controller.ts:86">
P2: A database or gate failure before `createIrisRun` leaves the organization lease held for 30 minutes, so subsequent signal/manual controllers return `controller_busy`. Wrap the entire post-acquisition flow in cleanup that always releases the lease.</violation>
<violation number="2" location="apps/dashboard/src/workflows/iris-controller.ts:343">
P1: Runs near a daily action or spend limit can execute every task in the plan and exceed that limit, because the gate is never checked again after the first task. Recheck/reserve the remaining action and cost budget before each task (including planner cost) so the configured daily limits are ceilings.</violation>
</file>
<file name="apps/dashboard/src/workflows/steps/iris-steps.ts">
<violation number="1" location="apps/dashboard/src/workflows/steps/iris-steps.ts:440">
P1: Retrying after a crash or overlapping stale execution reruns an action already marked `executing`, causing another model call and possible external work under the same idempotency key. Treat an existing nonterminal action as in-progress/recoverable rather than executing it again without ownership recovery.</violation>
<violation number="2" location="apps/dashboard/src/workflows/steps/iris-steps.ts:469">
P1: Lease expiry during a long capability run does not fence stale work: this step still generates/persists content and commits task state after ownership has moved. Thread the acquired fencing token through capability and persistence writes, or revalidate ownership before every side effect and final state update.</violation>
</file>
<file name="packages/ai/src/utils/iris-usage.ts">
<violation number="1" location="packages/ai/src/utils/iris-usage.ts:30">
P2: Image-review spend is priced with the default model rates, so run totals and the daily cost gate understate Sonnet 4.6 usage. Add a `vercel/anthropic/claude-sonnet-4.6` pricing alias (or normalize provider prefixes) before this helper calculates review costs.</violation>
</file>
<file name="apps/dashboard/src/utils/iris-signal-summary.ts">
<violation number="1" location="apps/dashboard/src/utils/iris-signal-summary.ts:25">
P2: Push-triggered planning cannot distinguish a launch from a typo because this summary drops all commit content. Include a bounded set of commit messages (and similarly useful PR metadata) so Iris can make the advertised signal-based decision rather than seeing only `N commits`.</violation>
</file>
<file name="packages/ai/src/autonomy/poll.ts">
<violation number="1" location="packages/ai/src/autonomy/poll.ts:64">
P2: A transient failure from one provider drops signals fetched from every other provider, because `Effect.all` fails before recording begins. Recover each source into a skipped/error summary (and log it) so GitHub or Linear ingestion remains available when another integration is down.</violation>
</file>
<file name="packages/ai/src/utils/iris-poll-github.ts">
<violation number="1" location="packages/ai/src/utils/iris-poll-github.ts:146">
P2: A newly pushed older commit can be missed entirely because the commits `since` filter is based on commit time, not ref-update time. Track the default-branch head (or consume GitHub push events) and compare it with the prior observed head instead of using this window as the push detector.</violation>
</file>
<file name="apps/dashboard/scripts/autonomy-smoke.ts">
<violation number="1" location="apps/dashboard/scripts/autonomy-smoke.ts:105">
P2: Cyclic planner output can be accepted while `smoke:autonomy` still exits successfully because this only logs the result. Assert rejection so the DAG contract regression fails the smoke command.</violation>
<violation number="2" location="apps/dashboard/scripts/autonomy-smoke.ts:119">
P2: Mandate enforcement regressions do not fail this smoke command; the expected violations are only printed. Assert the fixture's expected violations before continuing.</violation>
<violation number="3" location="apps/dashboard/scripts/autonomy-smoke.ts:136">
P2: Claim/database failures currently look like a successful smoke run because every lifecycle result is only logged. Check each expected claim result and require zero leftovers so `smoke:autonomy` provides a reliable pass/fail signal.</violation>
</file>
<file name="packages/ai/src/integrations/slack-post.ts">
<violation number="1" location="packages/ai/src/integrations/slack-post.ts:210">
P2: An incomplete successful response is recorded as delivered with no Slack message identity, so later delivery/card lifecycle cannot address that message. Treat a post response missing `channel` or `ts` as an invalid retryable response instead of defaulting `ts` to an empty string.</violation>
</file>
<file name="apps/dashboard/src/schemas/workflows/iris.ts">
<violation number="1" location="apps/dashboard/src/schemas/workflows/iris.ts:16">
P2: Wake deliveries can be classified as `manual`, `signal`, or `repair` even though this endpoint always creates a wake execution. Restrict this payload to `wake` so a misconfigured/published QStash message cannot activate manual-only controller behavior under a wake id.</violation>
</file>
<file name="apps/dashboard/src/lib/iris/history.ts">
<violation number="1" location="apps/dashboard/src/lib/iris/history.ts:146">
P2: Concurrent Run now requests can both pass this check before either workflow creates its `planning` row. The controller lease later drops one workflow as `controller_busy`, but the API already returned a successful run ID; reserve the run/lease atomically before reporting success.</violation>
</file>
<file name="packages/ai/src/schemas/autonomy/capability-params.ts">
<violation number="1" location="packages/ai/src/schemas/autonomy/capability-params.ts:60">
P2: A rejected image review can omit its revision instruction, causing the only retry to reuse `basePrompt` instead of fixing the rejected issue. Encode the `accept`/`revisionPrompt` invariant in the structured-output schema so invalid review output is rejected rather than silently retried unchanged.</violation>
</file>
<file name="packages/ai/src/schemas/autonomy/signal.ts">
<violation number="1" location="packages/ai/src/schemas/autonomy/signal.ts:16">
P2: Signal validation never runs before persistence, so malformed envelopes bypass the new length, enum, timestamp, and required-field constraints. Parse with `signalEnvelopeSchema` at ingestion or replace the loose `RecordSignalInput` boundary with this contract.</violation>
</file>
<file name="apps/dashboard/src/app/(dashboard)/[slug]/iris/page-client.tsx">
<violation number="1" location="apps/dashboard/src/app/(dashboard)/[slug]/iris/page-client.tsx:37">
P2: An open pause confirmation can carry across organization navigation and pause the newly selected organization on confirmation. Scope or reset `pauseOpen` when the organization changes, and bind confirmation to the organization/mandate that opened it.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| expiresAt, | ||
| updatedAt: now, | ||
| }, | ||
| setWhere: or( |
Contributor
There was a problem hiding this comment.
P1: Duplicate Iris deliveries for one executionId pass this claim and can create/run separate marketing jobs. The controller’s owner token is deterministic, so same-token reacquisition must not be treated as an acquired claim; only expired claims should be replaceable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/claims.ts, line 51:
<comment>Duplicate Iris deliveries for one `executionId` pass this claim and can create/run separate marketing jobs. The controller’s owner token is deterministic, so same-token reacquisition must not be treated as an acquired claim; only expired claims should be replaceable.</comment>
<file context>
@@ -0,0 +1,177 @@
+ expiresAt,
+ updatedAt: now,
+ },
+ setWhere: or(
+ lt(autonomyClaims.expiresAt, now),
+ eq(autonomyClaims.ownerToken, input.ownerToken)
</file context>
| db | ||
| .update(autonomySignals) | ||
| .set({ | ||
| status: "coalesced", |
Contributor
There was a problem hiding this comment.
P1: Any failure after coalescing permanently drops non-primary signals from future runs. This transition happens before planning/publishing, but retries load only pending rows; defer it until successful processing or restore these rows to pending in every failure path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/signals.ts, line 188:
<comment>Any failure after coalescing permanently drops non-primary signals from future runs. This transition happens before planning/publishing, but retries load only `pending` rows; defer it until successful processing or restore these rows to `pending` in every failure path.</comment>
<file context>
@@ -0,0 +1,247 @@
+ db
+ .update(autonomySignals)
+ .set({
+ status: "coalesced",
+ coalescedIntoSignalId: primary.id,
+ updatedAt: now,
</file context>
| yield* updateOutboxRow({ | ||
| outboxId: row.id, | ||
| operation: "markAttempting", | ||
| values: { status: "attempting", attempts }, |
Contributor
There was a problem hiding this comment.
P1: Pausing Iris can still publish a card if cancellation lands after loadDeliverableRows and before this unconditional state transition. Claim the row atomically with a pending status predicate and skip delivery when no row was claimed, so a canceled outbox item cannot be resurrected.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/deliver.ts, line 215:
<comment>Pausing Iris can still publish a card if cancellation lands after `loadDeliverableRows` and before this unconditional state transition. Claim the row atomically with a `pending` status predicate and skip delivery when no row was claimed, so a canceled outbox item cannot be resurrected.</comment>
<file context>
@@ -0,0 +1,428 @@
+ yield* updateOutboxRow({
+ outboxId: row.id,
+ operation: "markAttempting",
+ values: { status: "attempting", attempts },
+ });
+
</file context>
| }, | ||
| setWhere: or( | ||
| lt(autonomyControllerLeases.expiresAt, now), | ||
| eq(autonomyControllerLeases.ownerToken, input.ownerToken) |
Contributor
There was a problem hiding this comment.
P1: Duplicate deliveries for one executionId can both run Iris: this reentrant owner check accepts the same deterministic lease token while the first lease is still active. Use a per-workflow-attempt owner token, or otherwise reject a second active acquisition, so the lease remains exclusive.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/autonomy/leases.ts, line 48:
<comment>Duplicate deliveries for one `executionId` can both run Iris: this reentrant owner check accepts the same deterministic lease token while the first lease is still active. Use a per-workflow-attempt owner token, or otherwise reject a second active acquisition, so the lease remains exclusive.</comment>
<file context>
@@ -0,0 +1,136 @@
+ },
+ setWhere: or(
+ lt(autonomyControllerLeases.expiresAt, now),
+ eq(autonomyControllerLeases.ownerToken, input.ownerToken)
+ ),
+ })
</file context>
|
|
||
| await db | ||
| .update(autonomyMandates) | ||
| .set({ status: "paused", pausedAt: new Date(), updatedAt: new Date() }) |
Contributor
There was a problem hiding this comment.
P1: Pausing during the final in-flight task can still post a new Slack card after the kill switch returns success. Recheck mandate activity immediately before publishing, and suppress/cancel the summary when it was paused.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/lib/orpc/routers/iris.ts, line 272:
<comment>Pausing during the final in-flight task can still post a new Slack card after the kill switch returns success. Recheck mandate activity immediately before publishing, and suppress/cancel the summary when it was paused.</comment>
<file context>
@@ -0,0 +1,412 @@
+
+ await db
+ .update(autonomyMandates)
+ .set({ status: "paused", pausedAt: new Date(), updatedAt: new Date() })
+ .where(eq(autonomyMandates.id, mandate.id));
+
</file context>
| ]); | ||
| export type SignalSource = z.infer<typeof signalSourceSchema>; | ||
|
|
||
| export const signalEnvelopeSchema = z.object({ |
Contributor
There was a problem hiding this comment.
P2: Signal validation never runs before persistence, so malformed envelopes bypass the new length, enum, timestamp, and required-field constraints. Parse with signalEnvelopeSchema at ingestion or replace the loose RecordSignalInput boundary with this contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/schemas/autonomy/signal.ts, line 16:
<comment>Signal validation never runs before persistence, so malformed envelopes bypass the new length, enum, timestamp, and required-field constraints. Parse with `signalEnvelopeSchema` at ingestion or replace the loose `RecordSignalInput` boundary with this contract.</comment>
<file context>
@@ -0,0 +1,26 @@
+]);
+export type SignalSource = z.infer<typeof signalSourceSchema>;
+
+export const signalEnvelopeSchema = z.object({
+ id: z.string().min(1),
+ organizationId: z.string().min(1),
</file context>
| const removed = await Effect.runPromise( | ||
| deleteIrisWakeSchedule(mandate) | ||
| ); | ||
| if (!removed) { |
Contributor
There was a problem hiding this comment.
P2: A transient QStash deletion failure leaves the wake schedule running indefinitely while Iris is paused, despite the successful pause response. Surface the failure for retry or enqueue a durable deletion retry instead of only logging it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/lib/orpc/routers/iris.ts, line 279:
<comment>A transient QStash deletion failure leaves the wake schedule running indefinitely while Iris is paused, despite the successful pause response. Surface the failure for retry or enqueue a durable deletion retry instead of only logging it.</comment>
<file context>
@@ -0,0 +1,412 @@
+ const removed = await Effect.runPromise(
+ deleteIrisWakeSchedule(mandate)
+ );
+ if (!removed) {
+ console.warn("[Iris] The wake schedule was kept for a later retry", {
+ mandateId: mandate.id,
</file context>
| const organizationId = organization?.id ?? ""; | ||
| const isReady = organizationId.length > 0; | ||
| const queryClient = useQueryClient(); | ||
| const [pauseOpen, setPauseOpen] = useState(false); |
Contributor
There was a problem hiding this comment.
P2: An open pause confirmation can carry across organization navigation and pause the newly selected organization on confirmation. Scope or reset pauseOpen when the organization changes, and bind confirmation to the organization/mandate that opened it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/app/(dashboard)/[slug]/iris/page-client.tsx, line 37:
<comment>An open pause confirmation can carry across organization navigation and pause the newly selected organization on confirmation. Scope or reset `pauseOpen` when the organization changes, and bind confirmation to the organization/mandate that opened it.</comment>
<file context>
@@ -0,0 +1,216 @@
+ const organizationId = organization?.id ?? "";
+ const isReady = organizationId.length > 0;
+ const queryClient = useQueryClient();
+ const [pauseOpen, setPauseOpen] = useState(false);
+
+ const irisKey = dashboardOrpc.iris.key();
</file context>
| }, | ||
| { ...mandate, status: "paused" } | ||
| ); | ||
| console.log(`mandate violations detected: ${violations.length}`); |
Contributor
There was a problem hiding this comment.
P2: Mandate enforcement regressions do not fail this smoke command; the expected violations are only printed. Assert the fixture's expected violations before continuing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/scripts/autonomy-smoke.ts, line 119:
<comment>Mandate enforcement regressions do not fail this smoke command; the expected violations are only printed. Assert the fixture's expected violations before continuing.</comment>
<file context>
@@ -0,0 +1,197 @@
+ },
+ { ...mandate, status: "paused" }
+ );
+ console.log(`mandate violations detected: ${violations.length}`);
+ for (const violation of violations) {
+ console.log(` - ${violation}`);
</file context>
| }, | ||
| ], | ||
| }); | ||
| console.log(`cyclic plan rejected: ${!cyclePlan.success}`); |
Contributor
There was a problem hiding this comment.
P2: Cyclic planner output can be accepted while smoke:autonomy still exits successfully because this only logs the result. Assert rejection so the DAG contract regression fails the smoke command.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/scripts/autonomy-smoke.ts, line 105:
<comment>Cyclic planner output can be accepted while `smoke:autonomy` still exits successfully because this only logs the result. Assert rejection so the DAG contract regression fails the smoke command.</comment>
<file context>
@@ -0,0 +1,197 @@
+ },
+ ],
+ });
+ console.log(`cyclic plan rejected: ${!cyclePlan.success}`);
+
+ const violations = validatePlannerOutputAgainstMandate(
</file context>
There was a problem hiding this comment.
No blocking issues found across the changed files.
1 other commit review still in progress for this PR — findings may follow.
Commit 2263757 · Posted by Comp AI Code Reviews.
There was a problem hiding this comment.
No blocking issues found across the changed files.
Commit b226cf9 · Posted by Comp AI Code Reviews.
There was a problem hiding this comment.
3 issues found across 1 file.
Prompt for AI agents (all issues)
Check whether each issue below is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it, and use sub-agents to handle independent issues in parallel.
<file name="packages/ai/src/utils/iris-usage.ts">
<issue n="1" at="packages/ai/src/utils/iris-usage.ts:10-45" severity="MEDIUM">Sandbox cost path double-counts cache tokens vs. the text path — toIrisTokenUsage() (the text path) deliberately de-duplicates cache tokens from the input count before billing: `inputTokens = Math.max(0, rawInputTokens - cacheReadTokens - cacheWriteTokens)` (lines 10-13). This establishes the contract that calculateTokenCostCents() expects: `inputTokens` is the UNCACHED input, with cacheRead/cacheWrite billed separately at their own rates. However, irisSandboxCostCents() (lines 35-46) passes the `usage` object from extractRepoImageUsage() straight through to calculateAiCreditCostCents() WITHOUT performing this de-duplication. extractRepoImageUsage() sets `inputTokens = normalizedCost.inputTokens ?? ...` (the raw, cache-INCLUSIVE input count for Anthropic models, whose `input_tokens` includes cached tokens). When the sandbox runtime does not report a positive `totalUsd`, calculateAiCreditCostCents() falls back to calculateTokenCostCents(), which computes `inputTokens*inputRate + cacheRead*cacheReadRate + cacheWrite*cacheWriteRate`. Because `inputTokens` still includes the cache tokens, those tokens are billed once at the full input rate AND again at the cache rate — a systematic overcharge. For the image-gen model (vercel/anthropic/claude-opus-5: input $5/M, cacheWrite $6.25/M), cache-write tokens would be charged at $11.25/M instead of $6.25/M (~80% over on cache writes). The inconsistency between the two cost functions in this same file is the root cause: irisSandboxCostCents should normalize its input the way toIrisTokenUsage does. Fix: In irisSandboxCostCents (or in extractRepoImageUsage), subtract cacheReadTokens and cacheWriteTokens from inputTokens (clamped at 0) before passing usage to calculateAiCreditCostCents, mirroring toIrisTokenUsage. Alternatively, make calculateTokenCostCents tolerate cache-inclusive inputTokens by computing the uncached portion internally, so both callers are consistent.</issue>
<issue n="2" at="packages/ai/src/utils/iris-usage.ts:42-45" severity="MEDIUM">Sandbox self-reported totalUsd is trusted for billing with no floor against token-based cost — irisSandboxCostCents() forwards the sandbox runtime's `usage.totalUsd` to calculateAiCreditCostCents(). In ai-credit-cost.ts, when `usage.totalUsd` is a positive finite number, the charged costCents is `Math.max(Math.ceil(totalUsd * 100), 1)` and the token-based estimate (`tokenCostCents`) is computed but DISCARDED — there is no `Math.max(reportedCostCents, tokenCostCents)` guard. The `totalUsd` value originates from the Upstash Box agent runtime's `stream.cost` (parse by extractRepoImageUsage/repoImageCostSchema, which only validates finite+nonnegative with no upper or lower sanity bound). If the sandbox runtime under-reports cost (e.g., missing/zero/near-zero totalUsd due to a runtime bug, or a malformed cost payload that still validates), the customer is charged the tiny reported amount instead of the platform's own token-based estimate — a billing undercharge / metering gap. Conversely, an unbounded huge totalUsd would overcharge with no cap. The cost report comes from inside a sandbox that processes attacker-influenced prompts (image-generation descriptions derived from planner markers/org signals), so the trust boundary is non-trivial even though the attacker cannot directly set the number. The platform should not fully outsource billing truth to a third-party sandbox runtime without a sanity floor/ceiling based on its own token accounting. Fix: Add a guard in the billing path: charge `Math.max(Math.min(reportedCostCents, someSanityCeiling), tokenCostCents)` (or at minimum `Math.max(reportedCostCents, tokenCostCents)` to prevent undercharge, and a ceiling to prevent runaway charges), and log/alert when reported and token-based costs diverge by a large factor. Validate totalUsd against a reasonable bound, not just finite+nonnegative.</issue>
<issue n="3" at="packages/ai/src/utils/iris-usage.ts:32-44" severity="MEDIUM">Iris autonomy cost is always computed with applyMarkup=false, inconsistent with the rest of the platform — Both irisTextCostCents() (line 32) and irisSandboxCostCents() (line 44) hardcode `applyMarkup = false` when calling calculateAiCreditCostCents(). Everywhere else in the codebase, the markup flag is derived dynamically from the organization's balance via shouldApplyMarkup(balance) — markup is applied for pay-as-you-go orgs that have exhausted plan credits (see apps/api/src/lib/chat/run.ts, apps/dashboard/src/lib/billing/workflow-ai-credits.ts, apps/dashboard/src/app/api/organizations/[organizationId]/chat/route.ts, etc.). The Iris autonomy system is the only path that never applies the 10% markup (MARKUP_PERCENT). As a result, pay-as-you-go organizations using the Iris autonomy feature are billed at pure cost while the same org using chat/content/agent features is billed cost+10%. This is a billing inconsistency / revenue-leakage logic bug (an org could route consumption through the autonomy feature to avoid the markup). It may be intentional for internal agent usage, but it diverges from the platform-wide pricing rule with no documented rationale in this file. Fix: If markup is intended to apply to autonomy usage, compute useMarkup from the organization's balance (shouldApplyMarkup) and thread it into irisTextCostCents/irisSandboxCostCents like the chat/content paths do. If markup is intentionally suppressed for autonomy, add a comment documenting the pricing decision and ensure the billing spec reflects it.</issue>
</file>
Commit 35bbe4a · Posted by Comp AI Code Reviews.
Comment on lines
+10
to
+45
| const outputTokens = usage?.outputTokens ?? 0; | ||
| const cacheReadTokens = usage?.inputTokenDetails?.cacheReadTokens ?? 0; | ||
| const cacheWriteTokens = usage?.inputTokenDetails?.cacheWriteTokens ?? 0; | ||
| const inputTokens = Math.max( | ||
| 0, | ||
| rawInputTokens - cacheReadTokens - cacheWriteTokens | ||
| ); | ||
|
|
||
| return { | ||
| inputTokens, | ||
| outputTokens, | ||
| totalTokens: usage?.totalTokens ?? rawInputTokens + outputTokens, | ||
| cacheReadTokens, | ||
| cacheWriteTokens, | ||
| modelId, | ||
| }; | ||
| }; | ||
|
|
||
| export const irisTextCostCents = ( | ||
| usage: LanguageModelUsage | undefined, | ||
| modelId: string | ||
| ): number => | ||
| calculateAiCreditCostCents(toIrisTokenUsage(usage, modelId), modelId, false) | ||
| .costCents; | ||
|
|
||
| export const irisSandboxCostCents = ( | ||
| usage: AgentTokenUsage | undefined, | ||
| fallbackModelId: string | ||
| ): number => { | ||
| if (!usage) { | ||
| return 0; | ||
| } | ||
| return calculateAiCreditCostCents( | ||
| usage, | ||
| usage.modelId ?? fallbackModelId, | ||
| false |
There was a problem hiding this comment.
MEDIUM: Sandbox cost path double-counts cache tokens vs. the text path
toIrisTokenUsage() (the text path) deliberately de-duplicates cache tokens from the input count before billing: inputTokens = Math.max(0, rawInputTokens - cacheReadTokens - cacheWriteTokens) (lines 10-13). This establishes the contract that calculateTokenCostCents() expects: inputTokens is the UNCACHED input, with cacheRead/cacheWrite billed separately at their own rates. However, irisSandboxCostCents() (lines 35-46) passes the usage object from extractRepoImageUsage() straight through to calculateAiCreditCostCents() WITHOUT performing this de-duplication. extractRepoImageUsage() sets inputTokens = normalizedCost.inputTokens ?? ... (the raw, cache-INCLUSIVE input count for Anthropic models, whose input_tokens includes cached tokens). When the sandbox runtime does not report a positive totalUsd, calculateAiCreditCostCents() falls back to calculateTokenCostCents(), which computes inputTokens*inputRate + cacheRead*cacheReadRate + cacheWrite*cacheWriteRate. Because inputTokens still includes the cache tokens, those tokens are billed once at the full input rate AND again at the cache rate — a systematic overcharge. For the image-gen model (vercel/anthropic/claude-opus-5: input $5/M, cacheWrite $6.25/M), cache-write tokens would be charged at $11.25/M instead of $6.25/M (~80% over on cache writes). The inconsistency between the two cost functions in this same file is the root cause: irisSandboxCostCents should normalize its input the way toIrisTokenUsage does.
Suggestion: In irisSandboxCostCents (or in extractRepoImageUsage), subtract cacheReadTokens and cacheWriteTokens from inputTokens (clamped at 0) before passing usage to calculateAiCreditCostCents, mirroring toIrisTokenUsage. Alternatively, make calculateTokenCostCents tolerate cache-inclusive inputTokens by computing the uncached portion internally, so both callers are consistent.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/ai/src/utils/iris-usage.ts:10-45" severity="MEDIUM">Sandbox cost path double-counts cache tokens vs. the text path — toIrisTokenUsage() (the text path) deliberately de-duplicates cache tokens from the input count before billing: `inputTokens = Math.max(0, rawInputTokens - cacheReadTokens - cacheWriteTokens)` (lines 10-13). This establishes the contract that calculateTokenCostCents() expects: `inputTokens` is the UNCACHED input, with cacheRead/cacheWrite billed separately at their own rates. However, irisSandboxCostCents() (lines 35-46) passes the `usage` object from extractRepoImageUsage() straight through to calculateAiCreditCostCents() WITHOUT performing this de-duplication. extractRepoImageUsage() sets `inputTokens = normalizedCost.inputTokens ?? ...` (the raw, cache-INCLUSIVE input count for Anthropic models, whose `input_tokens` includes cached tokens). When the sandbox runtime does not report a positive `totalUsd`, calculateAiCreditCostCents() falls back to calculateTokenCostCents(), which computes `inputTokens*inputRate + cacheRead*cacheReadRate + cacheWrite*cacheWriteRate`. Because `inputTokens` still includes the cache tokens, those tokens are billed once at the full input rate AND again at the cache rate — a systematic overcharge. For the image-gen model (vercel/anthropic/claude-opus-5: input $5/M, cacheWrite $6.25/M), cache-write tokens would be charged at $11.25/M instead of $6.25/M (~80% over on cache writes). The inconsistency between the two cost functions in this same file is the root cause: irisSandboxCostCents should normalize its input the way toIrisTokenUsage does. Fix: In irisSandboxCostCents (or in extractRepoImageUsage), subtract cacheReadTokens and cacheWriteTokens from inputTokens (clamped at 0) before passing usage to calculateAiCreditCostCents, mirroring toIrisTokenUsage. Alternatively, make calculateTokenCostCents tolerate cache-inclusive inputTokens by computing the uncached portion internally, so both callers are consistent.</issue>
Commit 35bbe4a.
Comment on lines
+42
to
+45
| return calculateAiCreditCostCents( | ||
| usage, | ||
| usage.modelId ?? fallbackModelId, | ||
| false |
There was a problem hiding this comment.
MEDIUM: Sandbox self-reported totalUsd is trusted for billing with no floor against token-based cost
irisSandboxCostCents() forwards the sandbox runtime's usage.totalUsd to calculateAiCreditCostCents(). In ai-credit-cost.ts, when usage.totalUsd is a positive finite number, the charged costCents is Math.max(Math.ceil(totalUsd * 100), 1) and the token-based estimate (tokenCostCents) is computed but DISCARDED — there is no Math.max(reportedCostCents, tokenCostCents) guard. The totalUsd value originates from the Upstash Box agent runtime's stream.cost (parse by extractRepoImageUsage/repoImageCostSchema, which only validates finite+nonnegative with no upper or lower sanity bound). If the sandbox runtime under-reports cost (e.g., missing/zero/near-zero totalUsd due to a runtime bug, or a malformed cost payload that still validates), the customer is charged the tiny reported amount instead of the platform's own token-based estimate — a billing undercharge / metering gap. Conversely, an unbounded huge totalUsd would overcharge with no cap. The cost report comes from inside a sandbox that processes attacker-influenced prompts (image-generation descriptions derived from planner markers/org signals), so the trust boundary is non-trivial even though the attacker cannot directly set the number. The platform should not fully outsource billing truth to a third-party sandbox runtime without a sanity floor/ceiling based on its own token accounting.
Suggestion: Add a guard in the billing path: charge Math.max(Math.min(reportedCostCents, someSanityCeiling), tokenCostCents) (or at minimum Math.max(reportedCostCents, tokenCostCents) to prevent undercharge, and a ceiling to prevent runaway charges), and log/alert when reported and token-based costs diverge by a large factor. Validate totalUsd against a reasonable bound, not just finite+nonnegative.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/ai/src/utils/iris-usage.ts:42-45" severity="MEDIUM">Sandbox self-reported totalUsd is trusted for billing with no floor against token-based cost — irisSandboxCostCents() forwards the sandbox runtime's `usage.totalUsd` to calculateAiCreditCostCents(). In ai-credit-cost.ts, when `usage.totalUsd` is a positive finite number, the charged costCents is `Math.max(Math.ceil(totalUsd * 100), 1)` and the token-based estimate (`tokenCostCents`) is computed but DISCARDED — there is no `Math.max(reportedCostCents, tokenCostCents)` guard. The `totalUsd` value originates from the Upstash Box agent runtime's `stream.cost` (parse by extractRepoImageUsage/repoImageCostSchema, which only validates finite+nonnegative with no upper or lower sanity bound). If the sandbox runtime under-reports cost (e.g., missing/zero/near-zero totalUsd due to a runtime bug, or a malformed cost payload that still validates), the customer is charged the tiny reported amount instead of the platform's own token-based estimate — a billing undercharge / metering gap. Conversely, an unbounded huge totalUsd would overcharge with no cap. The cost report comes from inside a sandbox that processes attacker-influenced prompts (image-generation descriptions derived from planner markers/org signals), so the trust boundary is non-trivial even though the attacker cannot directly set the number. The platform should not fully outsource billing truth to a third-party sandbox runtime without a sanity floor/ceiling based on its own token accounting. Fix: Add a guard in the billing path: charge `Math.max(Math.min(reportedCostCents, someSanityCeiling), tokenCostCents)` (or at minimum `Math.max(reportedCostCents, tokenCostCents)` to prevent undercharge, and a ceiling to prevent runaway charges), and log/alert when reported and token-based costs diverge by a large factor. Validate totalUsd against a reasonable bound, not just finite+nonnegative.</issue>
Commit 35bbe4a.
Comment on lines
+32
to
+44
| calculateAiCreditCostCents(toIrisTokenUsage(usage, modelId), modelId, false) | ||
| .costCents; | ||
|
|
||
| export const irisSandboxCostCents = ( | ||
| usage: AgentTokenUsage | undefined, | ||
| fallbackModelId: string | ||
| ): number => { | ||
| if (!usage) { | ||
| return 0; | ||
| } | ||
| return calculateAiCreditCostCents( | ||
| usage, | ||
| usage.modelId ?? fallbackModelId, |
There was a problem hiding this comment.
MEDIUM: Iris autonomy cost is always computed with applyMarkup=false, inconsistent with the rest of the platform
Both irisTextCostCents() (line 32) and irisSandboxCostCents() (line 44) hardcode applyMarkup = false when calling calculateAiCreditCostCents(). Everywhere else in the codebase, the markup flag is derived dynamically from the organization's balance via shouldApplyMarkup(balance) — markup is applied for pay-as-you-go orgs that have exhausted plan credits (see apps/api/src/lib/chat/run.ts, apps/dashboard/src/lib/billing/workflow-ai-credits.ts, apps/dashboard/src/app/api/organizations/[organizationId]/chat/route.ts, etc.). The Iris autonomy system is the only path that never applies the 10% markup (MARKUP_PERCENT). As a result, pay-as-you-go organizations using the Iris autonomy feature are billed at pure cost while the same org using chat/content/agent features is billed cost+10%. This is a billing inconsistency / revenue-leakage logic bug (an org could route consumption through the autonomy feature to avoid the markup). It may be intentional for internal agent usage, but it diverges from the platform-wide pricing rule with no documented rationale in this file.
Suggestion: If markup is intended to apply to autonomy usage, compute useMarkup from the organization's balance (shouldApplyMarkup) and thread it into irisTextCostCents/irisSandboxCostCents like the chat/content paths do. If markup is intentionally suppressed for autonomy, add a comment documenting the pricing decision and ensure the billing spec reflects it.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/ai/src/utils/iris-usage.ts:32-44" severity="MEDIUM">Iris autonomy cost is always computed with applyMarkup=false, inconsistent with the rest of the platform — Both irisTextCostCents() (line 32) and irisSandboxCostCents() (line 44) hardcode `applyMarkup = false` when calling calculateAiCreditCostCents(). Everywhere else in the codebase, the markup flag is derived dynamically from the organization's balance via shouldApplyMarkup(balance) — markup is applied for pay-as-you-go orgs that have exhausted plan credits (see apps/api/src/lib/chat/run.ts, apps/dashboard/src/lib/billing/workflow-ai-credits.ts, apps/dashboard/src/app/api/organizations/[organizationId]/chat/route.ts, etc.). The Iris autonomy system is the only path that never applies the 10% markup (MARKUP_PERCENT). As a result, pay-as-you-go organizations using the Iris autonomy feature are billed at pure cost while the same org using chat/content/agent features is billed cost+10%. This is a billing inconsistency / revenue-leakage logic bug (an org could route consumption through the autonomy feature to avoid the markup). It may be intentional for internal agent usage, but it diverges from the platform-wide pricing rule with no documented rationale in this file. Fix: If markup is intended to apply to autonomy usage, compute useMarkup from the organization's balance (shouldApplyMarkup) and thread it into irisTextCostCents/irisSandboxCostCents like the chat/content paths do. If markup is intentionally suppressed for autonomy, add a comment documenting the pricing decision and ensure the billing spec reflects it.</issue>
Commit 35bbe4a.
Member
Author
There was a problem hiding this comment.
2 issues found across 1 file.
Prompt for AI agents (all issues)
Check whether each issue below is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it, and use sub-agents to handle independent issues in parallel.
<file name="packages/ai/src/utils/iris-billing.ts">
<issue n="1" at="packages/ai/src/utils/iris-billing.ts:12-28" severity="MEDIUM">Markup silently skipped when Autumn balance check fails or returns no breakdown — In `resolveIrisBilledCents`, the call to `client.check(...)` is wrapped in `Effect.catch(() => Effect.succeed(null))`, so ANY error from the Autumn API (transient outage, 5xx, network blip, malformed response) is swallowed and `balance` becomes `null`. `shouldApplyMarkup(null)` returns `false` (see token-pricing.ts: `if (!balance || balance.remaining <= 0) return false;` and the empty-breakdown branch also returns `false`). Consequently, on any Autumn `check` failure the org is billed only the raw `costCents` and the 10% markup (`MARKUP_PERCENT`) is never applied. This is a revenue-correctness/business-logic bug: the platform silently forfeits its margin during Autumn outages. It is not attacker-exploitable for free credits (the base cost is still tracked), so it is a BUG rather than a security issue. Note also that `shouldApplyMarkup` only returns `true` in the narrow case where a balance exists, has a breakdown, and NO breakdown entry has `remaining > 0` with a non-`one_off` interval — meaning markup is applied only for orgs with prepaid/top-up credits but no active plan credits, which may not match the intended markup policy. Fix: Distinguish a check *failure* from a legitimate `null`/zero balance. On `check` failure, either retry, fail closed (track the marked-up cost, or hold tracking), or explicitly log a billing anomaly rather than silently billing without markup. Revisit `shouldApplyMarkup` semantics with the billing team to confirm the markup is intended to apply only when plan credits are exhausted but prepaid credits remain.</issue>
<issue n="2" at="packages/ai/src/utils/iris-billing.ts:39-69" severity="MEDIUM">trackIrisRunUsage has no idempotency/dedupe guard, risking double billing on workflow step retries — `trackIrisRunUsage` sends a metered-usage `track` event to Autumn with `value: billedCents` and records `run_id` in `properties`, but performs no check for whether the run was already tracked. Autumn's `track` is append-only metered usage, so calling it twice for the same `runId` double-charges the organization's AI credits. The sole caller, `finalizeIrisRun` in apps/dashboard/src/workflows/steps/iris-steps.ts, is a Vercel Workflow step (`"use step"`) that can be retried by the orchestrator. Because `trackIrisRunUsage` swallows its own failures (returns on `tracked._tag === "Failure"`), a step that partially succeeds (e.g., `completeRun` commits, then a downstream operation throws and the step is retried) would re-invoke `trackIrisRunUsage` with the same `runId`/`costCents` and record a second usage event. The function lacks an internal dedupe keyed on `runId`, so the double-billing risk is proximately attributable to this helper. Confidence is medium because exploitation depends on workflow retry semantics and whether `completeRun`/run-status guards prevent re-finalization (both outside this file). Fix: Make usage tracking idempotent per run: either (a) check the run's existing tracked status before calling `client.track`, (b) use Autumn's idempotency key support (if available) keyed on `runId`, or (c) guard in `finalizeIrisRun` so `trackIrisRunUsage` only runs when the run transitions from a non-terminal to terminal status. At minimum, dedupe on `run_id` in properties before emitting a track event.</issue>
</file>
Commit d47ce71 · Posted by Comp AI Code Reviews.
Comment on lines
+12
to
+28
|
|
||
| const IRIS_USAGE_SOURCE = "iris"; | ||
| const MARKUP_MULTIPLIER = 1 + MARKUP_PERCENT / 100; | ||
|
|
||
| const resolveIrisBilledCents = Effect.fn("iris.billing.resolveMarkup")( | ||
| function* (organizationId: string, costCents: number) { | ||
| if (!autumn) { | ||
| return costCents; | ||
| } | ||
| const client = autumn; | ||
| const balance = yield* Effect.tryPromise({ | ||
| try: async () => { | ||
| const data = await client.check({ | ||
| customerId: organizationId, | ||
| featureId: FEATURES.AI_CREDITS, | ||
| }); | ||
| return data.balance ?? null; |
There was a problem hiding this comment.
MEDIUM: Markup silently skipped when Autumn balance check fails or returns no breakdown
In resolveIrisBilledCents, the call to client.check(...) is wrapped in Effect.catch(() => Effect.succeed(null)), so ANY error from the Autumn API (transient outage, 5xx, network blip, malformed response) is swallowed and balance becomes null. shouldApplyMarkup(null) returns false (see token-pricing.ts: if (!balance || balance.remaining <= 0) return false; and the empty-breakdown branch also returns false). Consequently, on any Autumn check failure the org is billed only the raw costCents and the 10% markup (MARKUP_PERCENT) is never applied. This is a revenue-correctness/business-logic bug: the platform silently forfeits its margin during Autumn outages. It is not attacker-exploitable for free credits (the base cost is still tracked), so it is a BUG rather than a security issue. Note also that shouldApplyMarkup only returns true in the narrow case where a balance exists, has a breakdown, and NO breakdown entry has remaining > 0 with a non-one_off interval — meaning markup is applied only for orgs with prepaid/top-up credits but no active plan credits, which may not match the intended markup policy.
Suggestion: Distinguish a check failure from a legitimate null/zero balance. On check failure, either retry, fail closed (track the marked-up cost, or hold tracking), or explicitly log a billing anomaly rather than silently billing without markup. Revisit shouldApplyMarkup semantics with the billing team to confirm the markup is intended to apply only when plan credits are exhausted but prepaid credits remain.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/ai/src/utils/iris-billing.ts:12-28" severity="MEDIUM">Markup silently skipped when Autumn balance check fails or returns no breakdown — In `resolveIrisBilledCents`, the call to `client.check(...)` is wrapped in `Effect.catch(() => Effect.succeed(null))`, so ANY error from the Autumn API (transient outage, 5xx, network blip, malformed response) is swallowed and `balance` becomes `null`. `shouldApplyMarkup(null)` returns `false` (see token-pricing.ts: `if (!balance || balance.remaining <= 0) return false;` and the empty-breakdown branch also returns `false`). Consequently, on any Autumn `check` failure the org is billed only the raw `costCents` and the 10% markup (`MARKUP_PERCENT`) is never applied. This is a revenue-correctness/business-logic bug: the platform silently forfeits its margin during Autumn outages. It is not attacker-exploitable for free credits (the base cost is still tracked), so it is a BUG rather than a security issue. Note also that `shouldApplyMarkup` only returns `true` in the narrow case where a balance exists, has a breakdown, and NO breakdown entry has `remaining > 0` with a non-`one_off` interval — meaning markup is applied only for orgs with prepaid/top-up credits but no active plan credits, which may not match the intended markup policy. Fix: Distinguish a check *failure* from a legitimate `null`/zero balance. On `check` failure, either retry, fail closed (track the marked-up cost, or hold tracking), or explicitly log a billing anomaly rather than silently billing without markup. Revisit `shouldApplyMarkup` semantics with the billing team to confirm the markup is intended to apply only when plan credits are exhausted but prepaid credits remain.</issue>
Commit d47ce71.
Comment on lines
+39
to
+69
| export const trackIrisRunUsage = Effect.fn("iris.billing.track")(function* ( | ||
| input: TrackIrisRunUsageInput | ||
| ) { | ||
| const client = autumn; | ||
| if (!client || allowUnmeteredAiInDevelopment || input.costCents <= 0) { | ||
| return; | ||
| } | ||
|
|
||
| const billedCents = yield* resolveIrisBilledCents( | ||
| input.organizationId, | ||
| input.costCents | ||
| ); | ||
|
|
||
| const tracked = yield* Effect.result( | ||
| Effect.tryPromise({ | ||
| try: () => | ||
| client.track({ | ||
| customerId: input.organizationId, | ||
| featureId: FEATURES.AI_CREDITS, | ||
| value: billedCents, | ||
| properties: { | ||
| source: IRIS_USAGE_SOURCE, | ||
| run_id: input.runId, | ||
| cost_cents: billedCents, | ||
| raw_cost_cents: input.costCents, | ||
| }, | ||
| }), | ||
| catch: (cause) => cause, | ||
| }) | ||
| ); | ||
|
|
There was a problem hiding this comment.
MEDIUM: trackIrisRunUsage has no idempotency/dedupe guard, risking double billing on workflow step retries
trackIrisRunUsage sends a metered-usage track event to Autumn with value: billedCents and records run_id in properties, but performs no check for whether the run was already tracked. Autumn's track is append-only metered usage, so calling it twice for the same runId double-charges the organization's AI credits. The sole caller, finalizeIrisRun in apps/dashboard/src/workflows/steps/iris-steps.ts, is a Vercel Workflow step ("use step") that can be retried by the orchestrator. Because trackIrisRunUsage swallows its own failures (returns on tracked._tag === "Failure"), a step that partially succeeds (e.g., completeRun commits, then a downstream operation throws and the step is retried) would re-invoke trackIrisRunUsage with the same runId/costCents and record a second usage event. The function lacks an internal dedupe keyed on runId, so the double-billing risk is proximately attributable to this helper. Confidence is medium because exploitation depends on workflow retry semantics and whether completeRun/run-status guards prevent re-finalization (both outside this file).
Suggestion: Make usage tracking idempotent per run: either (a) check the run's existing tracked status before calling client.track, (b) use Autumn's idempotency key support (if available) keyed on runId, or (c) guard in finalizeIrisRun so trackIrisRunUsage only runs when the run transitions from a non-terminal to terminal status. At minimum, dedupe on run_id in properties before emitting a track event.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/ai/src/utils/iris-billing.ts:39-69" severity="MEDIUM">trackIrisRunUsage has no idempotency/dedupe guard, risking double billing on workflow step retries — `trackIrisRunUsage` sends a metered-usage `track` event to Autumn with `value: billedCents` and records `run_id` in `properties`, but performs no check for whether the run was already tracked. Autumn's `track` is append-only metered usage, so calling it twice for the same `runId` double-charges the organization's AI credits. The sole caller, `finalizeIrisRun` in apps/dashboard/src/workflows/steps/iris-steps.ts, is a Vercel Workflow step (`"use step"`) that can be retried by the orchestrator. Because `trackIrisRunUsage` swallows its own failures (returns on `tracked._tag === "Failure"`), a step that partially succeeds (e.g., `completeRun` commits, then a downstream operation throws and the step is retried) would re-invoke `trackIrisRunUsage` with the same `runId`/`costCents` and record a second usage event. The function lacks an internal dedupe keyed on `runId`, so the double-billing risk is proximately attributable to this helper. Confidence is medium because exploitation depends on workflow retry semantics and whether `completeRun`/run-status guards prevent re-finalization (both outside this file). Fix: Make usage tracking idempotent per run: either (a) check the run's existing tracked status before calling `client.track`, (b) use Autumn's idempotency key support (if available) keyed on `runId`, or (c) guard in `finalizeIrisRun` so `trackIrisRunUsage` only runs when the run transitions from a non-terminal to terminal status. At minimum, dedupe on `run_id` in properties before emitting a track event.</issue>
Commit d47ce71.
There was a problem hiding this comment.
1 issue found across 1 file.
Prompt for AI agents (all issues)
Check whether each issue below is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it, and use sub-agents to handle independent issues in parallel.
<file name="packages/ai/src/utils/iris-billing.ts">
<issue n="1" at="packages/ai/src/utils/iris-billing.ts:58-77" severity="MEDIUM">Billing silently skipped (fail-open) when the idempotency claim DB call fails — In `trackIrisRunUsage`, the idempotency claim is acquired via `acquireClaim(...)` wrapped in `Effect.catch(() => Effect.succeed({ claimed: false }))` (lines 58-69). `acquireClaim` itself (packages/ai/src/autonomy/claims.ts) also catches all errors and returns `{ claimed: false }`. When `claim.claimed === false`, the code logs `iris.billing.alreadyTracked` and returns WITHOUT billing (lines 71-77), treating 'not claimed' as synonymous with 'already billed'. This conflates two distinct conditions: (a) a run that was genuinely already billed, and (b) a transient failure to reach the claims database. Under condition (b) — e.g. a brief DB outage, connection timeout, or the `autonomy_claims` table being unavailable — the usage for that run is never recorded against the customer's AI-credit balance, and because `finalizeIrisRun` is a one-shot workflow step that is not re-invoked after a claim-acquisition failure, the usage is permanently lost (free usage). The 30-day TTL claim that would normally prevent re-billing is also never written, so the dedup safety net is absent as well. This is a fail-open design in a billing-critical path: an infrastructure hiccup in the claims DB results in unbilled AI consumption rather than a retried or blocked charge. It is primarily a revenue-integrity / financial-loss issue for the vendor (not directly attacker-exploitable, since triggering it requires inducing a DB outage), hence classified as a logic bug rather than a security vulnerability. The scanner's `crypto.randomUUID()` flag at line 60 is a false positive — `crypto.randomUUID()` is CSPRNG-backed in both Node and Bun and is the correct API for generating the owner token. Fix: Distinguish 'already claimed' from 'acquisition failed'. Have `acquireClaim` return a tri-state (e.g. `{ status: 'claimed' | 'already_claimed' | 'error', error? }`) instead of collapsing errors into `claimed: false`. In `trackIrisRunUsage`, only short-circuit on an explicit `already_claimed` result; on `error`, either retry the claim acquisition with backoff or fail the step so the workflow can retry finalization, rather than silently skipping billing. At minimum, log `iris.billing.claimAcquisitionFailed` (distinct from `alreadyTracked`) and emit an alert so transient claim-DB failures are visible and can be reconciled.</issue>
</file>
Commit c93cdf5 · Posted by Comp AI Code Reviews.
Comment on lines
+58
to
+77
| } | ||
|
|
||
| const claimToken = crypto.randomUUID(); | ||
| const claim = yield* Effect.tryPromise({ | ||
| try: () => | ||
| acquireClaim({ | ||
| scope: IRIS_BILLING_CLAIM_SCOPE, | ||
| claimKey: input.runId, | ||
| ownerToken: claimToken, | ||
| ttlSeconds: IRIS_BILLING_CLAIM_TTL_SECONDS, | ||
| organizationId: input.organizationId, | ||
| }), | ||
| catch: (cause) => cause, | ||
| }).pipe(Effect.catch(() => Effect.succeed({ claimed: false }))); | ||
|
|
||
| if (!claim.claimed) { | ||
| yield* Effect.annotateLogs(Effect.logInfo("iris.billing.alreadyTracked"), { | ||
| organizationId: input.organizationId, | ||
| runId: input.runId, | ||
| }); |
There was a problem hiding this comment.
MEDIUM: Billing silently skipped (fail-open) when the idempotency claim DB call fails
In trackIrisRunUsage, the idempotency claim is acquired via acquireClaim(...) wrapped in Effect.catch(() => Effect.succeed({ claimed: false })) (lines 58-69). acquireClaim itself (packages/ai/src/autonomy/claims.ts) also catches all errors and returns { claimed: false }. When claim.claimed === false, the code logs iris.billing.alreadyTracked and returns WITHOUT billing (lines 71-77), treating 'not claimed' as synonymous with 'already billed'.
This conflates two distinct conditions: (a) a run that was genuinely already billed, and (b) a transient failure to reach the claims database. Under condition (b) — e.g. a brief DB outage, connection timeout, or the autonomy_claims table being unavailable — the usage for that run is never recorded against the customer's AI-credit balance, and because finalizeIrisRun is a one-shot workflow step that is not re-invoked after a claim-acquisition failure, the usage is permanently lost (free usage). The 30-day TTL claim that would normally prevent re-billing is also never written, so the dedup safety net is absent as well.
This is a fail-open design in a billing-critical path: an infrastructure hiccup in the claims DB results in unbilled AI consumption rather than a retried or blocked charge. It is primarily a revenue-integrity / financial-loss issue for the vendor (not directly attacker-exploitable, since triggering it requires inducing a DB outage), hence classified as a logic bug rather than a security vulnerability. The scanner's crypto.randomUUID() flag at line 60 is a false positive — crypto.randomUUID() is CSPRNG-backed in both Node and Bun and is the correct API for generating the owner token.
Suggestion: Distinguish 'already claimed' from 'acquisition failed'. Have acquireClaim return a tri-state (e.g. { status: 'claimed' | 'already_claimed' | 'error', error? }) instead of collapsing errors into claimed: false. In trackIrisRunUsage, only short-circuit on an explicit already_claimed result; on error, either retry the claim acquisition with backoff or fail the step so the workflow can retry finalization, rather than silently skipping billing. At minimum, log iris.billing.claimAcquisitionFailed (distinct from alreadyTracked) and emit an alert so transient claim-DB failures are visible and can be reconciled.
Prompt for AI agents
Check whether this issue is valid; if so, find the root cause and fix it. Read the referenced code to confirm the problem before changing it.
<issue at="packages/ai/src/utils/iris-billing.ts:58-77" severity="MEDIUM">Billing silently skipped (fail-open) when the idempotency claim DB call fails — In `trackIrisRunUsage`, the idempotency claim is acquired via `acquireClaim(...)` wrapped in `Effect.catch(() => Effect.succeed({ claimed: false }))` (lines 58-69). `acquireClaim` itself (packages/ai/src/autonomy/claims.ts) also catches all errors and returns `{ claimed: false }`. When `claim.claimed === false`, the code logs `iris.billing.alreadyTracked` and returns WITHOUT billing (lines 71-77), treating 'not claimed' as synonymous with 'already billed'. This conflates two distinct conditions: (a) a run that was genuinely already billed, and (b) a transient failure to reach the claims database. Under condition (b) — e.g. a brief DB outage, connection timeout, or the `autonomy_claims` table being unavailable — the usage for that run is never recorded against the customer's AI-credit balance, and because `finalizeIrisRun` is a one-shot workflow step that is not re-invoked after a claim-acquisition failure, the usage is permanently lost (free usage). The 30-day TTL claim that would normally prevent re-billing is also never written, so the dedup safety net is absent as well. This is a fail-open design in a billing-critical path: an infrastructure hiccup in the claims DB results in unbilled AI consumption rather than a retried or blocked charge. It is primarily a revenue-integrity / financial-loss issue for the vendor (not directly attacker-exploitable, since triggering it requires inducing a DB outage), hence classified as a logic bug rather than a security vulnerability. The scanner's `crypto.randomUUID()` flag at line 60 is a false positive — `crypto.randomUUID()` is CSPRNG-backed in both Node and Bun and is the correct API for generating the owner token. Fix: Distinguish 'already claimed' from 'acquisition failed'. Have `acquireClaim` return a tri-state (e.g. `{ status: 'claimed' | 'already_claimed' | 'error', error? }`) instead of collapsing errors into `claimed: false`. In `trackIrisRunUsage`, only short-circuit on an explicit `already_claimed` result; on `error`, either retry the claim acquisition with backoff or fail the step so the workflow can retry finalization, rather than silently skipping billing. At minimum, log `iris.billing.claimAcquisitionFailed` (distinct from `alreadyTracked`) and emit an alert so transient claim-DB failures are visible and can be reconciled.</issue>
Commit c93cdf5.
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
1 participant
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.
Iris — Notra's autonomous marketing agent
Iris (Greek messenger goddess, counterpart to Hermes) watches what your org ships and does the marketing: she observes signals, decides what is announcement-worthy, drafts content with images, and asks on Slack before anything goes live.
The loop
Foundation
autonomy_*tables (mandates, signals, goals, runs, tasks, actions, checkpoints, outbox, claims, controller leases) in one additive migration (0061).Verified
Naming note: user-facing agent name is Iris; internal infrastructure keeps the autonomy_ prefix.
Summary by cubic
Adds Iris, an autonomous marketing agent that watches your sources, plans tasks, drafts content, and routes a Slack “Ship it / Skip” card for approval. Access is controlled by a Databuddy org-scoped feature flag (single gate; env allowlist removed), with tenancy-checked Slack approvals and sanitized inputs.
New Features
@notra/ai: planner-validated task DAG; claims/leases; gate with daily action/cost budgets; signals dedupe; outbox with delivery/retry; ship flow.apps/agentinteractions; approver recorded; message updated; publishing via@notra/aiSlack integration./api/workflows/iris; dashboard page/[slug]/iris(start/pause, stats, runs, signals, artifacts); ORPCirisrouter; 30‑min wake schedule; flag-gated nav and “Iris unavailable” state.autonomy_*migrations into one additive migration.Bug Fixes
totalTokensfallback to avoid cache write double count when usage totals are unavailable; markup parity with Autumn, sandbox cost floor applied, cache normalization for consistent pricing; idempotent per-run billing with claim guard and a safe markup fallback if balance checks fail.Written for commit c93cdf5. Summary will update on new commits.
Summary by Comp AI
1 issue found.
Written for commit
c93cdf5. New commits will trigger a re-review. Generated by Comp AI.