-
Notifications
You must be signed in to change notification settings - Fork 0
ENG-3697: Bound Live and Get responses, and stop List deleting its own content #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
c5f4232
704ce22
81d0303
efa1c25
1a5bb3c
5b9b682
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| import type { | ||
| IExecuteSingleFunctions, | ||
| IHttpRequestOptions, | ||
| INodeProperties, | ||
| } from 'n8n-workflow'; | ||
| import { NodeOperationError } from 'n8n-workflow'; | ||
|
|
||
| // Connection ID is the one Live field an AI Agent gets wrong in a way the API | ||
| // cannot recover from. | ||
| // | ||
| // The node sets `usableAsTool`, so when it runs as an Agent tool the model | ||
| // fills the parameters from their descriptions. "Specific connection ID when | ||
| // the user has multiple connections for this source" reads, to a model, like | ||
| // an invitation to name the source — and that is exactly what happens. | ||
| // Observed in prod on 2026-08-10: `connection_id` arriving as 'linear', | ||
| // 'github', 'google_drive'. | ||
| // | ||
| // Core takes that string straight into a UUID-typed column | ||
| // (`live_access.py`: `stmt.where(Connection.id == connection_id)`), asyncpg | ||
| // rejects it with a DataError, and a blanket `except Exception` in | ||
| // `api/live.py` turns it into `502 "Upstream source error."`. That message is | ||
| // worse than useless: it blames the provider when no provider was ever | ||
| // contacted, so the workflow author reasonably concludes Linear is down. | ||
| // | ||
| // Core should return a 400 here and will (tracked separately) — but the node | ||
| // should not be shipping a malformed request in the first place, and this | ||
| // guard keeps working against every already-deployed core version. | ||
| const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; | ||
|
|
||
| /** | ||
| * preSend: reject a Connection ID that isn't a UUID, naming the likely mistake. | ||
| * | ||
| * Deliberately NOT a silent drop. Silently ignoring the field would make a | ||
| * request scoped to the wrong connection look like it succeeded — the same | ||
| * class of silent-wrong-answer this node has been bitten by before. | ||
| */ | ||
| export async function validateConnectionId( | ||
| this: IExecuteSingleFunctions, | ||
| requestOptions: IHttpRequestOptions, | ||
| ): Promise<IHttpRequestOptions> { | ||
| const raw = this.getNodeParameter('connection_id', '') as string | undefined; | ||
| const value = typeof raw === 'string' ? raw.trim() : ''; | ||
| if (value === '' || UUID_RE.test(value)) return requestOptions; | ||
|
|
||
| throw new NodeOperationError( | ||
| this.getNode(), | ||
| `Connection ID must be a connection UUID, not "${value}".`, | ||
| { | ||
| description: | ||
| 'Connection ID identifies ONE specific connection when a user has connected the same source more than once — it is not the source name. Leave it empty to use the caller\'s connection automatically. If this node is running as an AI Agent tool, pin this field to empty so the model cannot fill it.', | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * The Connection ID field, shared by the three document-shaped Live operations. | ||
| * | ||
| * `transport` differs by operation — Search POSTs a body, Get and List Resources | ||
| * put it on the query string — but the validation and the wording must not, so | ||
| * both go through here rather than being restated three times. | ||
| */ | ||
| export function connectionIdProperty( | ||
| show: NonNullable<INodeProperties['displayOptions']>['show'], | ||
| transport: 'body' | 'query' = 'body', | ||
| ): INodeProperties { | ||
| return { | ||
| displayName: 'Connection ID', | ||
| name: 'connection_id', | ||
| type: 'string', | ||
| default: '', | ||
| placeholder: '', | ||
| displayOptions: { show }, | ||
| // Description is the model's only input when this runs as a tool, so it | ||
| // states the format, and states the default, before it states the purpose. | ||
| description: | ||
| 'Leave empty. Optional UUID identifying one specific connection, used only when the same user has connected this source more than once. This is NOT the source name.', | ||
| routing: { | ||
| send: { | ||
| type: transport, | ||
| property: 'connection_id', | ||
| // `|| undefined` so an untouched field is OMITTED rather than sent as | ||
| // "". Core types it `str | None` and only falsy-checks it, so "" | ||
| // happens to be harmless — but the sibling Live ops say "no connection | ||
| // specified" by omission, and this one shouldn't rely on a | ||
| // falsy-string coincidence. | ||
| value: '={{ $value || undefined }}', | ||
| preSend: [validateConnectionId], | ||
|
entelligence-ai-pr-reviews[bot] marked this conversation as resolved.
|
||
| }, | ||
| }, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,29 +46,73 @@ interface Highlight { | |
| score?: number; | ||
| } | ||
|
|
||
| interface HyperdocNode extends IDataObject { | ||
| text?: string; | ||
| children?: HyperdocNode[]; | ||
| } | ||
|
|
||
| interface HyperspellDocument extends IDataObject { | ||
| summary?: string; | ||
| highlights?: Highlight[]; | ||
| document?: IDataObject; | ||
| document?: HyperdocNode; | ||
| } | ||
|
|
||
| // Result-array keys across the document-shaped responses: | ||
| // POST /memories/query → documents; GET /memories/list → items. | ||
| // POST /memories/query → documents; GET /memories/list → items; | ||
| // GET /live/{source}/resources → items. | ||
| const RESULT_ARRAY_KEYS = ['documents', 'items']; | ||
|
|
||
| /** The matched text, preferring the server's concatenation of the highlights. */ | ||
| // Only the QUERY path returns `ScoredDocumentResponse` (schemas.py:152), which | ||
| // is the sole model carrying `summary`/`highlights`. `/memories/list`, | ||
| // `/memories/get/*` and every `/live/*` route return the plain | ||
| // `DocumentResponse` (schemas.py:75) — no summary, no highlights, body only in | ||
| // the `document` tree. Simplifying those on the highlights-only path emitted | ||
| // `text: ''` and dropped the tree, i.e. deleted the content outright (shipped | ||
| // in 0.7.0; Document → List returned metadata-only rows). So the tree is the | ||
| // fallback source of text, flattened and capped rather than passed through. | ||
| // | ||
| // 2000 chars mirrors the API's own per-highlight cap, which keeps a simplified | ||
| // row roughly the size of one query hit no matter which endpoint produced it. | ||
| // Callers who need the whole body turn Simplify off. | ||
| export const MAX_SIMPLIFIED_TEXT = 2000; | ||
|
|
||
| /** Depth-first concatenation of a hyperdoc tree's `text` nodes, capped. */ | ||
| function flattenHyperdoc(node: HyperdocNode | undefined, budget: number): string { | ||
| if (node === undefined || node === null || budget <= 0) return ''; | ||
| const parts: string[] = []; | ||
| let remaining = budget; | ||
| const visit = (current: HyperdocNode | undefined): void => { | ||
| if (current === undefined || current === null || remaining <= 0) return; | ||
| if (typeof current.text === 'string' && current.text.length > 0) { | ||
| parts.push(current.text.slice(0, remaining)); | ||
| remaining -= Math.min(current.text.length, remaining); | ||
| } | ||
| const children = Array.isArray(current.children) ? current.children : []; | ||
| for (const child of children) visit(child); | ||
|
entelligence-ai-pr-reviews[bot] marked this conversation as resolved.
|
||
| }; | ||
| visit(node); | ||
| return parts.join('\n'); | ||
|
entelligence-ai-pr-reviews[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** | ||
| * The document's text: the server's highlight concatenation on the query path, | ||
| * otherwise the flattened hyperdoc tree. Never the raw tree, and never empty | ||
| * when the document has any text at all. | ||
| */ | ||
| function matchedText(document: HyperspellDocument): string { | ||
| if (typeof document.summary === 'string' && document.summary.length > 0) { | ||
| return document.summary; | ||
| } | ||
| const highlights = Array.isArray(document.highlights) ? document.highlights : []; | ||
| return highlights | ||
| const fromHighlights = highlights | ||
| .map((highlight) => highlight?.text) | ||
| .filter((text): text is string => typeof text === 'string' && text.length > 0) | ||
| .join('\n\n'); | ||
| if (fromHighlights.length > 0) return fromHighlights; | ||
| return flattenHyperdoc(document.document, MAX_SIMPLIFIED_TEXT); | ||
|
Comment on lines
151
to
+161
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Search and Answer still pass through summary verbatim or join every highlight without applying MAX_SIMPLIFIED_TEXT. Their callers in search/index.ts therefore can emit multi-kilobyte simplified rows, so the response bound only works for tree-fallback documents.
Comment on lines
151
to
+161
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
simplifyDocument returns summary or joined highlights without applying MAX_SIMPLIFIED_TEXT. Search's postReceive uses this helper for every query/answer hit, so a response with multiple 2,000-character highlights can still emit an unbounded row despite Simplify being advertised as the payload bound. Prompt to fix with AI
|
||
| } | ||
|
|
||
| function simplifyDocument(document: HyperspellDocument): IDataObject { | ||
| export function simplifyDocument(document: HyperspellDocument): IDataObject { | ||
| // Explicit allow-list, not a `delete document.document`: `summary` and | ||
| // `highlights[].text` carry the SAME text (summary is defined as their | ||
| // concatenation), so passing both through would ship every matched chunk | ||
|
|
@@ -117,3 +161,25 @@ export async function simplifyDocuments( | |
| return { ...item, json: { ...json, [key]: documents.map(simplifyDocument) } }; | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * postReceive for the single-document responses — `GET /memories/get/*` returns | ||
| * a bare `DocumentResponse`, not an array, so `simplifyDocuments` (which looks | ||
| * for a result-array key) passes it through untouched and the full tree reaches | ||
| * the caller. One document is exactly the case where the tree is largest per | ||
| * item, so this is the operation most worth bounding, not the least. | ||
| */ | ||
| export async function simplifyOne( | ||
| this: IExecuteSingleFunctions, | ||
| items: INodeExecutionData[], | ||
| ): Promise<INodeExecutionData[]> { | ||
| if (this.getNodeParameter('simplify', true) === false) return items; | ||
|
|
||
| return items.map((item) => { | ||
| const json = (item.json ?? {}) as HyperspellDocument; | ||
| // Only rewrite something that actually looks like a document response; | ||
| // an error body or a notice item must pass through untouched. | ||
| if (json.resource_id === undefined) return item; | ||
| return { ...item, json: simplifyDocument(json) }; | ||
| }); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.