Skip to content

feat(ext-api): InternalApiController — drop .join(), true fire-and-forget#1306

Merged
zbnerd merged 2 commits into
developfrom
feature/ext-api-controller-fire-forget
Jun 18, 2026
Merged

feat(ext-api): InternalApiController — drop .join(), true fire-and-forget#1306
zbnerd merged 2 commits into
developfrom
feature/ext-api-controller-fire-forget

Conversation

@zbnerd

@zbnerd zbnerd commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Summary

Removes `.join()` from `InternalApiController` trigger endpoints (lines 83, 123) so the controller returns 202 immediately without blocking the worker thread on phase completion. The CF chain runs async on the default executor; errors are observed via the existing `whenComplete` blocks inside `scheduler.triggerDailyRefresh`/`triggerPhase`.

Audit Reference

`docs/05_Reports/2026-06-18-blocking-audit.md` lines 61-74 (ext-api sites). This PR addresses Sub-PR 3 from the recommended path forward.

Changes (2 commits)

`module-external-api/runstatus/InternalApiController.kt`

  • Line 83: `executor.submit { scheduler.triggerDailyRefresh(runId).join() }` → `executor.submit { scheduler.triggerDailyRefresh(runId) }`
  • Line 123: `executor.submit { scheduler.triggerPhase(...).join() }` → `executor.submit { scheduler.triggerPhase(...) }`

`module-external-api/runstatus/InternalApiControllerTest.kt`

  • 2 new tests: `trigger daily returns 202 immediately without blocking` and `trigger phase returns 202 immediately without blocking`
  • Tests use a capturing-Future pattern: override `AbstractExecutorService.execute(Runnable)` to capture the `Future<*>` returned by `submit`, assert `future.get(500ms)` succeeds. With `.join()` present, the future never completes → TimeoutException → assertion fails.

CI gate

New `ExtApiBlockingPrimitiveGateTest` walks `module-external-api/{runstatus,scheduler}/` for `.join()`, `runBlocking{`, `Task.join()`, `Thread.sleep(`. Allowlist: `ExternalApiScheduler.kt` `runBlocking` (documented acceptable per async-patterns.md — VT carrier bridge), coroutine `*Job.join()` (suspend function, not blocking primitive), comments, `@Deprecated`, legitimate `.get()` in `.thenApply`/`allOf` chains.

Out of scope (follow-up)

  • `ExternalApiScheduler.kt:188` `runBlocking { ocidLookupPhase.execute(...) }` — acceptable per async-patterns.md (VT carrier is fine). Removing it would require converting `OcidLookupPhase.execute()` from `suspend` to `CompletableFuture` (larger refactor).

Verification

  • `./gradlew compileKotlin compileJava --continue` — clean
  • `./gradlew :module-external-api:test` — 30 tests pass (28 existing + 2 new)
  • `./gradlew :module-infra:test --tests "PgmqBlockingPrimitiveGateTest"` — pass
  • CI grep gate: 0 violations
  • (Load test SKIPPED per user direction)

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@zbnerd zbnerd merged commit f82bd83 into develop Jun 18, 2026
@zbnerd zbnerd deleted the feature/ext-api-controller-fire-forget branch June 18, 2026 22:08
@zbnerd

zbnerd commented Jun 19, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

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