Fix execution log URL mode schema exposure#343
Draft
cursor[bot] wants to merge 4 commits into
Draft
Conversation
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Contributor
Author
There was a problem hiding this comment.
No blocking findings.
Assumptions / residual risk:
params.return_download_urlstill appears to work via the genericparamsmerge path inharness_get, but the new regressions only cover the top-levelreturn_download_urlfield. I treated that as low risk because the architecture bug here was specifically the missing top-level schema field that strict MCP clients could strip before the handler ran.execution_logstill has some pre-existing metadata debt insrc/registry/toolsets/logs.ts(responseExtractor: passthroughandlistFilterFieldson a get-only resource), but this PR does not worsen that surface.
Change summary:
harness_getnow advertises top-levelreturn_download_url, restoring public schema/runtime parity forexecution_log.- The fix removes the silent fallback where schema-driven clients could lose URL-only mode and drop back to downloading/decompressing log content.
- Added the right shared-tool regressions for this bug: one proves the field is exposed in the registered schema, and one proves URL mode calls
resolveLogDownloadUrlinstead ofresolveLogContent. - PR head
21cd023d61b05ad448c6f15655c0b010626254b2matched the reviewed diff, and CI was green at review time.
Sent by Cursor Automation: Sunil On Demand Architecture Review
|
|
1 similar comment
|
|
Saranya-jena
approved these changes
Jun 15, 2026
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
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.


Description
Exposes
return_download_urlon the publicharness_getinput schema so schema-driven MCP clients can request URL-only execution log retrieval without the field being dropped before handler dispatch.Bug and impact:
execution_logdocumented and handledreturn_download_url=true, but the registered top-level schema omitted it. Strict/schema-driven clients could strip the unknown field, silently falling back to downloading and decompressing log content instead of returning a signed URL. That reintroduces the large-log buffering path that URL mode was meant to avoid.Root cause: the handler supported
input.return_download_url, but the public schema only had genericparamsfor resource-specific fields.Fix: add top-level
return_download_urltoharness_getand add tool-handler regressions for schema exposure and URL-mode dispatch.Type of Change
Checklist
Validation performed:
pnpm exec vitest run tests/tools/tool-handlers.test.ts -t "return_download_url"pnpm buildpnpm docs:generatepnpm typecheckpnpm exec vitest run tests/tools/tool-handlers.test.tspnpm testpnpm docs:check