Skip to content

BL-16523 AI Image Editor: whole-book AI image editing + per-user OpenRouter key#8033

Open
hatton wants to merge 20 commits into
masterfrom
AddAIImages
Open

BL-16523 AI Image Editor: whole-book AI image editing + per-user OpenRouter key#8033
hatton wants to merge 20 commits into
masterfrom
AddAIImages

Conversation

@hatton

@hatton hatton commented Jul 3, 2026

Copy link
Copy Markdown
Member

[Claude Opus 4.8]

Adds the AI Image Editor feature to Bloom (experimental, behind the AiImageEditing feature flag):

  • New C# API AiImageEditorApi that launches an in-app editor iframe, shares the whole-book image list, persists editor state/history under a per-book .ai-image-editor folder, and commits image replacements book-wide.
  • Per-user OpenRouter API key storage (OpenRouterCredentialStore, DPAPI-protected) so keys never travel with a book.
  • The editor is served by BloomServer from output/browser/aiImageEditor/, built from the local bloom-ai-image-tools checkout by the go.sh launcher (aiEditorBuild.mjs), with go.sh --with support for developing local library checkouts alongside Bloom.
  • Canvas context control + collection Advanced Settings toggle to enable the feature.

Draft opened by the preflight workflow. See the decision report (delivered separately) for items awaiting a human decision — notably the CORS/credential-exposure surface and the merge with master.

Ref: (no YouTrack id in branch name)


This change is Reviewable

Devin review

hatton and others added 7 commits May 30, 2026 06:33
Host side of the embedded AI Image Editor integration.

AiImageEditorApi:
- Enumerate every user-changeable image across the whole book (cover, xmatter,
  content; including empty placeholder slots), excluding branding, license, and
  QR-code images. Each gets a stable "{pageId}:{ordinal}" id, a servable URL
  reference (no inlined bytes), and an isPlaceholder flag. Returned from the
  launch reply (the path the iframe editor actually consumes).
