feat(slides): add insert-image to place a sized image on an existing slide

[Czaruno]

What

Adds gog slides insert-image: place a positioned, sized image on an existing slide.

add-slide is great for full-bleed image slides, but there's no way to drop a sized image (a logo top-right, a chart in a content area) onto a slide you already have. The Slides REST createImage only accepts a publicly-fetchable URL, so callers otherwise have to hand-roll the upload + temporary public permission + createImage + cleanup, or make files public by hand.

This reuses the exact private-image flow add-slide/replace-slide already use, so nothing is left public:

1. Upload the image to Drive as a temp file.
2. Grant a temporary anyone: reader permission so the Slides image fetcher can read it.
3. createImage onto the target slide with explicit size + transform.
4. Delete the temp Drive file (defer).

Usage

gog slides insert-image <presentationId> <slideId> <image> \
  --x=<n> --y=<n> --width=<n> [--height=<n>] [--unit=PT|EMU]

• Args: presentationId, slideId, image (PNG/JPG/GIF).
• --width is required; --height is optional and derived from the image's aspect ratio when omitted.
• --unit defaults to PT.
• Confirms the target slide exists before uploading anything.

Testing

• go build ./..., go vet ./internal/cmd/ clean.
• New unit tests (httptest-mocked Drive + Slides, mirroring slides_add_slide_test.go):
  • places the image on the target slide with the expected size/transform, sets the temp permission, and deletes the temp file;
  • rejects a missing slide before any Drive call;
  • aspect-ratio derivation.
• Existing ./internal/cmd tests still pass.
• Verified live end to end against a real presentation: inserted a 120pt square logo at the top-right of an existing slide and confirmed placement via the rendered thumbnail.

Notes

• No new module dependencies (stdlib image for aspect detection).
• Per-command docs under docs/commands/ can be regenerated by the maintainers' doc tooling.

[clawsweeper]

Codex review: found issues before merge. Reviewed June 7, 2026, 4:46 PM ET / 20:46 UTC.

Summary
The PR adds gog slides insert-image with positioned, sized local-image insertion on an existing slide, generated command docs, unit tests, and a changelog entry.

Reproducibility: not applicable. this is a feature PR rather than a bug report. The PR discussion includes redacted live terminal proof plus mocked tests for the new command behavior.

Review metrics: 2 noteworthy metrics.

• Diff surface: 8 files changed, +533/-1. The PR is focused but adds a new command, implementation, tests, generated docs, and release-note text.
• Public sharing paths: 1 new anyone reader flow. The new Drive permission path is the security-boundary decision that unit tests cannot settle.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P2] Add a finite timeout around the cancellation-immune Drive cleanup delete.
• Get maintainer acceptance or requested adjustment for the temporary public Drive sharing policy.

Risk before merge

• [P1] The new command temporarily grants anyone reader access to a user-provided local image; maintainers need to accept that Slides policy or require explicit consent.
• [P2] The cleanup delete uses a cancellation-immune context without a timeout, so a stalled Drive request can hang the command after insertion and leave the temporary public upload exposed longer.

Maintainer options:

1. Bound cleanup before merge (recommended)
   Wrap the cancellation-immune Drive delete in a finite timeout and keep the stderr warning so cleanup cannot hang indefinitely.
2. Accept the Slides sharing pattern
   Maintainers can explicitly allow insert-image to follow add-slide and replace-slide once the cleanup path is bounded.
3. Require explicit public-share consent
   If Slides should follow the Docs policy instead, ask for a prompt or opt-in path before granting temporary anyone reader access.

Next step before merge

• [P2] The author can make the cleanup timeout repair, but maintainers still need to decide whether this new command should use the temporary public Drive-sharing policy by default.

Security
Needs attention: The diff introduces a new temporary public Drive sharing path, and its cleanup needs a timeout before the security boundary is comfortable to merge.

Review findings

• [P2] Bound the cleanup delete context — internal/cmd/slides_insert_image.go:141

Review details

Best possible solution:

Land the focused command after bounding cleanup and after maintainers explicitly choose the intended temporary-public-sharing policy for Slides image commands.

Do we have a high-confidence way to reproduce the issue?

Not applicable; this is a feature PR rather than a bug report. The PR discussion includes redacted live terminal proof plus mocked tests for the new command behavior.

Is this the best way to solve the issue?

Unclear; the implementation is narrow and proof-positive, but it is not the best merge state until cleanup is bounded and maintainers accept or adjust the temporary public-sharing policy.

Full review comments:

• [P2] Bound the cleanup delete context — internal/cmd/slides_insert_image.go:141
  The cleanup defer deletes the temporary public Drive file with context.WithoutCancel(ctx) but no timeout. If that Drive delete request stalls, Run can hang after creating the image while the anyone-readable upload remains public longer; wrap the uncancelled cleanup context in a finite timeout and cancel it.
  Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 7256355c7fc8.

Label changes

Label justifications:

• P2: This is a normal-priority user-facing Slides feature with bounded scope but nontrivial merge review needs.
• merge-risk: 🚨 security-boundary: The diff adds a new command path that temporarily grants public read access to a user-provided Drive upload.
• merge-risk: 🚨 availability: The cleanup delete uses a cancellation-immune context without a timeout, so a stalled Drive request can hang the command after insertion.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR discussion includes redacted terminal proof for dry-run, a real insert run, read-slide confirmation, and temp-upload deletion verification.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion includes redacted terminal proof for dry-run, a real insert run, read-slide confirmation, and temp-upload deletion verification.

Evidence reviewed

Security concerns:

• [medium] New temporary public Drive permission path — internal/cmd/slides_insert_image.go:147
  The command grants anyone reader access to an uploaded local image so Google Slides can fetch it; this is intentional but expands a security boundary that maintainers need to accept or align with the Docs confirmation behavior.
  Confidence: 0.88
• [medium] Unbounded cleanup request — internal/cmd/slides_insert_image.go:141
  The deletion uses context.WithoutCancel(ctx) without a timeout, so a stalled cleanup request can hang the command and prolong public exposure of the temporary upload.
  Confidence: 0.86

Acceptance criteria:

• [P1] go test ./internal/cmd -run 'TestSlidesInsertImage|TestImageAspectRatio' -v.
• [P1] go vet ./internal/cmd.

What I checked:

• Current main lacks the command: The main Slides command tree has add/replace/read/update commands but no InsertImage registration. (internal/cmd/slides.go:32, 7256355c7fc8)
• PR registers the command: The PR head adds InsertImage SlidesInsertImageCmd to the Slides command tree. (internal/cmd/slides.go:33, 072e1bf214c8)
• Cleanup is unbounded: The PR deletes the temporary Drive file with context.WithoutCancel(ctx) directly, so a stalled Drive delete can hang after insertion. (internal/cmd/slides_insert_image.go:141, 072e1bf214c8)
• Temporary public sharing path: The PR grants anyone reader access to the uploaded local image so Google Slides can fetch it. (internal/cmd/slides_insert_image.go:147, 072e1bf214c8)
• Existing Slides precedent: Current add-slide and replace-slide already use temporary Drive upload, public read permission, and cleanup for Slides image fetches. (internal/cmd/slides_add_slide.go:99, 7256355c7fc8)
• Nearby safer cleanup pattern: docs insert-image asks before temporary public sharing and wraps cancellation-immune cleanup in a 30 second timeout. (internal/cmd/docs_insert_image.go:85, 7256355c7fc8)

Likely related people:

• steipete: Recent main-history commits touched Slides image URL handling and related Docs image cleanup behavior, and this PR branch also has maintainer follow-up commits for changelog, lint, and generated docs. (role: recent area contributor; confidence: high; commits: c11876a7e9b8, b980fe7d3d94, 3bed2b06e788; files: internal/cmd/slides_add_slide.go, internal/cmd/slides_replace_slide.go, internal/cmd/docs_insert_image.go)
• chrismdp: The original merged Slides command bundle added add-slide, read-slide, update-notes, and replace-slide, including the image-deck workflow this PR extends. (role: introduced related Slides behavior; confidence: medium; commits: 8b08d11777fe; files: internal/cmd/slides.go, internal/cmd/slides_add_slide.go, internal/cmd/slides_replace_slide.go)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[Czaruno]

