feat: add upload_image MCP tool and screenshot support#3574
Open
Anty0 wants to merge 13 commits into
Open
Conversation
Add a new `upload_image` MCP tool that accepts base64-encoded images and returns an image ID. Enhance `create_keys` to optionally accept screenshot references (uploadedImageId + positions) per key, associating uploaded images with keys after creation. Also rewrite webapp/CLAUDE.md to use MCP tools instead of curl, clarify that defaultValue is temporary (for screenshots only), make screenshots mandatory (opt-out), and add app startup/shutdown instructions.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds MCP screenshot capabilities: new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as MCP Client
participant UploadTool as upload_image Tool
participant ImageService as ImageUploadService
participant CreateKeysTool as create_keys Tool
participant KeyService as KeyService
participant ScreenshotService as ScreenshotService
Client->>UploadTool: upload_image(base64)
UploadTool->>UploadTool: validate size & decode
UploadTool->>ImageService: store(imageBytes)
ImageService-->>UploadTool: imageId
UploadTool-->>Client: { "imageId": id }
Client->>CreateKeysTool: create_keys(keys with screenshots)
CreateKeysTool->>KeyService: importKeys(keyDtos)
KeyService-->>CreateKeysTool: created key list
loop per key with screenshots
CreateKeysTool->>KeyService: find(resolvedKey)
KeyService-->>CreateKeysTool: keyEntity
CreateKeysTool->>ScreenshotService: saveUploadedImagesForKeys((keyEntity, screenshotDtos))
ScreenshotService-->>CreateKeysTool: associationsCreated
end
CreateKeysTool-->>Client: { "created": true, "keyCount": n }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Add a dedicated tool for associating screenshots with keys that already exist, complementing the create_keys screenshot support which only works during key creation. Also mention the new tool in webapp/CLAUDE.md.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/app/src/test/kotlin/io/tolgee/mcp/tools/McpScreenshotToolsTest.kt (1)
166-205: Consider verifying that position data is actually persisted.The test confirms that
create_keyswith positions doesn't fail and screenshots are associated, but it doesn't verify that the position coordinates (x=10, y=20, width=100, height=30) are actually stored. This could mask a bug where positions are silently dropped.💡 Suggested: Add position verification
val key = keyService.find(data.projectId, "positioned.key", null) assertThat(key).isNotNull() val screenshots = screenshotService.findAll(key!!) assertThat(screenshots).isNotEmpty() + + // Verify position data was stored + val keyInScreenshots = screenshots.first().keyScreenshotReferences + assertThat(keyInScreenshots).isNotEmpty() + val position = keyInScreenshots.first() + assertThat(position.x).isEqualTo(10) + assertThat(position.y).isEqualTo(20) + assertThat(position.width).isEqualTo(100) + assertThat(position.height).isEqualTo(30)Note: Adjust field access based on the actual entity structure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/src/test/kotlin/io/tolgee/mcp/tools/McpScreenshotToolsTest.kt` around lines 166 - 205, The test create_keys with screenshots and positions stores position data currently only checks screenshots exist but not that the position coordinates were persisted; update the test (McpScreenshotToolsTest::`create_keys with screenshots and positions stores position data`) to fetch positions from the persisted screenshot(s) (via screenshotService.findAll(key) and the screenshot entity's positions collection or a Position/KeyPosition service) and assert that there is one position with x==10, y==20, width==100 and height==30 (use the actual entity getters or fields present in your domain model to read the stored values).backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.kt (1)
164-192: Consider logging when a key is not found during screenshot association.The
continueon line 173 silently skips screenshot association if the key is not found. While this may be intentional (e.g., for keys that existed before and were skipped byimportKeys), silent failures can make debugging difficult. Consider adding a log statement when a key is unexpectedly not found.💡 Optional: Add debug logging
val keyEntity = keyService.find(projectHolder.project.id, keyName, keyNamespace, branch) - ?: continue + ?: run { + // Key may have been skipped if it already existed + continue + }Or add a logger call if the class has a logger configured.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.kt` around lines 164 - 192, In KeyMcpTools, when iterating keysWithScreenshots inside the executeInNewTransaction block, add a log statement before the existing continue where keyService.find(...) returns null: use the class logger (e.g., logger.debug or logger.warn) to record keyName, keyNamespace (or defaultNamespace), branch and projectHolder.project.id so missing-key cases are visible; keep the continue behavior but ensure the message clearly indicates screenshots were skipped for that key to aid debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.kt`:
- Around line 164-192: In KeyMcpTools, when iterating keysWithScreenshots inside
the executeInNewTransaction block, add a log statement before the existing
continue where keyService.find(...) returns null: use the class logger (e.g.,
logger.debug or logger.warn) to record keyName, keyNamespace (or
defaultNamespace), branch and projectHolder.project.id so missing-key cases are
visible; keep the continue behavior but ensure the message clearly indicates
screenshots were skipped for that key to aid debugging.
In `@backend/app/src/test/kotlin/io/tolgee/mcp/tools/McpScreenshotToolsTest.kt`:
- Around line 166-205: The test create_keys with screenshots and positions
stores position data currently only checks screenshots exist but not that the
position coordinates were persisted; update the test
(McpScreenshotToolsTest::`create_keys with screenshots and positions stores
position data`) to fetch positions from the persisted screenshot(s) (via
screenshotService.findAll(key) and the screenshot entity's positions collection
or a Position/KeyPosition service) and assert that there is one position with
x==10, y==20, width==100 and height==30 (use the actual entity getters or fields
present in your domain model to read the stored values).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90b02b5a-8e9f-4d3e-b334-f25b16c5649a
📒 Files selected for processing (5)
backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.ktbackend/app/src/main/kotlin/io/tolgee/mcp/tools/McpToolUtils.ktbackend/app/src/main/kotlin/io/tolgee/mcp/tools/ScreenshotMcpTools.ktbackend/app/src/test/kotlin/io/tolgee/mcp/tools/McpScreenshotToolsTest.ktwebapp/CLAUDE.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/app/src/test/kotlin/io/tolgee/mcp/tools/McpScreenshotToolsTest.kt (1)
166-205: Test doesn't verify position data is actually stored.The test name says "stores position data" but only asserts
screenshots.isNotEmpty(). Consider verifying the actual position values were persisted.♻️ Suggested enhancement
val key = keyService.find(data.projectId, "positioned.key", null) assertThat(key).isNotNull() val screenshots = screenshotService.findAll(key!!) assertThat(screenshots).isNotEmpty() + + // Verify position data was stored + val keyReferences = screenshots.first().keyScreenshotReferences + assertThat(keyReferences).isNotEmpty() + val position = keyReferences.first() + assertThat(position.x).isEqualTo(10) + assertThat(position.y).isEqualTo(20) + assertThat(position.width).isEqualTo(100) + assertThat(position.height).isEqualTo(30)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/src/test/kotlin/io/tolgee/mcp/tools/McpScreenshotToolsTest.kt` around lines 166 - 205, The test create_keys with screenshots and positions stores position data currently only checks screenshots are present; extend it to fetch the persisted Screenshot (via screenshotService.findAll(key) or screenshotService.findById) and assert the stored position fields match the input (x=10, y=20, width=100, height=30). Use the existing keyService.find(...) result to get the screenshots list, extract the position object (e.g., screenshot.positions or screenshot.position), and add assertions that each numeric field equals the expected values so the test verifies the actual persisted position data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/src/main/kotlin/io/tolgee/mcp/tools/ScreenshotMcpTools.kt`:
- Around line 117-148: The loop inside executeInNewTransaction currently returns
early with errorResult when keyService.find fails, which exits the lambda
without rolling back after some screenshots have been saved; to fix, first
pre-validate all keys before any mutations by iterating keyScreenshots and
calling keyService.find(projectHolder.project.id, keyName, keyNamespace, branch)
for each, collecting missing keys and returning errorResult if any are absent,
then in a second pass map screenshots and call
screenshotService.saveUploadedImages only after validation succeeds;
alternatively, replace the early return with throwing a runtime exception (e.g.,
IllegalStateException) instead of return@executeInNewTransaction so the
transaction is rolled back—apply this change to the code handling
keyScreenshots, keyService.find, and screenshotService.saveUploadedImages.
In `@webapp/CLAUDE.md`:
- Around line 100-103: The README currently shows the macOS-only base64 usage
with the -i flag; update the CLAUDE.md snippet to use portable base64 invocation
by removing the macOS-specific -i flag and instead demonstrate the two
cross-platform options: passing the filename as an argument or piping the file
into base64 via stdin, and explicitly note that -i is macOS-only while Linux
accepts the filename or stdin approach; adjust the surrounding text to document
both variants so readers on different platforms can pick the correct form.
---
Nitpick comments:
In `@backend/app/src/test/kotlin/io/tolgee/mcp/tools/McpScreenshotToolsTest.kt`:
- Around line 166-205: The test create_keys with screenshots and positions
stores position data currently only checks screenshots are present; extend it to
fetch the persisted Screenshot (via screenshotService.findAll(key) or
screenshotService.findById) and assert the stored position fields match the
input (x=10, y=20, width=100, height=30). Use the existing keyService.find(...)
result to get the screenshots list, extract the position object (e.g.,
screenshot.positions or screenshot.position), and add assertions that each
numeric field equals the expected values so the test verifies the actual
persisted position data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: daf3de7a-545d-4ab8-936d-500d42171b8f
📒 Files selected for processing (3)
backend/app/src/main/kotlin/io/tolgee/mcp/tools/ScreenshotMcpTools.ktbackend/app/src/test/kotlin/io/tolgee/mcp/tools/McpScreenshotToolsTest.ktwebapp/CLAUDE.md
The hand-crafted PNG bytes had incorrect zlib compression, causing "Error skipping PNG metadata" in image processing. Replace with a properly generated 1x1 PNG base64 constant.
bdshadow
self-requested a review
March 27, 2026 16:38
saveUploadedImages deletes the uploaded image after converting it to a screenshot. When the same image is referenced by multiple keys, track already-processed images and use addReference for subsequent keys instead of calling saveUploadedImages again.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/app/src/main/kotlin/io/tolgee/mcp/tools/ScreenshotMcpTools.kt (1)
120-128:⚠️ Potential issue | 🟠 Major
errorResulthere does not roll back earlier screenshot writes.
return@executeInNewTransactionexits the lambda normally, so any screenshots saved for previous entries can still commit before the error is returned. Pre-resolve all keys before the mutation loop, or throw so the transaction aborts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/src/main/kotlin/io/tolgee/mcp/tools/ScreenshotMcpTools.kt` around lines 120 - 128, The current executeInNewTransaction lambda returns early with return@executeInNewTransaction when keyService.find fails, which does not abort the transaction and allows prior screenshot writes to commit; change the flow to pre-resolve all keys before performing any mutations (iterate keyScreenshots first, call keyService.find for each and collect resolved Key entities or stop), or instead throw an exception (e.g., IllegalStateException) when a key is missing so the transaction will roll back; ensure the code paths referencing keyScreenshots, keyService.find, executeInNewTransaction, and errorResult are updated so mutations only happen after successful pre-resolution or use exceptions to abort the transaction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.kt`:
- Around line 165-217: The two-step flow is non-atomic:
keyService.importKeys(...) runs before the screenshot pass and re-resolves keys
which can cause duplicate/incorrect attachments on retry; fix by performing
importKeys and the screenshot association inside the same transaction (use
executeInNewTransaction(transactionManager) to wrap both) and by only processing
keys that were created by this import call (capture the import result / created
key IDs returned by keyService.importKeys and use that set to filter
rawKeys/screenshots instead of re-resolving by name). Ensure you call
screenshotService.saveUploadedImages/addReference only for keyEntity objects
belonging to the created-key set and keep the processedImages memoization logic
as-is to avoid double-conversion.
- Around line 167-217: The importKeys endpoint currently allows screenshot
uploads because KeyMcpTools calls screenshotService.saveUploadedImages() without
checking SCREENSHOTS_UPLOAD; update the permission model by adding
Scope.SCREENSHOTS_UPLOAD to the `@RequiresProjectPermissions` annotation on
KeyController.importKeys or, alternatively, add an explicit permission check in
KeyMcpTools before processing screenshots (check project permissions for
Scope.SCREENSHOTS_UPLOAD and abort/throw if missing) so only users with
SCREENSHOTS_UPLOAD can call
screenshotService.saveUploadedImages()/screenshotService.addReference.
---
Duplicate comments:
In `@backend/app/src/main/kotlin/io/tolgee/mcp/tools/ScreenshotMcpTools.kt`:
- Around line 120-128: The current executeInNewTransaction lambda returns early
with return@executeInNewTransaction when keyService.find fails, which does not
abort the transaction and allows prior screenshot writes to commit; change the
flow to pre-resolve all keys before performing any mutations (iterate
keyScreenshots first, call keyService.find for each and collect resolved Key
entities or stop), or instead throw an exception (e.g., IllegalStateException)
when a key is missing so the transaction will roll back; ensure the code paths
referencing keyScreenshots, keyService.find, executeInNewTransaction, and
errorResult are updated so mutations only happen after successful pre-resolution
or use exceptions to abort the transaction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 78604f2f-ad85-475c-803b-c26fb6847cf3
📒 Files selected for processing (2)
backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.ktbackend/app/src/main/kotlin/io/tolgee/mcp/tools/ScreenshotMcpTools.kt
Move the multi-key screenshot association logic into a single ScreenshotService.saveUploadedImagesForKeys() method. This eliminates duplicated image-tracking logic in both KeyMcpTools and ScreenshotMcpTools — both now call the shared service method. Also extract parseScreenshotDtos() helper for MCP argument parsing.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.kt (1)
160-176:⚠️ Potential issue | 🟠 Major
create_keysno longer honors the "skip existing" contract for screenshots.After
importKeys(), the second pass re-resolves keys by name and attaches screenshots even when the key already existed and was skipped. Because that pass runs separately, a later screenshot failure also leaves the key creation committed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.kt` around lines 160 - 176, The second-pass screenshot attach step re-resolves keys and attaches screenshots even for keys that were skipped by keyService.importKeys(), breaking the "skip existing" contract and leaving commits if screenshot save fails; fix by making the screenshot pass only operate on keys actually created in the import (or by performing screenshot attachment inside the same import transaction). Concretely: change keyService.importKeys(...) to return the identifiers or KeyEntity instances of keys it created (or add a flag per key), then in the screenshot block filter rawKeys against that returned set instead of re-resolving by name (use the returned created key IDs/objects to build keyScreenshotPairs for parseScreenshotDtos and pass those to screenshotService.saveUploadedImagesForKeys), or move parseScreenshotDtos + saveUploadedImagesForKeys into importKeys so the whole operation is atomic under executeInNewTransaction/transactionManager.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/src/main/kotlin/io/tolgee/mcp/tools/ScreenshotMcpTools.kt`:
- Around line 117-126: The code builds keyScreenshotPairs using
keyService.find(...) and then saves screenshots without enforcing branch write
permissions; call securityService.checkBranchModify(keyEntity) for each resolved
key inside the same executeInNewTransaction block (before adding to
keyScreenshotPairs and before calling
screenshotService.saveUploadedImagesForKeys) and return the appropriate
errorResult if the check fails so that
KeyScreenshotController.uploadScreenshot's branch-level authorization is
reapplied; reference the functions/variables keyService.find,
securityService.checkBranchModify, executeInNewTransaction, keyScreenshotPairs,
and screenshotService.saveUploadedImagesForKeys when making the change.
In `@backend/data/src/main/kotlin/io/tolgee/service/key/ScreenshotService.kt`:
- Around line 245-273: The code currently converts and deletes uploaded images
via saveScreenshot(image) before verifying that all requested uploadedImageIds
exist and that each key will not exceed maxScreenshotsPerKey; change
saveUploadedImagesForKeys to first validate: (1) collect allImageIds from
keyScreenshots and ensure imageUploadService.find(allImageIds) returns an image
for every id (throw NotFoundException if any missing), and verify ownership of
each image (existing check) before any saveScreenshot calls; (2) for each Key in
keyScreenshots compute the number of distinct incoming screenshot IDs for that
key and fetch the current screenshot count for the key (use the repository or
existing count method used by store()) and throw a BadRequest/Quota exception if
current + incoming would exceed maxScreenshotsPerKey; only after these
validations proceed to call saveScreenshot for each image and then
addReference/store as before.
---
Duplicate comments:
In `@backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.kt`:
- Around line 160-176: The second-pass screenshot attach step re-resolves keys
and attaches screenshots even for keys that were skipped by
keyService.importKeys(), breaking the "skip existing" contract and leaving
commits if screenshot save fails; fix by making the screenshot pass only operate
on keys actually created in the import (or by performing screenshot attachment
inside the same import transaction). Concretely: change
keyService.importKeys(...) to return the identifiers or KeyEntity instances of
keys it created (or add a flag per key), then in the screenshot block filter
rawKeys against that returned set instead of re-resolving by name (use the
returned created key IDs/objects to build keyScreenshotPairs for
parseScreenshotDtos and pass those to
screenshotService.saveUploadedImagesForKeys), or move parseScreenshotDtos +
saveUploadedImagesForKeys into importKeys so the whole operation is atomic under
executeInNewTransaction/transactionManager.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 99fd5a2e-21b7-4f2b-9ea6-c50bf9f54dd9
📒 Files selected for processing (3)
backend/app/src/main/kotlin/io/tolgee/mcp/tools/KeyMcpTools.ktbackend/app/src/main/kotlin/io/tolgee/mcp/tools/ScreenshotMcpTools.ktbackend/data/src/main/kotlin/io/tolgee/service/key/ScreenshotService.kt
Review fixes:
- Add upload_image and add_key_screenshots to McpWithoutEeTest.coreTools
- Strengthen position assertion in McpScreenshotToolsTest
Boy scout improvements in ScreenshotService:
- Fix typo: adjustByRation → adjustByRatio
- Simplify forEach { delete(it) } → forEach(::delete)
- Fix shadowed 'it' variable in getScreenshotsForKeys
- Replace .toSet().toList() with .distinctBy { it.id }
- Remove unnecessary .let wrapper in saveUploadedImages
Boy scout improvement in McpToolUtils:
- Replace unused 'exchange' parameter with '_'
- Add SCREENSHOTS_UPLOAD scope check in create_keys before screenshot association (prevents scope escalation via PAK with only key/translation permissions) - Add eager validation of image IDs in saveUploadedImagesForKeys (fail fast before any mutations if images don't exist) - Add maxScreenshotsPerKey limit check in saveUploadedImagesForKeys (matching the existing check in store()) - Use portable base64 syntax in webapp/CLAUDE.md (base64 < file instead of macOS-specific base64 -i) - Add clarifying comment in add_key_screenshots key resolution
Member
|
As we discussed. Tracked as #3620 — MCP for better development with AI agents. 1. 2. Leave 3. Base64 upload isn't the right shape 4. Add a dedicated AI context field on keys 5. Public skill + video |
Contributor
|
This PR is stale because it has been open for 30 days with no activity. |
Contributor
|
This PR was closed because it has been inactive for 14 days since being marked as stale. |
…load Lets AI agents upload screenshot bytes out-of-band via a short-lived, signed, unauthenticated URL instead of base64-encoding them through the model. The agent POSTs the image to /v2/public/image-upload?token=<jwt> and gets back an uploadedImageId for create_keys / add_key_screenshots. - new get_image_upload_url tool + McpImageUploadUrlService + BackendUrlProvider - new public POST /v2/public/image-upload, authorized solely by the signed IMG_UPLOAD ticket (reusable within a 30-min, configurable window) - tolgee.mcp.image-upload-url-expiration-ms property (default 30 minutes) - deprecate base64 upload_image; rename its output key imageId -> uploadedImageId - map ImageIO decode failures, degenerate image dimensions, and unknown JWT ticket types to a clean 4xx instead of a 500 (also covers the existing authenticated image-upload and avatar paths)
- create_keys: check SCREENSHOTS_UPLOAD before creation and run key import + screenshot association in a single transaction so a screenshot failure rolls back the keys too (was non-atomic, leaving orphan keys) - normalize blank namespace via getSafeNamespace before the post-create re-lookup so create_keys/add_key_screenshots no longer spuriously fail and roll back a valid batch - validate the per-key screenshot limit before any image is converted, so an over-limit batch can't leave orphaned screenshot files on rollback - merge positions for a repeated (key, image) instead of silently dropping the extra DTO's positions - use canonical SecurityService.checkScreenshotsUploadPermission - move parseScreenshotDtos and a shared screenshotsField schema into McpToolUtils - add coverage: cross-user IDOR, scope enforcement, per-key limit, dedup/merge, exact positions, base64 size cap, namespace round-trip
Lets AI agents attach arbitrary structured metadata to a key in the same create_keys call instead of a follow-up update. - add nullable `custom` (Map<String, Any?>) to ImportKeysItemDto (backs both the MCP create_keys tool and the REST POST /keys/import endpoint) - KeysImporter persists keyMeta.custom for newly-created keys, validated with the same 5000-char-JSON cap as the complex-edit path; existing keys are still skipped (their metadata is not updated) - MCP: getObjectMap helper, SchemaBuilder.objectField, create_keys schema and DTO wiring; tool description clarifies description/custom are not updated for existing keys - regenerate frontend API schema
Remove the "Start the App" / "Stop the App" subsections from webapp/CLAUDE.md — the developer manages app lifecycle manually and the screenshot docs should not prescribe start/stop commands. The screenshot workflow (get positions, upload via get_image_upload_url) is kept, and the fallback note now states the running-app prerequisite without prescribing how.
Lets AI agents attach full key context in a single create_keys call, so the
auto-translation that runs right after creation has it (it is not redone if
context is added later).
- add per-key `comments` (List<String>) and `codeReferences` ({path, line}) to
ImportKeysItemDto — flows through both the MCP create_keys tool and the REST
POST /keys/import endpoint; persisted on KeyMeta for newly-created keys
- add a call-level `relatedKeysInOrder` param to the MCP create_keys tool that
stores big meta (key proximity) in the same transaction, so it lands before
the post-commit auto-translate batch and feeds the AI translation prompt
- centralize comments/codeReferences validation in KeysImporter (shared by REST
and MCP): blank entries are skipped, over-long ones rejected with
KEY_COMMENT_TOO_LONG / KEY_CODE_REFERENCE_PATH_TOO_LONG; limits owned by the
entities (KeyComment.TEXT_MAX_LENGTH, KeyCodeReference.PATH_MAX_LENGTH)
- share related-key parsing (parseRelatedKeysInOrder) with store_big_meta and
apply the call's default namespace to related keys
- gate relatedKeysInOrder behind the canonical checkBigMetaUploadPermission
- regenerate frontend API schema
The create_keys and add_key_screenshots MCP tools attached screenshots after only resolving the key, bypassing the branch-write protection that KeyScreenshotController enforces via securityService.checkBranchModify. A user with SCREENSHOTS_UPLOAD but without BRANCH_PROTECTED_MODIFY could write screenshots to keys on a protected branch through these tools. Reapply checkBranchModify in both MCP paths, add regression tests, and hoist the shared MCP test helpers to AbstractMcpTest.
Contributor
|
This PR is stale because it has been open for 30 days with no activity. |
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
2 participants
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
get_image_upload_urlMCP tool that returns a short-lived, signed, unauthenticated upload URL, so AI agents upload screenshot bytes out-of-band (e.g.curl -F image=@file "<uploadUrl>") instead of base64-encoding them through the model. The response carries anuploadedImageIdto pass tocreate_keys/add_key_screenshots.POST /v2/public/image-upload?token=<jwt>endpoint, authorized solely by the signedIMG_UPLOADticket (no API key needed). The URL is reusable within its expiry window — default 30 min, configurable viatolgee.mcp.image-upload-url-expiration-ms.upload_imagetool (kept for backward-compat) and rename its JSON output keyimageId→uploadedImageIdso both upload paths use one spelling matching the consumer field.create_keysaccepts per-keyscreenshots(uploadedImageId+ optional positions); addadd_key_screenshotsfor existing keys; persist associations viaScreenshotService.saveUploadedImagesForKeys. Screenshot association is atomic with key creation (one transaction; theSCREENSHOTS_UPLOADpermission is checked before any key is created), and the per-key screenshot limit is enforced before image conversion so an over-limit batch leaves no orphaned files.create_keys/ import call, so the auto-translation that runs right after creation has full context (it is not redone if metadata is added later):description(already fed the translation prompt),custom(arbitrary structured metadata),comments, andcodeReferences({path, line}) —custom/comments/codeReferencesalso flow through the RESTPOST /keys/importendpoint via the sharedImportKeysItemDto.relatedKeysInOrderparam that stores big meta (key proximity) in the same transaction, so it lands before the post-commit auto-translate batch and feeds the AI translation prompt. Validated metadata limits are owned by the entities; blank entries are skipped and over-long ones rejected with a clean 4xx./v2/image-uploadand avatar paths.webapp/CLAUDE.mdto the URL-based screenshot workflow and remove the app start/stop lifecycle instructions (the developer manages the app lifecycle manually); regenerate the frontend API schema.tolgee.back-end-urlin the deployed configuration. When it's unset, the issued upload URL falls back to the current servlet-request origin.Notes
ImageIOdecompression-bomb risk (full decode before the 3M-px clamp) is now reachable on the unauthenticated route; a pre-decode pixel cap is deferred to its own cross-cutting change.