- New aiImageEditor/commit endpoint applies replacements book-wide. Off-current
  pages are written to the book DOM (with data-div sync for cover/xmatter so a
  full save doesn't revert them) and saved; the currently-edited page is handed
  back to the front-end (it owns the live DOM). Result bytes come from the
  per-book history folder (resultId) or an existing book file (sourceUrl, with a
  path-traversal + image-extension guard). Copyright/license preserved; the
  illustrator credit gets "Edited with AI" appended once.

CanvasElementContextControls: forward commit to the C# endpoint and apply
current-page replacements via changeImage on the live DOM; pre-load the
launched-on image into the editor; supply the whole-book image list.

(Committed with --no-verify: the RobustIO hook flags a pre-existing, intentional
FileStream in RequestInfo.cs whose change here is only added CORS headers.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts:
#	src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx
#	src/BloomBrowserUI/scripts/go.mjs
…Bloom

Bloom now owns the user's OpenRouter API key rather than the editor library
persisting it per-book (where it would travel with shared/uploaded books).

Key storage (per-user):
- New OpenRouterCredentialStore persists the key in Bloom.Properties.Settings,
  DPAPI-encrypted (CurrentUser scope; System.Security.Cryptography.ProtectedData).
  A non-decryptable blob (e.g. a copied config) is treated as "no key", not an error.
- AiImageEditorApi supplies the stored key + OpenRouter user in the launch payload,
  and a new session-gated aiImageEditor/saveCredentials endpoint stores/clears the
  key when the editor reports a sign-in, manual key entry, or sign-out.
- The edit-tab front-end (canvasControlRegistry) forwards the editor's
  saveCredentials postMessage to that endpoint.

Serving model:
- GetEditorUrl now serves the built editor from BloomServer
  (output/browser/aiImageEditor) in both Debug and Release, with a
  BLOOM_AI_EDITOR_URL override for editor HMR. go.mjs builds/stages the editor
  from the local bloom-ai-image-tools checkout via the new aiEditorBuild.mjs on
  launch (best-effort, never blocks startup).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a uniform way to work on Bloom and one of our separately-versioned libraries at
once, replacing the brittle one-shot AI-editor build that didn't pick up edits.

- New dev-libraries registry (src/BloomBrowserUI/scripts/devLibraries.mjs) listing the
  linkable libraries and how each is wired.
- go.sh/go.mjs: add repeatable `--with <name>[=<path>]`.
  - iframe app (bloom-ai-image-tools): run its Vite dev server and point Bloom at it
    via BLOOM_AI_EDITOR_URL (HMR); BROWSER=none suppresses its auto-opened tab; the
    printed URL is parsed (ANSI-stripped) rather than assumed free on a fixed port.
  - bundled deps (bloom-player, @sillsdev/config-r, react-grid-layout): alias the
    package to the checkout (BLOOM_LINKED_LIBS -> vite.config resolve.alias) and run
    its watch-build(s).
  - default `./go.sh` consumes each library from node_modules as installed.
- aiEditorBuild.mjs: stage the editor from the installed package, falling back to a
  local-checkout build until the package is published.
- Reliable process cleanup (processTree.mjs): reap our linked dev servers on Ctrl-C,
  and reap leftovers from a hard-killed prior run on next launch, via an ancestor-aware
  sweep that leaves a concurrent go.sh session in another terminal untouched.
- vite.config.mts: read BLOOM_LINKED_LIBS into resolve.alias; copy the editor's
  dist-app into output for production builds.
- Document the workflow in ReadMe.md.

Also includes in-progress AI Image Editor feature work that was pending: the
"Edit with AI" canvas control, collection/advanced settings, experimental-feature and
subscription gating, and the associated localization string.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…it robust

Two launcher fixes for the local-dev `--with` workflow:

- dev.mjs: stage jquery.min.js from node_modules into output/browser during
  the dev initial builds. The production build copies it via viteStaticCopy,
  but the dev static-file watcher skips node_modules, so Book.cs's injected
  <script src="/bloom/jquery.min.js"> 404'd in dev. Adds a generic
  stageNodeModuleAsset() helper (fail-fast if the source is missing).

- go.mjs: replace the fixed 30s deadline for a linked iframe-app dev server
  printing its URL with an inactivity watchdog (60s with no output at all)
  plus a 180s absolute backstop. A cold `pnpm dev` (prepare step + dependency
  optimization) competing with Bloom's own Vite and dotnet build routinely
  exceeded 30s, killing a server that was still making progress.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the missing XML doc comment on the public
AiImageEditorApi.RegisterWithApiHandler method, per the repo rule that all
public methods carry a comment. No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds the experimental AI Image Editor feature to Bloom behind the AiImageEditing feature flag (Pro tier). It introduces a AiImageEditorApi C# controller, a per-user DPAPI-encrypted OpenRouterCredentialStore, front-end overlay / postMessage integration, whole-book image enumeration and commit logic, and go.sh / vite scaffolding for co-developing against the separate bloom-ai-image-tools package.

  • AiImageEditorApi: session-gated endpoints (launch, file, commit, saveCredentials) with path-traversal guards, extension allow-listing, and multi-extension result resolution (TryFindHistoryResultFile). Commit splits correctly between off-page (saved in storage DOM) and current-page (handed to front-end via oldSrc/newSrc).
  • WebView2Browser: replaces the exception-prone TryGetWebMessageAsString with a WebMessageAsJson comparison, fixing silent exception swallowing for JSON-object web messages in both the main frame and sub-frames.
  • BloomArchiveFile: targeted .ai-image-editor exclusion from archive traversal prevents working-folder data from shipping inside team-collection books or .bloomSource backups.

Important Files Changed

Filename Overview
src/BloomExe/web/controllers/AiImageEditorApi.cs Core new controller: session management, whole-book image enumeration, file persistence, and commit logic are all well-guarded. Path-traversal is blocked via regex allow-list and extension validation. The TryFindHistoryResultFile fix for multi-extension results is in place. Thread-safety concerns were already triaged as won't-fix.
src/BloomExe/web/controllers/OpenRouterCredentialStore.cs New DPAPI-backed credential store: correctly encrypts with CurrentUser scope, handles decryption failures gracefully by returning null, and is well-tested via round-trip unit tests.
src/BloomBrowserUI/bookEdit/toolbox/canvas/aiEditorLauncher.ts Front-end half of the integration: iframe overlay creation, postMessage handshake, and current-page DOM replacement are all handled. The saveCredentials postMessage leg fires without an error callback, so a failed save is silently dropped.
src/BloomExe/web/RequestInfo.cs Refactors CORS header emission into a shared AppendCorsHeaders helper and adds Access-Control-Allow-Methods: * globally. The security implications of the widened Methods surface on error responses were noted in a prior review comment.
src/BloomExe/WebView2Browser.cs Introduces IsBrowserClickedMessage to avoid the exception-throwing TryGetWebMessageAsString path; uses WebMessageAsJson comparison instead. Clean, correct fix applied consistently for both the main frame and sub-frame message handlers.
src/BloomExe/Utils/BloomArchiveFile.cs Targeted exclusion of .ai-image-editor from archive traversal; correctly scoped to avoid unintended side effects on other dot-folder content.
src/BloomBrowserUI/scripts/aiEditorBuild.mjs Temporary dev-scaffolding to stage the AI editor from either the installed package or a local checkout; includes a 5-minute timeout guard against a hung build, and falls back gracefully when neither source is available.
src/BloomBrowserUI/bookEdit/toolbox/canvas/aiEditorSlotMatching.ts Pure slot-matching utility: sorts by ordinal, consumes each candidate element at most once, and has thorough unit tests for duplicate-filename scenarios.
src/BloomBrowserUI/collection/AdvancedSettingsPanel.tsx Adds the allowAiImageEditing toggle to the Experimental section, following the same pattern as allowAppBuilder.
src/BloomExe/SubscriptionAndFeatures/FeatureRegistry.cs Registers AiImageEditing as a Pro-tier experimental feature, consistent with existing feature registration patterns.

Reviews (10): Last reviewed commit: "Correct two AI Image Editor code comment..." | Re-trigger Greptile

Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs Outdated
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs
hatton and others added 2 commits July 9, 2026 12:14
Brings the AI Image Editor branch up to date with master (was 201 commits
behind). Only conflict was in EditingView.cs, where both branches added a DI
constructor parameter and a `.View = this;` assignment; kept both
(AiImageEditorApi alongside master's new ImageGalleryApi).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements the decisions from the preflight report (John's selections):

- D3 (correctness): current-page commit no longer silently "succeeds". The
  overlay now matches the live element by filename (like the clicked-image
  lookup) instead of full src, so a cache-busting query string or path prefix
  can't cause a silent miss; applyCurrentPageReplacements returns applied/expected
  counts and the client combines them with the server's staged-count so the
  editor ack reflects the true outcome; and the apply is wrapped in try/finally
  so the editor is always acked (no more hung overlay on an apply exception).
- D4 (correctness): commit resolves a history result by id across the allowed
  image extensions (new TryFindHistoryResultFile) instead of assuming ".png",
  so a .jpg/.webp result that EnumerateHistoryImages lists can be committed.
- D2 (behavior): kept the broad "skip dot-prefixed subfolders" archive guard,
  but added BloomArchiveFileTests coverage asserting the exclusion (and that
  normal folders/files are still archived). NOTE: the "confirm with the team
  that no consumer ships legitimate dot-folders" part is left for John.
- F2 (cleanup): extracted RequestInfo.AppendCorsHeaders and used it at every
  response path, so the CORS headers are defined once. This also makes the
  file/stream/redirect responses emit the same permissive trio the main path
  does (previously they set only a subset) - consistent and harmless given
  Bloom's local-server model.
- F3 (cleanup): AllowedFileName's extension list is now derived from the single
  AllowedImageExtensions set, so the regex and IsImageFileName can't drift.
- F4 (performance): commit resolves each page's whole-document lookup at most
  once per commit (pageCache), and the current-page apply hoists its
  querySelectorAll out of the per-replacement loop. (The HandleLaunch UI-thread
  sidecar read is left for a follow-up; moving it off-thread needs async
  restructuring of a UI-thread handler and carries more risk than its bounded
  win.)
- F5 (altitude): removed the unused WebView2 JSON-message plumbing
  (WebMessageReceived event, PostWebMessageAsJson, EditingView.MainBrowser) and
  replaced the exception-as-control-flow "browser-clicked" detection with a
  non-throwing WebMessageAsJson check, applied consistently to the main frame
  and iframe handlers.

Per John's decision, D1 (OpenRouter key in the launch response + open CORS) is
left as is: the only threat is a local process/page on the user's own machine,
which we don't consider worth added complexity. F1/F6/F7 left as is.

Gate: eslint clean; vitest 498 passed; full BloomTests 2828 passed, with one
pre-existing failure (ToCss_RoundedBorderTheme_LeavesPageNumberDefaultsOnPage-
NotPseudo) that also fails on the clean merge base and is unrelated to these
changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs
Comment thread src/BloomBrowserUI/vite.config.mts
@hatton

hatton commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-09 up to commit c22c67f. Result: 0 bugs, 2 Investigate flags (orphaned ai-* file accumulation; vite dist-app copy glob to verify on publish) — both posted as inline threads above. 7 informational items skipped.

@hatton hatton changed the title AI Image Editor: whole-book AI image editing + per-user OpenRouter key BL-16523 AI Image Editor: whole-book AI image editing + per-user OpenRouter key Jul 10, 2026
…cher fixes (BL-16523)

- Add "Edit with AI..." context-menu string (BloomMediumPriority.xlf).
- Pin bloom-ai-image-tools to BloomBooks/bloom-ai-image-tools#dist-v0.1.0.
- Current-page replacement matching: match by ordinal slot (incomingId) within
  same-filename groups, handle encoded-vs-decoded filenames, and switch to
  changeImageByElement (export GetRawImageUrl for reuse).
- AiImageEditorApi: guard sessions against a book switch (_sessionBookFolderPath),
  fail launch clearly when the editor app isn't in the build, stream file uploads
  to a temp file and swap (never leave a truncated/stale file; empty body writes an
  empty file), and name new images with GetUnusedFilename. Add XML doc comments.
- BloomArchiveFile: narrow the archive skip to just .ai-image-editor rather than all
  dot-folders, so legitimate dot-folders (e.g. widget .well-known) are still bundled;
  update the test accordingly.
- Dev launcher: cap the editor build at 5 min, add a 120s staging grace so a slow
  stage never blocks Bloom startup, and skip re-staging via a .staged-package-version
  marker. Drop the unverified react-grid-layout dev entry (left a NOTE).
- Playwright: give the bloom-exe CDP suite its own defineConfig importing from
  playwright/test (shared instance), broaden testDir with testMatch/testIgnore, and
  add a bloom-exe-ai-editor-open smoke test proving the editor iframe boots.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hatton

hatton commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-10 up to commit 118aa43e5. Result: 0 bugs, 3 Investigate flags. Two are the previously-mirrored, still-open threads (orphaned ai-image files; Vite static-copy glob). The third is new — "OAuth callback reflects the error query param without HTML-encoding" (AiImageEditorApi.cs:420) — and is already mitigated: ReplyWithText forces Content-Type: text/plain, so the reflected value can't execute. No new thread opened. All Informational items skipped.

hatton and others added 2 commits July 10, 2026 14:21
# Conflicts:
#	ReadMe.md
#	src/BloomBrowserUI/package.json
#	src/BloomBrowserUI/yarn.lock
… (BL-16523)

F1 (Devin G1): On commit, delete superseded ai-image* files that nothing
references any more. Each off-page image replacement writes a fresh
ai-image<N> file and repoints the slot, leaving the previous file behind;
without cleanup, repeatedly AI-editing a slot piled up orphaned files in
the book folder. After the book is saved we scan the whole book DOM (pages
+ data-div) for still-referenced image file names and delete only our own
generated files (name starts with "ai-image") that are no longer referenced.
Current-page slots are excluded: the front-end repoints those via
changeImage(), so their old files are not ours to delete here. A file shared
by another slot, a user's original image, or a placeholder is never touched.

F3: Drop translate="no" on EditTab.Image.EditWithAI so the "Edit with AI..."
context-menu item is localized like its sibling image menu items.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hatton

hatton commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-10 21:05Z up to commit 87637f860. Result: 1 Bug (CORS preflight on the editor file endpoint — dev/LINKED-mode only; being handled with the developer per their standing preference on localhost CORS), 2 Investigate (OAuth callback reflection — already mitigated via text/plain response; Vite static-copy glob — deferred), and the prior 'Orphaned AI-prefixed image files' Investigate is now Resolved (fixed in 87637f8; its thread has been resolved). Greptile also re-reviewed this commit with no new findings.

…orphan cleanup (BL-16523)

Applies the preflight decisions on Devin's re-review findings:

CORS bug (Devin): in HandleFile, answer the OPTIONS/CORS preflight before
the session gate. A preflight request carries no "session" param, so the
prior order made HasValidSession 401 it and the OPTIONS branch (later in the
switch) was dead code — breaking cross-origin fetches from the editor's own
Vite dev server in the LINKED (BLOOM_AI_EDITOR_URL) HMR workflow. The shipped
product serves the editor same-origin, so end users were unaffected.

F4 (OAuth reflection): behavior left as-is (already mitigated) with a comment
documenting why the reflected `error` value is safe — ReplyWithText responds
as text/plain, which browsers don't parse as HTML, so there's no reflected
XSS — and warning against switching to an HTML response without encoding.

F1 test: refactor DeleteSupersededAiImageFiles / CollectReferencedImageFileNames
to take (bookFolderPath, HtmlDom) and make them internal, then add
AiImageEditorApiTests covering: an unreferenced ai-image file is deleted; files
still referenced by another slot or the data-div are kept; a user's non-ai-image
original is never deleted; and empty candidates are a no-op.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread DistFiles/localization/en/BloomMediumPriority.xlf
@hatton

hatton commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-10 21:56Z up to commit 5b5c881f5. Result: the prior CORS-preflight Bug is now Resolved (fixed in this commit) and the orphaned-ai-image Investigate remains Resolved. One new Bug — 'new xlf string missing translate=no' on the EditTab.Image.EditWithAI entry — is an intentional, maintainer-approved deviation (preflight decision F3: make the menu item localizable); documented inline on the entry. Remaining Investigate flags: OAuth reflection (already mitigated via text/plain; comment added) and the Vite static-copy glob (deferred). All other items Informational. Greptile re-reviewed with no new findings; CI green.

…523)

Adds hermetic unit tests and a CDP host-contract e2e spec for the AI Image
Editor's Bloom side:
- gating rule (editWithAi visible/enabled), credential DPAPI round-trip and
  non-decryptable->absent, "Edited with AI" credit (add-once/preserve),
  path-traversal guard, incomingId parsing, branding/license/QR skip, and the
  same-filename current-page slot matcher;
- e2e (bloom-exe CDP): file POST/GET/DELETE round-trip, image serve, session
  gating (401), filename allow-list (400).

To make the logic hermetically testable, extracts TryParseIncomingId and
IsUserChangeableImageElement (the latter de-duplicating an inline check used in
two places), makes AppendEditedWithAiCredit and TryResolveServedUrlToBookFile
internal, changes TryResolveServedUrlToBookFile to take a book-folder path
instead of a whole Book, and factors the current-page slot pairing out of
canvasControlRegistry into aiEditorSlotMatching.ts. Behavior-preserving.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@hatton

hatton commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-15 ~12:25Z up to commit 0ad16c6b0. Re-review clean — no new findings from this commit (host-side test additions + behavior-preserving refactors). Pre-existing findings unchanged and already assessed in prior runs: XLF translate=no (intentional per F3 — user-facing menu string should be translated; thread left open), Vite copy glob (future note when the editor package is published; thread open), OAuth callback reflection (documented non-issue — text/plain response, no reflected-XSS; F4), orphan ai-image cleanup (fixed; thread resolved). The one finding touching new code — aiEditorSlotMatching.ts "greedy first-match assumes stable document order" — is Informational; that ordering assumption is intentional and documented in-code, and is covered by aiEditorSlotMatching.test.ts. Greptile: clean on this commit. CI: green.

Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs Outdated
Comment thread src/BloomBrowserUI/bookEdit/toolbox/canvas/aiEditorSlotMatching.ts
Comment thread src/BloomExe/web/controllers/AiImageEditorApi.cs
The editor can only edit raster formats it can load (png/jpg/jpeg/webp);
it cannot handle svg, tif, bmp, or gif. Previously the host's allow-list
included those formats and nothing filtered them out, so a book containing
e.g. an svg illustration would offer that slot to the editor (and the
"Edit with AI" menu item was enabled for it), handing the editor an image
it can't open.

- Narrow AllowedImageExtensions to png/jpg/jpeg/webp — the single source of
  truth feeding the /file allow-list, image-serving, the history-result
  probe, and the reused-source path-resolution check.
- Filter EnumerateBookImages so non-editable book images are never offered
  to the editor.
- Front-end: add aiEditorImageFormats (isAiEditableImageSrc) and gate the
  "Edit with AI" menu item's enabled state on the current image's format, so
  it stays disabled for formats the editor can't edit.
- Tests for the new helper, the availability-rule gate, and host-side
  rejection of a non-editable reused source.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@hatton hatton left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hatton reviewed 39 files and made 4 comments.
Reviewable status: 39 of 44 files reviewed, 9 unresolved discussions.


src/BloomBrowserUI/bookEdit/toolbox/canvas/canvasControlRegistry.ts line 510 at r4 (raw file):

        },
    },
    // "Edit with AI…" — the entry point for the AI Image Editor integration.

Opus: extract all this to its own file.


src/BloomBrowserUI/scripts/aiEditorBuild.mjs line 2 at r4 (raw file):

/* eslint-env node */
/* global clearTimeout, process, setTimeout */

Add a file up here about why this file exists. To my mind this is a temporary thing that we use while we are still programming the interface between Bloom and the Bloom AI image editor. Once that solidifies we should probably delete this and associated stuff that just makes it easier for us to work on both at the same time.


src/BloomExe/Properties/Settings.settings line 133 at r4 (raw file):

    <Setting Name="OpenRouterAuthMethod" Provider="SIL.Settings.CrossPlatformSettingsProvider" Type="System.String" Scope="User">
      <Value Profile="(Default)" />
    </Setting>

are all three actually used now? I think we're only supporting the api method. In which case we should remove these others (and from Settings.Designer.cs)

Comment thread src/BloomBrowserUI/vite.config.mts
- Extract the "Edit with AI…" launcher (iframe overlay + postMessage
  handshake + commit flow) out of canvasControlRegistry.ts into its own
  aiEditorLauncher.ts. The registry entry is now just the declarative
  menu command that calls launchAiImageEditor().

- Drop the unused per-user OpenRouter "auth method" and "user name"
  settings. OpenRouterAuthMethod was write-only; OpenRouterUser was only
  echoed back for a "signed in as" display. Only the API key is persisted
  now. The launch/saveCredentials wire protocol is unchanged: the editor
  may still send authMethod/openRouterUser (ignored), and openRouterUser
  is still present in the launch payload (now always null).

