Skip to content

fix(images): fix mship image handling - #6357

Merged
Sg312 merged 1 commit into
stagingfrom
mship-images
Aug 7, 2026
Merged

fix(images): fix mship image handling#6357
Sg312 merged 1 commit into
stagingfrom
mship-images

Conversation

@Sg312

@Sg312 Sg312 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Brief description of what this PR does and why.

Companion: https://github.com/simstudioai/mothership/pull/412

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

How has this been tested? What should reviewers focus on?

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

@Sg312
Sg312 requested a review from a team as a code owner August 7, 2026 00:30
@vercel

vercel Bot commented Aug 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 7, 2026 12:34am

Request Review

@greptile-apps

greptile-apps Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR changes Copilot attachment preparation so one failed upload is represented by an omission marker while valid sibling attachments continue to be prepared.

  • Replaces request-wide failure with per-attachment failure isolation.
  • Updates payload tests to cover a failed attachment alongside a successful image.
  • Removes the top-level configVersion field from bun.lock.

Confidence Score: 4/5

The PR appears safe to merge after considering the non-blocking need to normalize filenames in failed-attachment context markers.

Per-file failure isolation preserves valid sibling attachments as intended, but the new failure path exposes raw attachment metadata to model-visible context instead of following the successful path's normalization.

Files Needing Attention: apps/sim/lib/copilot/chat/payload.ts

Important Files Changed

Filename Overview
apps/sim/lib/copilot/chat/payload.ts Adds per-attachment failure isolation, but embeds an unnormalized client filename in the new model-visible failure marker.
apps/sim/lib/copilot/chat/payload.test.ts Updates coverage to verify that failed attachments are omitted while valid siblings remain available.
bun.lock Removes only the top-level configVersion metadata field, with no dependency resolution changes shown.

Reviews (1): Last reviewed commit: "fix(mship): fix image handling" | Re-trigger Greptile

Comment thread apps/sim/lib/copilot/chat/payload.ts
@github-actions github-actions Bot added the requires-mothership-merge Has a companion PR on the mothership/copilot side — merge in lockstep label Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

⚠️ Cross-repo companion check

One or more companion PRs aren't merged into staging yet. Merging this without them will leave copilot and sim out of sync — merge them in lockstep.

@Sg312
Sg312 merged commit 5c0c364 into staging Aug 7, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the mship-images branch August 7, 2026 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-mothership-merge Has a companion PR on the mothership/copilot side — merge in lockstep

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant