Conversation
Preview:
|
52d1d8e to
91b665b
Compare
91b665b to
0e81ac3
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| collectionDocuments.push({ | ||
| path: location.path, | ||
| name: "SKILL.md", | ||
| description: candidate.description.trim(), | ||
| contentType: "text/markdown", | ||
| lastUpdated: new Date(), | ||
| }); |
There was a problem hiding this comment.
🟡 Failed uploads reserve unused names
When createContextSkill fails, its path remains reserved in collectionDocuments. Later same-name candidates receive unnecessary suffixes despite the free destination.
Learn more
The local document list prevents candidates in one batch from choosing the same directory. It is updated before the server confirms creation, so a rejected candidate still occupies its computed directory for every later candidate in that batch.
Example: Two selected files both resolve to deployment-check. The first exceeds the server's document-size limit and fails. The second succeeds as deployment-check-2, although deployment-check was never created.
Recommended fix: Add the synthetic summary to collectionDocuments only after createContextSkill succeeds. Keep existing server conflict handling for destinations occupied by stale or concurrent data.
Was this helpful? React with 👍 or 👎 to provide feedback.
| try { | ||
| await context.createContextSkill(collectionId, location.path, { | ||
| description: candidate.description.trim(), | ||
| body: writeSkillUploadMetadata( | ||
| candidate.manifestBody, | ||
| location.name, | ||
| candidate.description, | ||
| ), | ||
| contentType: "text/markdown", | ||
| }); | ||
| } catch (error) { | ||
| nextFailures.push(`${candidate.label}: ${error instanceof Error ? error.message : "upload failed"}`); | ||
| continue; | ||
| } | ||
| created++; | ||
|
|
||
| const supportResults: PromiseSettledResult<void>[] = []; | ||
| for (let index = 0; index < candidate.supportingFiles.length; index += 6) { | ||
| const batch = candidate.supportingFiles.slice(index, index + 6); | ||
| supportResults.push(...await Promise.allSettled(batch.map((file) => ( | ||
| context.putContextDocument(collectionId, `${location.directory}/${file.path}`, { | ||
| description: extractDescription(file.contentType, file.body) ?? "", | ||
| body: file.body, | ||
| contentType: file.contentType, | ||
| }) | ||
| )))); |
There was a problem hiding this comment.
🔍 Bundle upload remains non-atomic
Manifest creation and support writes use separate RPCs. The documented partial-upload state remains until a transactional bundle RPC replaces this sequence.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@FBalint Bonk workflow failed. Check the logs for details. View workflow run · To retry, trigger Bonk again. |
|
@maxwellpeterson Bonk workflow failed. Check the logs for details. View workflow run · To retry, trigger Bonk again. |
706f6fe to
e4f866b
Compare
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
| ): string => { | ||
| const { frontmatter, content } = splitFrontmatter(body); | ||
| let document = frontmatter === null ? new Document({}) : parseDocument(frontmatter); | ||
| if (document.errors.length > 0 || !isMap(document.contents)) document = new Document({}); |
There was a problem hiding this comment.
P2: Preserve Markdown mistaken for invalid frontmatter
splitFrontmatter() recognizes any leading pair of --- lines. For a valid standalone Markdown file that uses those as thematic rules, YAML can parse the captured instructions as a non-map, and this reset then silently discards everything between the rules. The upload reports success, but the stored SKILL.md is missing that content. When converting loose Markdown, retain the invalid/non-map block as Markdown content (or reject the file) rather than dropping it.
| if (options.inferUnknownBinary && pathContentType === "text/markdown" && !hasMarkdownExtension) { | ||
| if (file.type) { | ||
| contentType = file.type; | ||
| body = isTextContentType(contentType) ? await file.text() : await fileToBase64(file); |
There was a problem hiding this comment.
P2: Keep uploaded MIME compatible with the path-derived editor
This branch can store an unknown extension such as asset.bin as application/octet-stream with a base64 body, but DocumentEditor ignores the stored type and derives text/markdown from the path. Opening and saving that support file therefore treats the base64 as literal Markdown and rewrites its content type, so agents no longer receive the original binary data URI. The previous generic uploader explicitly stayed path-derived for this reason. Either the editor needs to honor the stored contentType end-to-end, or this upload path must use encoding compatible with contentTypeFromPath().
|
Submitted 2 actionable inline findings. |
e4f866b to
7f31883
Compare
|
LGTM! |
7f31883 to
6c13e78
Compare
|
LGTM! |
1 participant
Stacked on #504.
Adds support for uploading skill bundles (directories containing a SKILL.md manifest and supporting files) into the skills navigator.