summercms/.planning/phases/14-domain-jobs-and-external-integrations/14-REVIEW-FIX.md

phase, fixed_at, review_path, iteration, findings_in_scope, fixed, skipped, status

phase	fixed_at	review_path	iteration	findings_in_scope	fixed	skipped	status
14-domain-jobs-and-external-integrations	2026-10-04T06:20:00Z	.planning/phases/14-domain-jobs-and-external-integrations/14-REVIEW.md	1	7	7	0	all_fixed

Phase 14: Code Review Fix Report

Fixed at: 2026-10-04T06:20:00Z Source review: .planning/phases/14-domain-jobs-and-external-integrations/14-REVIEW.md Iteration: 1

Summary:

• Findings in scope: 7 (CR-01, WR-01 to WR-06; the Info findings were out of scope)
• Fixed: 7
• Skipped: 0

All fixes landed in fonoteka.go and its plugin submodules sm-feedback-plugin and sm-golem-plugin. No change was needed in summercms.go. Each submodule fix is committed in the submodule first, then the pointer is bumped in fonoteka.go with its own fix(14): <id> bump … commit.

Verification

• Where it ran: the main checkout, because workflow.use_worktrees is false. The numbers can be reproduced from the current tree.
• After every fix: go vet and go test on the affected module, with the testcontainers Postgres suites running under Docker.
• After the last fix: go vet and go test over the whole fonoteka.go workspace (root, parity, fonoteka, golem, feedback and user plugins). Everything was green.
• Environment note: in this sandbox /tmp hit a disk quota. The linker and the CSV multipart uploads failed with "disk quota exceeded" or a 500 until GOTMPDIR and TMPDIR pointed at the nvme volume. This affects the environment only, not the code. TestCsvStoreAndShow failed the same way on the unmodified tree.
• Each new regression test was checked to fail on the pre-fix code, where practical (CR-01, WR-01, WR-05).

Fixed Issues

CR-01: Album writes after slow Discogs I/O save every column from a stale struct, silently reverting concurrent edits

Status: fixed: requires human verification (write-shape and logic change) Files modified: fonoteka.go/plugins/golem15/fonoteka/classes/album_write_service.go, fonoteka.go/plugins/golem15/fonoteka/classes/discogs/applicator.go, fonoteka.go/plugins/golem15/fonoteka/classes/discogs/cover_fetcher.go, fonoteka.go/plugins/golem15/fonoteka/classes/discogs/postgres_test.go Commit: d64d3ab (fonoteka.go) Applied fix:

• Added classes.SaveAlbumColumns(ctx, db, a, cols...). It runs the Album model rules, then Model(a).Select(cols…).Updates(a), so GORM hooks and lighthouse broadcasts still fire. It also adds the columns Album.BeforeSave derives from the given ones (tracklist → track_titles; market_price_stored → currency, source and checked_at; market_price_currency → source) and always adds updated_at.
• The applicator now records each field it actually sets and writes only those.
• persistMarketPrice writes only the market-price columns.
• attachCover writes only cover_import_failures.
• A new subtest edits name, notes and shelf after the album is loaded and checks that the apply keeps them.
• One behaviour to check: an apply that changes no catalog field still issues an updated_at-only UPDATE and broadcast. The old full Save did the same. Eloquent would skip the write.

WR-01: A stale CSV match pass can cancel, advance or fork a remapped import (the race D-10 claims to close)

Status: fixed: requires human verification (concurrency logic) Files modified: fonoteka.go/plugins/golem15/fonoteka/csv_match_job.go, fonoteka.go/plugins/golem15/fonoteka/phase14_jobs_test.go Commit: 41fffae (fonoteka.go) Applied fix:

• The cancel branch, the final preview write and fail() now require status = matching AND (match_job_id IS NULL OR match_job_id = <this job>). The cancel branch used to be unconditional.
• pause() re-dispatches only when the locked row still names this job (or none). Otherwise it ends through lostImport.
• NULL match_job_id is tolerated, the same way the existing start update tolerates it, because a remap always sets a non-NULL id.
• A new subtest covers each of the four branches (cancel, final, fail, pause) against a mid-pass remap to a newer job. All four failed before the fix.