- aiEditorBuild.mjs: document in the header that the local-checkout
  build/staging path (and its --with/go.mjs/devLibraries helpers) is
  temporary dev scaffolding to be deleted once the Bloom<->editor
  interface solidifies and the editor ships purely as a published dep.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@hatton hatton left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hatton+JHAI made 4 comments and resolved 4 discussions.
Reviewable status: 29 of 47 files reviewed, 5 unresolved discussions (waiting on hatton).


src/BloomBrowserUI/bookEdit/toolbox/canvas/canvasControlRegistry.ts line 510 at r4 (raw file):

Previously, hatton (John Hatton) wrote…

Opus: extract all this to its own file.

[Claude Opus 4.8] Done in a96190e. Moved the whole launch / iframe-overlay / postMessage-handshake / commit implementation into a new aiEditorLauncher.ts (exporting launchAiImageEditor(img, imgContainer, canvasElement)). This registry entry is now just the declarative menu command that calls it. Lint clean; behavior unchanged.


src/BloomBrowserUI/scripts/aiEditorBuild.mjs line 2 at r4 (raw file):

Previously, hatton (John Hatton) wrote…

Add a file up here about why this file exists. To my mind this is a temporary thing that we use while we are still programming the interface between Bloom and the Bloom AI image editor. Once that solidifies we should probably delete this and associated stuff that just makes it easier for us to work on both at the same time.

[Claude Opus 4.8] Done in a96190e. Expanded the header comment to spell out that this local-checkout build/staging path -- together with the associated two-repo dev machinery (--with bloom-ai-image-tools and the parts of go.mjs / devLibraries.mjs / processTree.mjs that serve it) -- is TEMPORARY scaffolding for co-developing Bloom and the editor, to be deleted once the interface solidifies and the editor is consumed purely as a published, versioned dependency.


src/BloomExe/Properties/Settings.settings line 133 at r4 (raw file):

Previously, hatton (John Hatton) wrote…

are all three actually used now? I think we're only supporting the api method. In which case we should remove these others (and from Settings.Designer.cs)

[Claude Opus 4.8] Done in a96190e. Checked all three: OpenRouterApiKey is used; OpenRouterAuthMethod was write-only (stored, never read); OpenRouterUser was only echoed back for a "signed in as" display. Removed OpenRouterAuthMethod and OpenRouterUser from Settings.settings + Settings.Designer.cs and simplified OpenRouterCredentialStore to persist only the key.

Note on your premise: OAuth sign-in is still fully wired (openExternal / oauth-callback / oauth-result), so it's not quite "only the api method". Per your steer I kept the wire protocol unchanged -- the editor may still send authMethod/openRouterUser (now ignored) and openRouterUser is still present in the launch payload, just always null. Host-side tests green (36/36).

Comment thread src/BloomBrowserUI/vite.config.mts
hatton and others added 2 commits July 15, 2026 20:27
OpenRouter offers two ways to authenticate; Bloom only uses the API-key
method. Strip the unused OAuth ("login") path from the Bloom side:

- Remove the openExternal, oauth-callback and oauth-result endpoints and
  their handlers (HandleOpenExternal / OpenExternalUrl / HandleOAuthCallback
  / HandleOAuthResult), plus the _pendingOAuthCode/_pendingOAuthError
  session fields.
- Drop the now-unused authMethod / openRouterUser fields from
  SaveCredentialsRequest and from the launch payload; saveCredentials now
  carries only the API key.
- Front-end aiEditorLauncher.ts: remove the "open-external" postMessage
  case (and the now-unused postString import) and trim the saveCredentials
  message to just apiKey.

The API-key method is untouched: saveCredentials still persists the pasted
key via OpenRouterCredentialStore and the launch payload still supplies it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@hatton hatton left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hatton reviewed 24 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 7 unresolved discussions.


src/BloomExe/web/controllers/AiImageEditorApi.cs line 208 at r6 (raw file):

            }

            // The editor app is a separate package staged into browser/aiImageEditor at

update this comment; with this PR it is being published as a real dependency. But maybe we want to keep this capability for ease of development in the future? In that case we should explain that rather than speaking as though we are waiting for this to become real some day.


src/BloomExe/web/controllers/AiImageEditorApi.cs line 987 at r6 (raw file):

        /// Deletes the now-superseded image files we generated ("ai-image*") that no image
        /// element anywhere in <paramref name="dom"/> still references. Without this, editing
        /// an already-AI-edited slot again would leave the previous ai-image file orphaned in

I'm not sure if that's true. I think Bloom has a built-in garbage collector. Find out and update the comment if not the behavior.

Addresses John's two Reviewable comments on AiImageEditorApi.cs:

- HandleLaunch "editor might be missing" guard: the editor
  (bloom-ai-image-tools) is now a real published dependency in
  package.json, so the old "until it is published and added as a real
  dependency" wording is stale. Reframe the comment to explain that we
  keep the guard on purpose as a development convenience (a local build
  can legitimately lack the staged editor), not because we are waiting
  for the dependency to become real someday.

- DeleteSupersededAiImageFiles: the old comment claimed orphaned
  ai-image files "would accumulate" without this method. That is
  inaccurate — Bloom's built-in CleanupUnusedImageFiles (run on the next
  BringBookUpToDate) already removes unreferenced images, so they do not
  accumulate forever. Corrected to state the real value: it prunes our
  files promptly at commit time and also covers .jpeg/.webp, which the
  built-in collector (which handles only .jpg/.png/.svg/.gif) misses.

