-
Notifications
You must be signed in to change notification settings - Fork 66
Improve remote site handling: prefetch plan and truncate large API responses #3011
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
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,60 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { stripNoisyFields } from '../wpcom-tools'; | ||
|
|
||
| describe( 'stripNoisyFields', () => { | ||
| it( 'removes _links and other noisy top-level fields', () => { | ||
| const input = { | ||
| id: 1, | ||
| title: 'Hello', | ||
| _links: { self: '/posts/1' }, | ||
| _embedded: { author: [] }, | ||
| guid: { rendered: 'http://example.com/?p=1' }, | ||
| ping_status: 'open', | ||
| comment_status: 'open', | ||
| generated_slug: 'hello', | ||
| permalink_template: 'http://example.com/%postname%/', | ||
| }; | ||
| const { data, removed } = stripNoisyFields( input ); | ||
| expect( data ).toEqual( { id: 1, title: 'Hello' } ); | ||
| expect( removed ).toContain( '_links' ); | ||
| expect( removed ).toContain( 'guid' ); | ||
| expect( removed ).toContain( 'permalink_template' ); | ||
| } ); | ||
|
|
||
| it( 'drops rendered when raw is present in the same object', () => { | ||
| const input = { | ||
| title: { raw: 'Hello', rendered: '<p>Hello</p>' }, | ||
| content: { raw: '<!-- wp:paragraph -->', rendered: '<p>rendered html</p>' }, | ||
| }; | ||
| const { data, removed } = stripNoisyFields( input ); | ||
| expect( data ).toEqual( { | ||
| title: { raw: 'Hello' }, | ||
| content: { raw: '<!-- wp:paragraph -->' }, | ||
| } ); | ||
| expect( removed ).toContain( 'rendered (raw exists)' ); | ||
| } ); | ||
|
|
||
| it( 'keeps rendered when raw is absent', () => { | ||
| const input = { title: { rendered: 'Hello' } }; | ||
| const { data } = stripNoisyFields( input ); | ||
| expect( data ).toEqual( { title: { rendered: 'Hello' } } ); | ||
| } ); | ||
|
|
||
| it( 'recursively strips from arrays', () => { | ||
| const input = [ | ||
| { id: 1, _links: { self: '/1' }, title: { raw: 'A', rendered: '<p>A</p>' } }, | ||
| { id: 2, _links: { self: '/2' }, title: { raw: 'B', rendered: '<p>B</p>' } }, | ||
| ]; | ||
| const { data } = stripNoisyFields( input ); | ||
| expect( data ).toEqual( [ | ||
| { id: 1, title: { raw: 'A' } }, | ||
| { id: 2, title: { raw: 'B' } }, | ||
| ] ); | ||
| } ); | ||
|
|
||
| it( 'passes through primitives unchanged', () => { | ||
| expect( stripNoisyFields( 42 ).data ).toBe( 42 ); | ||
| expect( stripNoisyFields( 'hello' ).data ).toBe( 'hello' ); | ||
| expect( stripNoisyFields( null ).data ).toBe( null ); | ||
| } ); | ||
| } ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,53 @@ function getErrorMessage( error: unknown ): string { | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| type ApiResponse = any; | ||
|
|
||
| export { stripNoisyFields }; | ||
|
|
||
| // Fields that bloat API responses without being useful to the agent. | ||
| const NOISY_FIELDS = new Set( [ | ||
| '_links', | ||
| '_embedded', | ||
| 'guid', | ||
| 'ping_status', | ||
| 'comment_status', | ||
| 'generated_slug', | ||
| 'permalink_template', | ||
| ] ); | ||
|
Contributor
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. There are situations where we need these fields, this is a bit dangerous IMO. Like if we want to retrieve the global styles we need the links... It's also a bit random, we seem to be stripping random fields from all requests.
Contributor
Author
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. The idea is to strip them only when results exceed the limit, and note which ones were stripped and why in the reply so the agent is aware and can ask again with some filtering if it needs the full response.
Contributor
Author
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.
|
||
|
|
||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
| function stripNoisyFields( | ||
| data: any, | ||
| removed = new Set< string >() | ||
| ): { data: any; removed: Set< string > } { | ||
| return { data: doStrip( data, removed ), removed }; | ||
| } | ||
| /* eslint-enable @typescript-eslint/no-explicit-any */ | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| function doStrip( data: any, removed: Set< string > ): any { | ||
| if ( Array.isArray( data ) ) { | ||
| return data.map( ( item ) => doStrip( item, removed ) ); | ||
| } | ||
| if ( data !== null && typeof data === 'object' ) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const out: Record< string, any > = {}; | ||
| for ( const [ key, value ] of Object.entries( data ) ) { | ||
| if ( NOISY_FIELDS.has( key ) ) { | ||
| removed.add( key ); | ||
| continue; | ||
| } | ||
| // Drop `rendered` when `raw` is present in the same object | ||
| if ( key === 'rendered' && 'raw' in data ) { | ||
| removed.add( 'rendered (raw exists)' ); | ||
| continue; | ||
| } | ||
| out[ key ] = doStrip( value, removed ); | ||
| } | ||
| return out; | ||
| } | ||
| return data; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a generic WP.com REST API tool for managing a remote WordPress.com site. | ||
| * Instead of hardcoding individual endpoints, this provides a single flexible tool | ||
|
|
@@ -41,7 +88,9 @@ export function createWpcomToolDefinitions( token: string, siteId: number ) { | |
| 'Defaults to the WordPress REST API (wp/v2). Use this to manage posts, pages, templates, template parts, ' + | ||
| 'media, plugins, themes, settings, and any other site resource. ' + | ||
| 'The path is relative to /sites/{siteId}/ — for example, pass "/posts" to call /wp/v2/sites/{siteId}/posts. ' + | ||
| 'For non-site endpoints, start the path with "!" (e.g., "!/me") to use an absolute path.', | ||
| 'For non-site endpoints, start the path with "!" (e.g., "!/me") to use an absolute path. ' + | ||
| 'IMPORTANT: To avoid oversized responses, always use the "fields" query parameter to request only the fields you need ' + | ||
| '(e.g., query: { "fields": "ID,title,slug,status" }). Use "per_page" to limit list results (default: 10, max: 100).', | ||
| { | ||
| method: z | ||
| .enum( [ 'GET', 'POST', 'PUT', 'DELETE' ] ) | ||
|
|
@@ -106,7 +155,36 @@ export function createWpcomToolDefinitions( token: string, siteId: number ) { | |
| break; | ||
| } | ||
|
|
||
| return textResult( JSON.stringify( result, null, 2 ) ); | ||
| // The Claude Agent SDK limits MCP tool results to ~25K tokens | ||
| // (MAX_MCP_OUTPUT_TOKENS, default 25000). At ~3 chars/token | ||
| // for JSON, 50K chars ≈ 16.7K tokens. | ||
| const MAX_RESPONSE_CHARS = 50000; | ||
| const json = JSON.stringify( result ); | ||
|
|
||
| if ( json.length <= MAX_RESPONSE_CHARS ) { | ||
| return textResult( json ); | ||
| } | ||
|
|
||
| // Over limit — strip noisy fields and retry | ||
| const { data: cleaned, removed } = stripNoisyFields( result ); | ||
| const stripped = JSON.stringify( cleaned ); | ||
| const removedList = [ ...removed ].join( ', ' ); | ||
| const fieldsHint = | ||
| `\n\n[Stripped fields: ${ removedList }. ` + | ||
| 'Re-request with "fields" parameter if you need any of these, or reduce "per_page".]'; | ||
|
|
||
| if ( stripped.length + fieldsHint.length <= MAX_RESPONSE_CHARS ) { | ||
| return textResult( stripped + fieldsHint ); | ||
| } | ||
|
|
||
| // Still too large — truncate | ||
| const truncationNote = | ||
| `\n\n[Truncated — response was ${ stripped.length.toLocaleString() } chars. ` + | ||
| `Stripped fields: ${ removedList }. ` + | ||
| 'Use "fields" to request only needed fields (e.g., { "fields": "ID,title,slug" }), or reduce "per_page".]'; | ||
| return textResult( | ||
| stripped.slice( 0, MAX_RESPONSE_CHARS - truncationNote.length ) + truncationNote | ||
| ); | ||
| } catch ( error ) { | ||
| return errorResult( | ||
| `WP.com API request failed (${ args.method } ${ args.path }): ${ getErrorMessage( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there cases where the plan is undefined and we need a fallback (I don't know wp.com enough)