feat(core): Add the instance-context block and activity tool (no-changelog) - #37867
Draft
DeveloperTheExplorer wants to merge 14 commits into
Draft
Conversation
Gates handing the agent a per-turn block of instance context, and the tool that reads further back. Default off. Separate from the write flag: that one decides whether the record exists at all, and the record is core with other potential consumers. Two of the three sources this gates are not the activity log in the first place — what exists comes from the workflows, and runs come from `execution_entity`.
Adds the per-resource index the table deliberately left out, along with the two reads that need it: one entry by id, and one resource's own history. Both are project-scoped in the query rather than by the caller, so an entry in another project answers exactly as a pruned one does. A reader cannot use an id it guessed to find out what it cannot see.
One row per workflow, with the run and failure counts and the id of the last failure. For telling an agent what has been running and what broke. Aggregated in the database rather than folded in memory: a busy instance has far more runs than a reader would show. Grouping is also what makes runs fold per workflow across the whole window rather than only where they happen to sit next to each other — two schedules on different intervals interleave. Bounded by `stoppedAt` rather than by an execution id, because a run that started before a caller's last read and failed after it has a low id and a recent outcome. Evaluation runs are excluded; they are machine-paced and would bury everything a person did.
…angelog) An event log cannot say what exists: a workflow nobody has touched lately produces no events at all, while being exactly the work somebody might want picked up. `active` is aggregated as an integer rather than as the boolean column. Postgres has no `max(boolean)`, so aggregating it directly fails there while passing on sqlite.
Renders what is going on in an instance as one tagged block: what exists here, what changed lately, and what has run. Three sources with three lifetimes, so each gets its own freshness rule — the workflows themselves for what exists, `activity_event` for changes, `execution_entity` for runs. A thread gets the full window once and only additions afterwards. The delta reads a band below the high-water mark and drops what it has already shown, because ids are an ordering key and not a completeness watermark: Postgres allocates a sequence value outside the surrounding transaction, so a cursor asking for "everything above the highest id seen" skips a row that committed late — and deletions, the entries most worth surfacing, are written by whichever request happens to be committing. Runs use their own bound on `stoppedAt`, since a run can start before a read and fail after it. Nothing is emitted when nothing exists, changed or ran: a block saying so is pure cost, and invites the agent to comment on it. A failure to build one is logged and dropped rather than failing the user's turn.
The block rides the per-turn user message, never `getSystemPrompt()`: that prompt is one shared cache entry across every thread on the instance, which is why the clock and the project name ride the turn too. It sits last of the leading blocks, nearest the user's own words, since it is background for reading their intent rather than a statement of what they are looking at. The cursor is stored on the thread, so the next turn sends only additions. Machine follow-up turns skip it. A checkpoint or a planned build is the agent continuing its own task, where nobody is reading intent and the whole block would be paid for unread.
The block the agent is handed is a window; this is how it looks further back, filters to one kind of entry or one resource, or opens a single one. Expanding an entry also returns the rest of that resource's history, which is how the agent sees a workflow's recent past in one call. Read-only, and it returns log entries rather than live records: `workflows`, `executions` and `credentials` already fetch those and carry their own permission checks, so duplicating them here would drift from them. The adapter binds the reader to the conversation's user and project, so the tool cannot widen its own scope. A pruned id and an id in another project both answer `notFound`, so it cannot be used to find out what it cannot see. The tool is registered only when the reader is enabled, and is never deferred behind `search_tools` — the block names it, so an expand would otherwise cost two extra rounds.
How to use the block and the tool: read state rather than asking about it, stop at the cheapest rung that answers the question, and resolve a vague opener from the most recent thing rather than handing the user a list they can already see. Gated on the reader like `config-evals` is on its own flag. The skill is entirely about a block and a tool that do not exist with the reader off, so listing it would advertise both.
…angelog) Against a real database, because the properties worth proving are the ones mocks cannot: that no leg of the block leaks another project's work, that an entry sitting behind the high-water mark and never shown still reaches the reader, that an out-of-scope id answers exactly as a pruned one does, and that evaluation runs stay out. The straggler case states the scenario through the cursor rather than by forcing ids: an explicit id is honoured on sqlite and ignored on Postgres, so a test written that way would only test one driver.
…no-changelog) The workspace test builds a partial service off the prototype, so it has to supply the reader the skill gate now asks for. Disabled there, which is what the skill catalog that test asserts on expects.
Review found the delta's read cap interacting with the lag band. The cap is newest-first, so when more rows sit above the floor than it returns, the ones dropped are the lowest and the mark then advances past them. No row the window could have shown is lost by this — a dropped row sits below a cap's worth of newer ones, and the window shows far fewer than that. What was wrong is the promise: a single capped read cannot cover an unbounded number of arrivals plus the band, so the comment now states the guarantee the code keeps rather than one it cannot. The cap is derived from the window size, which is what makes even that much true, and the seen-id cap is derived from the band width so the two cannot drift apart. What was genuinely missing is the signal. A cut list that does not say it is cut reads as the whole story, so it now names the tool that reads the rest. Also from review: a type guard in place of a cast when reading the cursor, matching the local helper four other files in this module already use.
Contributor
|
Running ultrareview automatically — This adds a cross-cutting instance-context feature with new DB queries, an index migration, and a new agent tool — subtle bugs here could leak project-scoped data or misreport run/workflow history, warranting a deep, multi-pass review.. I'll post findings when complete. |
Contributor
! PR exceeds size limit (1,172 lines added)This PR adds 1,172 lines, exceeding the 1,000-line limit (test files excluded). Large PRs are harder to review and increase the risk of bugs going unnoticed. Please consider:
If the size is genuinely justified (e.g. generated code, large migrations, test fixtures), a maintainer can override by commenting |
Contributor
PR review overviewBased on ownership of the 32 changed files in this PR:
Required reviewsSome changed files have a
Request a review from the team — GitHub assigns reviewers according to the team's review settings. The ❗ Source code additions (1,172) exceed the 1,000-line limit. |
Bundle ReportChanges will increase total bundle size by 53.43kB (0.08%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: editor-ui-esmAssets Changed:
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
Instance AI Discovery Eval ✅Branch: Eval output |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Ultrareview completed in 15m 45s
14 issues found across 32 files
Confidence score: 2/5
- The delta-boundary logic in
packages/cli/src/modules/instance-ai/instance-context.service.tscan movestoppedAfterahead ofrunsThrough, causing the second context block to include removals despite the additions-only contract; correct the cursor range handling. - The pagination logic in
packages/cli/src/modules/instance-ai/instance-context.service.tscan stop after filling a page with already-seen rows, leaving unseen events unrecovered and making later context stale; continue paging until an unseen event is found or the source is exhausted. packages/cli/src/modules/instance-ai/instance-ai.service.tscan advance the context cursor before the SDK accepts the input, so setup failures or an intervening Stop can permanently suppress the context block on the next turn; commit the cursor only after successful injection.- The execution summary in
packages/@n8n/db/src/repositories/execution.repository.tscan misidentify the latest failed run and report workflows with cancellations as fully successful, producing incorrect run-status context; order failures bystoppedAtand preserve a canceled/non-success breakdown.
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/cli/src/modules/instance-ai/instance-context.service.ts">
<violation number="1" location="packages/cli/src/modules/instance-ai/instance-context.service.ts:208">
P2: According to linked Linear issue CONTEXT-100, only genuine emptiness should suppress the block. Use per-leg settled results, preserve each failed leg's prior cursor, and still render successful legs.</violation>
<violation number="2" location="packages/cli/src/modules/instance-ai/instance-context.service.ts:315">
P1: Custom agent: **Backend**
According to linked Linear issue CONTEXT-100, a delta must recover an unseen event that commits behind the activity mark. After the query page is filled by already-seen rows, this limit prevents the reader from reaching older unseen rows inside the lag band, so the same page repeats and the late event is lost. Fetch the full lag band for cursor reads or paginate past seen IDs before advancing the mark.</violation>
<violation number="3" location="packages/cli/src/modules/instance-ai/instance-context.service.ts:352">
P1: Custom agent: **Backend**
According to linked Linear issue CONTEXT-100, the second block must contain only additions. When the next turn is within two minutes, this subtraction moves `stoppedAfter` before `runsThrough`, so the aggregate returns previously summarized executions and `renderRuns` emits them as `Runs since then` without execution-level de-duplication. Persist execution IDs for the overlap and filter already reported runs, or use an exclusive timestamp range bounded by the read time.</violation>
<violation number="4" location="packages/cli/src/modules/instance-ai/instance-context.service.ts:426">
P2: When a workflow has a canceled run and no error, this output says it succeeded. Render `no failures` or carry a canceled count so cancellations are not reported as successes.</violation>
</file>
<file name="packages/@n8n/instance-ai/skills/instance-awareness/SKILL.md">
<violation number="1" location="packages/@n8n/instance-ai/skills/instance-awareness/SKILL.md:53">
P2: `expand` returns at most 20 recent entries for a resource, so this promise can make the agent treat a partial history as complete. Say that it returns up to 20 recent entries.</violation>
<violation number="2" location="packages/@n8n/instance-ai/skills/instance-awareness/SKILL.md:65">
P2: For large workflows, this call returns only a structural summary by default, so it cannot answer the parameter and prompt-structure questions listed below. Pass `full=true` for this rung.</violation>
<violation number="3" location="packages/@n8n/instance-ai/skills/instance-awareness/SKILL.md:77">
P2: When an instance has more than eight workflows, the block contains only the eight most recently updated workflows and a count. Route complete-inventory questions to `workflows(action="list")` instead of treating rung 0 as complete.</violation>
</file>
<file name="packages/cli/src/modules/instance-ai/__tests__/internal-messages.test.ts">
<violation number="1" location="packages/cli/src/modules/instance-ai/__tests__/internal-messages.test.ts:111">
P3: The test name says the trailing project-context block is left intact, but the assertion strips it: the fixture ends with `<project-context>...</project-context>` and `cleanStoredUserMessage` returns only 'Carry on'. Rename the test to say it strips the trailing project/clock blocks (or to 'leaves the user text intact after the trailing blocks are stripped') so the name does not mislead a future maintainer about the stripping contract.</violation>
</file>
<file name="packages/cli/src/modules/instance-ai/internal-messages.ts">
<violation number="1" location="packages/cli/src/modules/instance-ai/internal-messages.ts:60">
P2: When an instance-context value contains `</instance-context>\n\n`, this matcher ends the block early and corrupts the message shown after reload. Escape delimiter-bearing names/details before rendering the block, or replace this tag protocol with a delimiter-safe representation.</violation>
</file>
<file name="packages/@n8n/db/src/repositories/execution.repository.ts">
<violation number="1" location="packages/@n8n/db/src/repositories/execution.repository.ts:716">
P2: When a workflow has only canceled runs, or successes plus cancellations, this returns `failed === 0` and `renderRuns` says all runs succeeded. Return a success/non-success breakdown or change the renderer so cancellation is not reported as success.</violation>
<violation number="2" location="packages/@n8n/db/src/repositories/execution.repository.ts:725">
P2: According to linked Linear issue CONTEXT-100, `stoppedAt` is the ordering bound for late failures. When failed runs stop out of order, `MAX(execution.id)` labels an older failure as the last failure; select the failed ID by `stoppedAt` with a deterministic ID tie-breaker.</violation>
</file>
<file name="packages/@n8n/instance-ai/src/types.ts">
<violation number="1" location="packages/@n8n/instance-ai/src/types.ts:635">
P2: When SQLite age pruning removes every activity row, the next row can reuse an earlier id, so `expand(id)` returns unrelated history for a stale bracketed id. According to linked Linear issue CONTEXT-100, pruned ids must return `notFound`; use a non-reusable event token or include the original event generation/timestamp in the expansion lookup.</violation>
</file>
<file name="packages/cli/src/modules/instance-ai/instance-ai.service.ts">
<violation number="1" location="packages/cli/src/modules/instance-ai/instance-ai.service.ts:3976">
P2: If agent setup fails or Stop arrives after this patch but before the SDK accepts the input, the cursor marks the context as shown even though no `<instance-context>` block is in the conversation; the next turn then emits only a delta and silently loses the opening context. Persist the cursor only after the SDK has accepted the message, and make that metadata update best-effort so a cursor-write failure cannot fail the user's turn.</violation>
</file>
<file name="packages/@n8n/instance-ai/src/tools/activity.tool.ts">
<violation number="1" location="packages/@n8n/instance-ai/src/tools/activity.tool.ts:117">
P3: When `resourceId` is the valid-but-empty string, this truthiness check drops the filter and returns the entire scoped feed instead of no matches. Preserve explicitly supplied values by checking `input.resourceId !== undefined`, or reject empty IDs in the schema.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| now: Date; | ||
| }): Promise<RunSummary[]> { | ||
| const stoppedAfter = input.cursor | ||
| ? new Date(Date.parse(input.cursor.runsThrough) - runLagMs) |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Backend
According to linked Linear issue CONTEXT-100, the second block must contain only additions. When the next turn is within two minutes, this subtraction moves stoppedAfter before runsThrough, so the aggregate returns previously summarized executions and renderRuns emits them as Runs since then without execution-level de-duplication. Persist execution IDs for the overlap and filter already reported runs, or use an exclusive timestamp range bounded by the read time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/instance-ai/instance-context.service.ts, line 352:
<comment>According to linked Linear issue CONTEXT-100, the second block must contain only additions. When the next turn is within two minutes, this subtraction moves `stoppedAfter` before `runsThrough`, so the aggregate returns previously summarized executions and `renderRuns` emits them as `Runs since then` without execution-level de-duplication. Persist execution IDs for the overlap and filter already reported runs, or use an exclusive timestamp range bounded by the read time.</comment>
<file context>
@@ -0,0 +1,608 @@
+ now: Date;
+ }): Promise<RunSummary[]> {
+ const stoppedAfter = input.cursor
+ ? new Date(Date.parse(input.cursor.runsThrough) - runLagMs)
+ : new Date(input.now.getTime() - maxAgeMs);
+
</file context>
| const floor = cursor ? Math.max(0, cursor.activityMark - activityLagIds) : undefined; | ||
|
|
||
| const rows = await this.activityEventRepository.findFeed({ | ||
| limit: entryFetchLimit, |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Backend
According to linked Linear issue CONTEXT-100, a delta must recover an unseen event that commits behind the activity mark. After the query page is filled by already-seen rows, this limit prevents the reader from reaching older unseen rows inside the lag band, so the same page repeats and the late event is lost. Fetch the full lag band for cursor reads or paginate past seen IDs before advancing the mark.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/instance-ai/instance-context.service.ts, line 315:
<comment>According to linked Linear issue CONTEXT-100, a delta must recover an unseen event that commits behind the activity mark. After the query page is filled by already-seen rows, this limit prevents the reader from reaching older unseen rows inside the lag band, so the same page repeats and the late event is lost. Fetch the full lag band for cursor reads or paginate past seen IDs before advancing the mark.</comment>
<file context>
@@ -0,0 +1,608 @@
+ const floor = cursor ? Math.max(0, cursor.activityMark - activityLagIds) : undefined;
+
+ const rows = await this.activityEventRepository.findFeed({
+ limit: entryFetchLimit,
+ projectIds: input.projectIds,
+ ...(floor !== undefined ? { afterId: floor } : {}),
</file context>
Suggested change
| limit: entryFetchLimit, | |
| limit: cursor ? activityLagIds : entryFetchLimit, |
|
|
||
| ### 2. One workflow, read in full | ||
|
|
||
| `workflows(action="get", workflowId)` on **one** example — the one the block |
Contributor
There was a problem hiding this comment.
P2: For large workflows, this call returns only a structural summary by default, so it cannot answer the parameter and prompt-structure questions listed below. Pass full=true for this rung.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/@n8n/instance-ai/skills/instance-awareness/SKILL.md, line 65:
<comment>For large workflows, this call returns only a structural summary by default, so it cannot answer the parameter and prompt-structure questions listed below. Pass `full=true` for this rung.</comment>
<file context>
@@ -0,0 +1,117 @@
+
+### 2. One workflow, read in full
+
+`workflows(action="get", workflowId)` on **one** example — the one the block
+points at, or the one the user named. This rung is for what an entry cannot
+express: parameter values, naming, retry settings, error-workflow wiring, how a
</file context>
|
|
||
| `list` looks further back than the block, or filters to one category or one | ||
| resource. `expand` opens a single entry in full **and returns everything else | ||
| the log knows about the same resource** — which is how you see one workflow's |
Contributor
There was a problem hiding this comment.
P2: expand returns at most 20 recent entries for a resource, so this promise can make the agent treat a partial history as complete. Say that it returns up to 20 recent entries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/@n8n/instance-ai/skills/instance-awareness/SKILL.md, line 53:
<comment>`expand` returns at most 20 recent entries for a resource, so this promise can make the agent treat a partial history as complete. Say that it returns up to 20 recent entries.</comment>
<file context>
@@ -0,0 +1,117 @@
+
+`list` looks further back than the block, or filters to one category or one
+resource. `expand` opens a single entry in full **and returns everything else
+the log knows about the same resource** — which is how you see one workflow's
+change history in a single call.
+
</file context>
| plannedBuild?.isPlannedBuildFollowUp === true, | ||
| }); | ||
| if (instanceContext) { | ||
| await patchThread(memory, { |
Contributor
There was a problem hiding this comment.
P2: If agent setup fails or Stop arrives after this patch but before the SDK accepts the input, the cursor marks the context as shown even though no <instance-context> block is in the conversation; the next turn then emits only a delta and silently loses the opening context. Persist the cursor only after the SDK has accepted the message, and make that metadata update best-effort so a cursor-write failure cannot fail the user's turn.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/instance-ai/instance-ai.service.ts, line 3976:
<comment>If agent setup fails or Stop arrives after this patch but before the SDK accepts the input, the cursor marks the context as shown even though no `<instance-context>` block is in the conversation; the next turn then emits only a delta and silently loses the opening context. Persist the cursor only after the SDK has accepted the message, and make that metadata update best-effort so a cursor-write failure cannot fail the user's turn.</comment>
<file context>
@@ -3949,6 +3958,29 @@ export class InstanceAiService {
+ plannedBuild?.isPlannedBuildFollowUp === true,
+ });
+ if (instanceContext) {
+ await patchThread(memory, {
+ threadId,
+ update: ({ metadata }) => ({
</file context>
| expect(cleanStoredUserMessage(stored)).toBe('Why did it fail?'); | ||
| }); | ||
|
|
||
| it('leaves the project and clock blocks that follow the user text intact when stripping it', () => { |
Contributor
There was a problem hiding this comment.
P3: The test name says the trailing project-context block is left intact, but the assertion strips it: the fixture ends with <project-context>...</project-context> and cleanStoredUserMessage returns only 'Carry on'. Rename the test to say it strips the trailing project/clock blocks (or to 'leaves the user text intact after the trailing blocks are stripped') so the name does not mislead a future maintainer about the stripping contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/instance-ai/__tests__/internal-messages.test.ts, line 111:
<comment>The test name says the trailing project-context block is left intact, but the assertion strips it: the fixture ends with `<project-context>...</project-context>` and `cleanStoredUserMessage` returns only 'Carry on'. Rename the test to say it strips the trailing project/clock blocks (or to 'leaves the user text intact after the trailing blocks are stripped') so the name does not mislead a future maintainer about the stripping contract.</comment>
<file context>
@@ -81,6 +92,32 @@ describe('cleanStoredUserMessage', () => {
+ expect(cleanStoredUserMessage(stored)).toBe('Why did it fail?');
+ });
+
+ it('leaves the project and clock blocks that follow the user text intact when stripping it', () => {
+ const stored = [
+ instanceContextMarker(),
</file context>
Suggested change
| it('leaves the project and clock blocks that follow the user text intact when stripping it', () => { | |
| it('strips the project and clock blocks that follow the user text as well', () => { |
| const entries = await service.list({ | ||
| limit: input.limit ?? defaultListLimit, | ||
| ...(input.category ? { category: input.category } : {}), | ||
| ...(input.resourceId ? { resourceId: input.resourceId } : {}), |
Contributor
There was a problem hiding this comment.
P3: When resourceId is the valid-but-empty string, this truthiness check drops the filter and returns the entire scoped feed instead of no matches. Preserve explicitly supplied values by checking input.resourceId !== undefined, or reject empty IDs in the schema.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/@n8n/instance-ai/src/tools/activity.tool.ts, line 117:
<comment>When `resourceId` is the valid-but-empty string, this truthiness check drops the filter and returns the entire scoped feed instead of no matches. Preserve explicitly supplied values by checking `input.resourceId !== undefined`, or reject empty IDs in the schema.</comment>
<file context>
@@ -0,0 +1,123 @@
+ const entries = await service.list({
+ limit: input.limit ?? defaultListLimit,
+ ...(input.category ? { category: input.category } : {}),
+ ...(input.resourceId ? { resourceId: input.resourceId } : {}),
+ ...(input.beforeId !== undefined ? { beforeId: input.beforeId } : {}),
+ });
</file context>
Suggested change
| ...(input.resourceId ? { resourceId: input.resourceId } : {}), | |
| ...(input.resourceId !== undefined ? { resourceId: input.resourceId } : {}), |
…ngelog) Two faults review found in the new reads, both invisible on a UTC sqlite box. sqlite stores `stoppedAt` as UTC wall-clock text with no zone, and `new Date` reads that as local time — so west of UTC every run moved into the future and `formatAge` clamped it to "1m ago", making a stale failure look current on the one signal the block exists for. `parseDbTime` already existed for exactly this and is now used. The test asserts the instant survives the round trip; it fails by the size of the local offset without the fix, so run it under `TZ=America/New_York` to see it bite. `active` is deprecated in favour of `activeVersionId`, and every sibling reader in this repository had already moved. A workflow can carry `active: true` with no published version, so the inventory called it published when it is not.
…hangelog) **Names could break out of the block.** One holding a newline and a closing tag ended it early, so what followed read to the model as the user's own words, and on reload `cleanStoredUserMessage` stripped the wrong span and showed the injected text as the message. The boundary here is the project, not authorship, so the name need not be the reader's own. Every stored value now passes one sanitiser: control characters become spaces, angle brackets are escaped so a name that legitimately holds one still reads as itself, and the value is bounded. A round trip asserts the block keeps exactly one pair of tags and that the user gets their own message back. **The reader had a second scoping path, and it was unreachable.** A run throws without a bound project, so the "every project the user belongs to" branch could never run — while being the likeliest thing here to drift into a leak, and resting on bare membership, which is not read access. It is gone, along with the project cap that only guarded it and cited a bind limit this repo documents differently. Scope is now the conversation's own project or nothing. **A narrowing request could widen the read.** An unrecognised `category` dropped the filter and returned the whole feed; it now matches nothing, and the tool's schema no longer admits one. Also: `activitySeen` recorded rows read rather than rows shown, so an entry the window cut was buried under a mark it never reached; the tool follows the discriminated-union shape its eight siblings use, which makes a missing id unrepresentable rather than hand-checked; `isRecord` comes from `@n8n/utils` instead of a local copy; a failed build logs at warn, since debug hides a leg that never works; and the note on the fetch cap no longer claims the wrong constant is the derived one.
…-changelog) Thread access proves the thread belongs to the caller and nothing more, so a user removed from a project kept reading its inventory, runs and activity here for the life of the thread — while every other read in this module refused them. Scope is now re-checked against the permission tables each turn and each tool call, and all three entry points fail closed. `workflow:read` stands for the whole block: it is what the inventory and run legs expose, and credential entries carry a name and a type rather than a secret. The integration test binds a conversation to a project the user is not a member of and asserts nothing comes back, which exercises the real permission tables rather than a mock.
Contributor
There was a problem hiding this comment.
2 issues found across 11 files (changes from recent commits).
Confidence score: 2/5
- In
packages/@n8n/instance-ai/src/tools/activity.tool.ts, enabling the activity tool for an Anthropic agent can expose a discriminated-union schema with a non-object root, causing the provider to reject the tool schema; wrap the schema in an object before exposing it. - In
packages/cli/src/modules/instance-ai/__tests__/instance-context.service.test.ts, theentry()fixture setsuserinstead of theuserIdconsumed bytoActivityEntry, which can invalidate thebyCurrentUserassertion and obscure activity attribution regressions; update the fixture to provideuserIdand align it withActivityEvent.
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/@n8n/instance-ai/src/tools/activity.tool.ts">
<violation number="1" location="packages/@n8n/instance-ai/src/tools/activity.tool.ts:19">
P1: When the activity flag enables this tool for an Anthropic agent, the new discriminated union is exposed without provider sanitization, so the provider can reject the tool schema because its root is not an object. Wrap this union with `sanitizeInputSchema`, as the other native union tools do.</violation>
</file>
<file name="packages/cli/src/modules/instance-ai/__tests__/instance-context.service.test.ts">
<violation number="1" location="packages/cli/src/modules/instance-ai/__tests__/instance-context.service.test.ts:33">
P2: The `entry()` factory now sets `user: USER` and no longer sets `userId`, but `ActivityEvent` has no `user` field — the service reads `row.userId` in `toActivityEntry` (`byCurrentUser: row.userId === currentUserId`) and `toFeedEntry` (`row.userId && row.userId !== currentUserId ? 'by another user'`). Every entry built from this factory now carries `userId === undefined`, so `byCurrentUser` is always false and entries render as 'by another user' if a test ever asserts on it. Set `userId: USER_ID` in the factory instead; `user: USER` belongs only on the `buildBlock`/`list`/`expand` arguments, not on the `ActivityEvent` fixture.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| const defaultListLimit = 30; | ||
| const maxListLimit = 100; | ||
|
|
||
| const activityInputSchema = z.discriminatedUnion('action', [ |
Contributor
There was a problem hiding this comment.
P1: When the activity flag enables this tool for an Anthropic agent, the new discriminated union is exposed without provider sanitization, so the provider can reject the tool schema because its root is not an object. Wrap this union with sanitizeInputSchema, as the other native union tools do.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/@n8n/instance-ai/src/tools/activity.tool.ts, line 19:
<comment>When the activity flag enables this tool for an Anthropic agent, the new discriminated union is exposed without provider sanitization, so the provider can reject the tool schema because its root is not an object. Wrap this union with `sanitizeInputSchema`, as the other native union tools do.</comment>
<file context>
@@ -16,42 +16,40 @@ import { DOMAIN_TOOL_IDS } from './tool-ids';
- .optional()
- .describe(`For \`list\`: how many entries to return (default ${defaultListLimit}).`),
-});
+const activityInputSchema = z.discriminatedUnion('action', [
+ z.object({
+ action: z
</file context>
| category: 'workflow', | ||
| action: 'saved', | ||
| typeVersion: 1, | ||
| user: USER, |
Contributor
There was a problem hiding this comment.
P2: The entry() factory now sets user: USER and no longer sets userId, but ActivityEvent has no user field — the service reads row.userId in toActivityEntry (byCurrentUser: row.userId === currentUserId) and toFeedEntry (row.userId && row.userId !== currentUserId ? 'by another user'). Every entry built from this factory now carries userId === undefined, so byCurrentUser is always false and entries render as 'by another user' if a test ever asserts on it. Set userId: USER_ID in the factory instead; user: USER belongs only on the buildBlock/list/expand arguments, not on the ActivityEvent fixture.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/instance-ai/__tests__/instance-context.service.test.ts, line 33:
<comment>The `entry()` factory now sets `user: USER` and no longer sets `userId`, but `ActivityEvent` has no `user` field — the service reads `row.userId` in `toActivityEntry` (`byCurrentUser: row.userId === currentUserId`) and `toFeedEntry` (`row.userId && row.userId !== currentUserId ? 'by another user'`). Every entry built from this factory now carries `userId === undefined`, so `byCurrentUser` is always false and entries render as 'by another user' if a test ever asserts on it. Set `userId: USER_ID` in the factory instead; `user: USER` belongs only on the `buildBlock`/`list`/`expand` arguments, not on the `ActivityEvent` fixture.</comment>
<file context>
@@ -26,7 +30,7 @@ function entry(overrides: Partial<ActivityEvent> = {}): ActivityEvent {
action: 'saved',
typeVersion: 1,
- userId: USER_ID,
+ user: USER,
projectId: PROJECT_ID,
resourceType: 'workflow',
</file context>
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.
Summary
Gives the assistant progressive access to what is going on in an instance, behind an off-by-default
read flag. Follows #37750, which writes the record this reads. Three parts.
1. A per-turn
<instance-context>block. It rides the user message, nevergetSystemPrompt():that prompt is one shared prompt-cache entry across every thread on the instance, which is why the
clock and the project name ride the turn too. It carries three legs, each from the source that can
actually answer it:
workflow+shared_workflowactivity_eventexecution_entity2. An
activitytool (list/expand) so the agent can look further back than the window, oropen one entry together with the rest of that resource's history.
3. Cursors on the thread, so a thread gets the full window once and only additions afterwards.
Re-sending the window would leave two blocks whose counts disagree — "ran 3×, all succeeded" above
"ran 40×, 2 failed" for the same workflow — and nothing can retract the older one.
Three details worth a reviewer's attention
The activity cursor is not a high-water mark, on purpose. Postgres allocates a sequence value
outside the surrounding transaction, so id 101 can commit before id 100. A cursor asking for
"everything above the highest id seen" skips 100 for good — and deletions, the entries most worth
surfacing, are written by whichever request happens to be committing. So a delta re-reads a bounded
band below the mark and drops the ids it has already shown. The read is capped and newest-first, so
the guarantee it keeps is "a straggler is recovered whenever it could be displayed" rather than
"every straggler is recovered"; the cap is derived from the window size, which is what makes that
much true. When the list is cut, the block says so instead of reading as the whole story.
Runs use a separate
stoppedAtbound, not an id cursor. A run can start before a read and failafter it, so its id is low while its outcome is new — an id cursor would miss exactly the failure
worth surfacing.
Every leg is project-scoped in the query, not by the caller. A run has no acting user, so project
is the only boundary available.
findEntryandfindByResourcetake the scope themselves, whichmakes "an out-of-scope id answers exactly like a pruned one" a property of the repository rather
than of caller discipline — the tool cannot be used to find out what it cannot see.
Also here: the follow-up-turn guard. A checkpoint or planned-build turn is the agent continuing its
own task, where nobody is reading intent, so the block is skipped before any read happens.
How to test
The reader needs both flags — this one, and the write flag from #37750 that fills the log:
instance-aiis a default module, so nothing else needs enabling. An empty log on a fresh instanceis expected — the workflow and run legs still work, since neither reads it.
its failure rather than asking you to choose.
resource's history.
longer repeat the workflow inventory.
N8N_INSTANCE_AI_INSTANCE_CONTEXT_ENABLEDoff. The block, theactivitytool and theinstance-awarenessskill should all disappear, and no reads should be issued.Automated:
That Postgres run is not decoration — it caught two faults that pass on sqlite. Postgres has no
max(boolean), so aggregating theactivecolumn directly fails there; and an explicit primary keyis honoured on sqlite and ignored on Postgres, which made an earlier version of the
out-of-order-commit test green on one driver only.
Related Linear tickets, Github issues, and Community forum posts
Deliberately out of scope, each with its own ticket: telemetry and rollout (CONTEXT-104), trace
visibility of the injected block (CONTEXT-106), docs (CONTEXT-107), and MCP exposure (CONTEXT-109).
Review / Merge checklist
Backport to Beta,Backport to Stable, orBackport to v1(if the PR is an urgent fix that needs to be backported)🤖 PR Summary generated by AI