[openipc-ai]

Follow-up to #538.

The web role refreshed every published report's board unit (refreshReportUnits) before it started listening. #538 added a private-dump count to every unit, so the first start rewrote all ~82 of them. On production that took longer than openipc-deploy's 90-second /up gate, and the deploy rolled back twice. Dev has fewer reports and passed. It went through only after the refresh was run once in a separate container with the new image.

What changes

• The refresh now starts in the background once the routes are registered, using the role's background context. /up answers at once.
• The role's stop cancels it and waits for it before giving up the web lock, so a second process can't race it.
• A refresh cut short by a stop is finished by the next start, because a unit whose files are missing is rewritten (unchanged checks the files).

Meanwhile the board pages serve the units as they were, exactly as they do between a review and its refresh.

Tests: vet passes, and ./cmd/... and ./internal/boards pass.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Refresh published report board units after web startup

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Move startup refresh into the background so rewriting report units cannot delay /up.

Diagram

sequenceDiagram
    participant Web as Web role
    participant Lock as Web lock
    participant Routes as Route mux
    participant Refresh as Refresh worker
    participant Units as Board units
    participant Server as HTTP server
    Web->>Lock: claim
    Web->>Routes: register handlers
    Web->>Refresh: start in background
    Web->>Server: begin listening
    Server-->>Routes: serve /up
    Refresh->>Units: reconcile reports
    Web->>Refresh: cancel on stop
    Refresh-->>Web: finish
    Web->>Lock: release

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Refresh as a separate deployment job

• ➕ Keeps reconciliation entirely outside the web process.
• ➖ Adds deployment coordination and another step that must run after relevant changes.

Recommendation: Keep the background startup refresh: it preserves automatic reconciliation on every start without blocking readiness, while waiting before lock release protects shutdown ordering. A separate deployment job would add operational complexity for this one startup task.

Files changed (1) +13 / -1

Bug fix (1) +13 / -1

main.goRun report-unit refresh after route registration +13/-1

Run report-unit refresh after route registration

• Moves the startup refresh from the blocking setup path to a goroutine using the web role's background context. The stop function cancels and waits for that goroutine before releasing the web lock.

service/cmd/openipc/main.go

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Board files disappear during startup ✓ Resolved

Description

The new goroutine lets refreshReportUnits run while board requests are served, but applyOne
removes each unit directory before writing replacement files and updating its database rows. When
startup rewrites units, the board API can still return artifact URLs from the old rows while nginx
finds no files; cancellation between directory removal and transaction commit can leave those URLs
broken until another refresh.

Code

service/cmd/openipc/main.go[R570-572]

+	go func() {
+		defer close(refreshed)
+		refreshReportUnits(bg, cfg, log, pool)

Evidence

The added goroutine moves an existing non-atomic replacement operation into the period when the web
role serves requests.

service/cmd/openipc/main.go[569-579]
service/internal/boards/contributions.go[270-299]
service/internal/boards/api.go[490-500]
deploy/nginx/sites-available/org.openipc[411-414]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Startup refresh now runs while requests are served, but replacing a unit deletes its files before replacing its database artifacts. Readers can receive URLs for missing files, and cancellation can leave them missing.
## Fix Focus Areas
- service/cmd/openipc/main.go[569-579]
- service/internal/boards/contributions.go[270-299]
- service/internal/boards/api.go[490-500]
## Recommended Fix
Stage replacement files without removing files referenced by existing artifact rows. Switch the database references to the staged files when they are ready, and retire old files only after they are no longer referenced; ensure cancellation preserves the previously served version.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Rejected reports return to public boards ✓ Resolved

Description

refreshReportUnits reads the published-report list before acquiring the importer lock, so the new
startup goroutine can apply an older list after a rejection and its review-triggered refresh have
completed. If that list includes a report subsequently rejected on an existing board, the stale
apply recreates its unit and copied photos or text under anonymously served /board-files/ until
another refresh removes them.

Code

service/cmd/openipc/main.go[R570-572]

+	go func() {
+		defer close(refreshed)
+		refreshReportUnits(bg, cfg, log, pool)

Evidence

The startup snapshot is taken before the apply lock, while rejection commits before its callback
refresh. A review refresh can therefore prune the rejected unit before startup applies its stale
list; the importer copies listed files back to the directory nginx serves without authentication.

service/cmd/openipc/club.go[66-107]
service/internal/boards/contributions.go[172-190]
service/internal/club/api.go[748-763]
service/internal/reports/club.go[899-922]
service/internal/boards/contributions.go[270-299]
deploy/nginx/sites-available/org.openipc[411-428]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A startup refresh can apply a published-report snapshot after a later review refresh, restoring a rejected report's public board copies.
## Fix Focus Areas
- service/cmd/openipc/main.go[569-573]
- service/cmd/openipc/club.go[66-107]
- service/internal/boards/contributions.go[172-190]
## Recommended Fix
Serialize the published-report query and its application under the same lock used by review-triggered refreshes, so an older snapshot cannot be applied after a newer one. Preserve that ordering across both startup and review refresh paths.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Shutdown can stall behind a review ✓ Resolved

Description

The new shutdown callback waits unconditionally for the startup refresh, even though that refresh
can be blocked on the importer’s non-cancellable mutex after its background context is cancelled. A
review-triggered refresh uses a context detached from its request and can hold that mutex beyond the
server's 20-second shutdown deadline, leaving the web process waiting before it releases its lock.

Code

service/cmd/openipc/main.go[578]

+		<-refreshed

Evidence

Shutdown cancels the background context but then waits without a deadline. The startup refresh can
be queued on a mutex that does not observe that cancellation, while review callbacks deliberately
use a detached request context.

service/cmd/openipc/main.go[341-353]
service/cmd/openipc/main.go[575-579]
service/internal/boards/contributions.go[172-185]
service/internal/club/api.go[752-763]
service/cmd/openipc/club.go[27-32]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Shutdown waits for the startup refresh, but cancelling that refresh cannot interrupt its wait for the importer mutex while a review refresh holds it.
## Fix Focus Areas
- service/cmd/openipc/main.go[575-579]
- service/internal/boards/contributions.go[172-185]
## Recommended Fix
Replace the mutex wait with context-aware serialization so a queued startup refresh exits on cancellation. Keep the shutdown wait and web-lock release ordered after any startup refresh that actually entered the importer has finished.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[openipc-ai]

All three Qodo findings were real; fixed in 441179b:

1. Board files disappear during startup: a rewrite now stages its files under Root/.staging/<unit> and swaps the directory in (two renames) only after the unit's rows commit. A failure before the commit removes the stage and leaves the unit as it was. Covered by TestARewriteSwapsTheUnitInWhole.
2. Rejected reports return to public boards: refreshReportUnits now goes through Importer.RefreshReportUnits, which calls its loader (PublishedTexts) only once the apply's lock is held. That covers both startup and review refreshes, so an older list can't be applied after a newer one. Covered by TestARefreshReadsItsListUnderTheLock.
3. Shutdown can stall behind a review: the stop function waits at most 10 s for the refresh, then logs and releases the web lock. What the refresh left undone, the next start finishes (unchanged rewrites a unit whose files are missing).