WR-02: The anonymous feedback upload keeps an attacker-chosen file extension in public storage

Files modified: fonoteka.go/plugins/golem15/feedback/classes/submission_store.go, fonoteka.go/plugins/golem15/feedback/feedback_test.go Commit: 5a8b7f8 (sm-feedback-plugin), pointer bump 19add38 (fonoteka.go) Applied fix:

• The disk-name extension now comes from the sniffed content type. The client's extension is kept only when it names that type (jpg/jpeg, png, gif, webp), so ordinary uploads keep their Winter-style names. Anything else, such as x.html, x.svg or no extension, gets the type's own extension. An unknown type is refused.
• file_name is reduced to the client's base name, the same cleaning attach.Store does.
• A new subtest checks x.html, ../../x.svg, noext and Shot.PNG.

WR-03: Credential-test routes are unthrottled oracles for third-party keys (parity security hole)

Files modified: fonoteka.go/plugins/golem15/fonoteka/routes.go, fonoteka.go/plugins/golem15/fonoteka/routes_table_phase14_test.go, fonoteka.go/plugins/golem15/fonoteka/ai_routes_test.go Commit: e3c8599 (fonoteka.go) Applied fix:

• throttle:10,1 added to POST ai-credential/test and POST discogs-credential/test, matching the other sensitive JWT routes. Responses below the limit are unchanged, and the parity suite stays green.
• The route table records the deviation from routes.php.
• TestAICredentialTestRoute made 13 calls as one user, so its stored-credential half now runs as a second user.

WR-04: The "write-only" admin AI key can be redirected to any host by changing base_url

Files modified: fonoteka.go/plugins/golem15/golem/controllers/models_admin_controller.go, fonoteka.go/plugins/golem15/golem/golem_test.go, fonoteka.go/plugins/golem15/golem/README.md Commit: 93b2b05 (sm-golem-plugin), pointer bump 80cafb2 (fonoteka.go) Applied fix:

• FormBeforeUpdate now loads the stored adapter and base_url along with the key. When the key is sent empty or redacted and either value differs, it returns a validation error on api_key ("Re-enter the API key when changing the adapter or base URL."). Base URLs are compared with nil and blank treated as equal.
• The README describes the new rule.
• Not done: the review's secondary suggestion to narrow TrustedMode for admin models. That is a design decision (D-05 accepts http and local model servers), so it is left for a decision note.

WR-05: golem:import-settings silently drops models from a repeater saved with non-sequential keys

Files modified: fonoteka.go/plugins/golem15/golem/console/import_settings.go, fonoteka.go/plugins/golem15/golem/golem_admin_test.go Commit: 9f60b61 (sm-golem-plugin), pointer bump 8d60670 (fonoteka.go) Applied fix:

• An object-shaped repeater now reads every key. Integer keys come first in ascending order, then any other keys in byte order.
• A new case imports {"10":…,"0":…,"2":…} and checks that all three rows arrive in the order Zero, Two, Ten. The old code imported only 2.

WR-06: The G15Office sync job is not idempotent, and a retry files duplicate tasks

Files modified: fonoteka.go/plugins/golem15/feedback/classes/sync_g15office.go, fonoteka.go/plugins/golem15/feedback/jobs.go, fonoteka.go/plugins/golem15/feedback/classes/classes_test.go, fonoteka.go/plugins/golem15/feedback/README.md Commit: e52b1ec (sm-feedback-plugin), pointer bump 98fb4a8 (fonoteka.go) Applied fix:

• SyncG15Office returns early when a submission is already sent with a task id.
• The task id is stored as soon as CreateTask returns it. A retry with a stored id only re-attempts the attachment.
• jobs.go logs a CompleteJob failure (job and submission id only) instead of returning it.
• The attach-failure test now checks:
  • the task id survives the failed attempt;
  • the retry attaches to the same task;
  • a third run on the sent submission sends nothing.
  • In total: 1 task request and 2 attachment requests.
• Behaviour change: a submission that is failed because its attachment failed now carries its g15office_task_id. The task does exist on G15Office.

---

Fixed: 2026-10-04T06:20:00Z Fixer: Claude (gsd-code-fixer) Iteration: 1