Thanks for the review. Addressed both P1 findings and added the requested behavior proof.

Changes (commit on this branch)

• Drive setup ordering: the target slide is now validated before the Drive service is created or anything is uploaded, so a bad slide id never touches Drive. The missing-slide test now asserts Drive is never called.
• Checked cleanup of the temporary public image: the temp file is deleted via a cancellation-immune context (context.WithoutCancel), and a failed delete now logs a loud Warning: to stderr instead of being swallowed (a failed cleanup could otherwise leave the upload world-readable). Added a cleanup-failure test asserting the warning fires.

Tests

$ go build ./...        # clean
$ go vet ./internal/cmd # clean
$ go test ./internal/cmd -run 'TestSlidesInsertImage|TestImageAspectRatio' -v
--- PASS: TestSlidesInsertImage_PlacesSizedImageOnExistingSlide
--- PASS: TestSlidesInsertImage_RejectsMissingSlide
--- PASS: TestSlidesInsertImage_WarnsWhenCleanupFails
--- PASS: TestImageAspectRatio
ok  github.com/steipete/gogcli/internal/cmd

Live behavior proof

Run against a real (now-deleted) scratch presentation, account redacted. A neutral 300x150 (2:1) PNG was used; note --height is omitted and auto-derived from the aspect ratio.

# dry-run shows the computed request (width 100 -> height 50 from the 2:1 image)
$ gog slides insert-image <PID> p /tmp/insimg_test.png --x=560 --y=24 --width=100 --dry-run -p
dry_run        true
op             slides.insert-image
request_json   {"height":50,"image":"/tmp/insimg_test.png","mime_type":"image/png",
                "presentation_id":"<PID>","slide_id":"p","unit":"PT","width":100,"x":560,"y":24}

# real run
$ gog slides insert-image <PID> p /tmp/insimg_test.png --x=560 --y=24 --width=100 -j
{
  "imageObjectId": "img_1780625295086419000",
  "slideObjectId": "p",
  "presentationId": "<PID>",
  "link": "https://docs.google.com/presentation/d/<PID>/edit"
}

# read-slide confirms the image element is now on the existing slide
$ gog slides read-slide <PID> p -p
Slide 1  (p)
Images:
OBJECT ID                URL
img_1780625295086419000  https://lh7-rt.googleusercontent.com/slidesz/AGV_vU...=s2048  (rendered, redacted)

The temporary Drive upload was deleted automatically after insertion (verified separately; a subsequent Drive listing shows no residual file).

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/26991112289
• Updated: 2026-06-05T02:16:11.521Z

[Czaruno]

Fixed the test-isolation nit: TestSlidesInsertImage_RejectsMissingSlide now saves and restores the package-global newDriveService in its cleanup (alongside newSlidesService). Full go test ./internal/cmd passes. That clears the only code finding; the remaining item is the maintainer call on the temporary public-Drive pattern, which matches the existing add-slide/replace-slide flow and now warns on cleanup failure.

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/27015675546
• Updated: 2026-06-05T12:51:54.272Z

[steipete]

Landed in bedfa7d.

Thanks @Czaruno. I accepted the temporary public Drive read handoff for this command so Slides can fetch the uploaded local image, with cleanup warning coverage if deletion fails.

Proof:

• go test ./internal/cmd -run 'TestSlidesInsertImage|TestImageAspectRatio'
• make lint
• make ci
• autoreview --mode branch --base origin/main: clean, no accepted/actionable findings
• GitHub Actions on 072e1bf: ci test/windows/darwin-cgo-build/worker passed; docker image passed; Socket checks passed