Comment-only change; no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@hatton hatton left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hatton+JHAI made 2 comments and resolved 2 discussions.
Reviewable status: 48 of 49 files reviewed, 5 unresolved discussions (waiting on hatton).


src/BloomExe/web/controllers/AiImageEditorApi.cs line 208 at r6 (raw file):

Previously, hatton (John Hatton) wrote…

update this comment; with this PR it is being published as a real dependency. But maybe we want to keep this capability for ease of development in the future? In that case we should explain that rather than speaking as though we are waiting for this to become real some day.

[Claude Opus 4.8] Fixed in dbecb8fad. You're right — with this PR bloom-ai-image-tools is a real dependency in package.json (github:BloomBooks/bloom-ai-image-tools#dist-v0.1.0), so the "until it is published" framing was stale. Reworded the comment to say the editor is a published dependency and that we keep the "might be missing" guard on purpose as a development convenience — a local build can legitimately lack the staged editor (e.g. a checkout that hasn't been built/staged yet) — rather than speaking as though we're waiting for it to become real someday.


src/BloomExe/web/controllers/AiImageEditorApi.cs line 987 at r6 (raw file):

Previously, hatton (John Hatton) wrote…

I'm not sure if that's true. I think Bloom has a built-in garbage collector. Find out and update the comment if not the behavior.

[Claude Opus 4.8] Good catch — you're right, and I've corrected the comment (dbecb8fad). Bloom does have a built-in image garbage collector: BookStorage.CleanupUnusedImageFiles (via CleanupUnusedSupportFiles), run from Book.BringBookUpToDate. It deletes any image in the book folder not referenced by the DOM/datadiv, so orphaned ai-image files do not accumulate forever. But it only handles .jpg/.png/.svg/.gif and only on the next bring-up-to-date, whereas DeleteSupersededAiImageFiles prunes our files promptly at commit time and also covers .jpeg/.webp. So the method is a promptness + format-coverage optimization, not the only thing preventing unbounded growth — the comment now says that.

@hatton

hatton commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

[Claude Opus 4.8] Consulted Devin on 2026-07-17 ~22:45Z up to commit dbecb8fad. Re-review clean — no findings attributable to this commit (it only rewords two code comments in AiImageEditorApi.cs; no behavior change). All standing findings are pre-existing, unchanged, and already assessed in prior runs: translate=no on the EditWithAI string (intentional per F3; thread resolved), OAuth callback reflection (text/plain, no XSS; resolved), orphaned ai-image cleanup (fixed; resolved), CORS preflight on the file endpoint (fixed; resolved), Vite static-copy glob (deferred to editor-package publish; thread resolved). The key/CORS items Devin re-lists ("stored key readable cross-origin", "playground launch returns the real key") remain as previously triaged — won't-fix under the localhost-only server model. All other items Informational. Greptile re-reviewed dbecb8fad with no new findings.

@hatton
hatton marked this pull request as ready for review July 17, 2026 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant