Phase 1F: Cut over Order→Manufacturing calls to REST with feature flag

[devin-ai-integration]

Implemented: Feature-flagged REST cutover for the 3 Order→Manufacturing dispatcher calls (Sub-Phase 1F of the decoupling plan).

• Added manufacturing.properties with manufacturing.api.use-rest=false (default: dispatcher path, no behavior change)
• Added ManufacturingApiConfig utility to read the feature flag and base URL
• Added ManufacturingRestClient in the Order module with retry logic (1 retry, 1s delay) and circuit breaker (fail-fast after 5 consecutive failures, 30s reset)
• Modified CheckOutHelper.java (~line 753): feature-flagged routing for createProductionRunFromConfiguration
• Modified OrderServices.java (~lines 1359, 1460): feature-flagged routing for createProductionRunForMktgPkg at both call sites
• All REST failures fall back to the dispatcher path with a warning logged

Explanation

With the flag defaulting to false, existing behavior is completely unchanged. When toggled to true, the Order module calls the Phase 1E REST endpoints instead of dispatcher.runSync, with automatic fallback to the dispatcher on any REST error or when the circuit breaker is open.

⚠️ Items for reviewer attention:

1. config object not transmitted via REST — In CheckOutHelper, the dispatcher path passes a ProductConfigWrapper object as inputMap.put("config", config). The REST client passes configId=null and relies on orderId/orderItemSeqId instead. Verify the REST endpoint can reconstruct the configuration from the order context alone, or this path will silently produce different results.

2. No auth/session forwarding in REST client — ManufacturingRestClient uses raw HttpURLConnection without forwarding session cookies or credentials. The Phase 1E REST endpoints require a valid userLogin in the HTTP session (getAuthenticatedUser). The REST path will likely get 401s unless session propagation is added or the auth model is changed.

3. Naive JSON parsing — parseJsonResponse splits on , without respecting quoted strings. Works for the simple Manufacturing API responses but would break on values containing commas.

4. Potential NPE — readResponse() accesses conn.getErrorStream() which can return null for some error conditions, causing an NPE in the InputStreamReader constructor.

5. Cross-module import direction — Order now imports ManufacturingApiConfig from the manufacturing module, which is a dependency in the "wrong" direction for the decoupling goal. Consider moving the config class to a shared location or into the Order module itself.

Link to Devin session: https://jack-meigel.devinenterprise.com/sessions/0d65aa5ff5b1494f945f5b308779e22a
Requested by: @cogjack

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[devin-ai-integration]

Devin Review found 3 potential issues.

View 5 additional findings in Devin Review.

[devin-ai-integration]

🔴 NullPointerException in readResponse when getErrorStream() returns null

HttpURLConnection.getErrorStream() can return null when the server sends an error status code (4xx/5xx) but does not include a response body. At line 226, the null is passed directly to new InputStreamReader(null, StandardCharsets.UTF_8), which throws a NullPointerException. This NPE propagates up to doPost where it is caught as a generic IOException, masking the root cause. Since this REST client is used for production run creation during order processing, it can cause confusing error messages during order fulfillment.

Null-safety fix needed for getErrorStream()

The fix should check if getErrorStream() returns null before using it. A typical pattern is:

InputStream errStream = conn.getErrorStream();
InputStream stream = (status >= 400 && errStream != null) ? errStream : conn.getInputStream();

Suggested change

	try (BufferedReader br = new BufferedReader(new InputStreamReader(
	status >= 400 ? conn.getErrorStream() : conn.getInputStream(),
	StandardCharsets.UTF_8))) {
	try (BufferedReader br = new BufferedReader(new InputStreamReader(
	status >= 400 && conn.getErrorStream() != null ? conn.getErrorStream() : conn.getInputStream(),
	StandardCharsets.UTF_8))) {

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔴 REST path for createProductionRunFromConfiguration always fails, poisoning shared circuit breaker

The REST path in CheckOutHelper.java:767 passes null for configId and cannot send the config (ProductConfigWrapper) object via REST. Looking at the service implementation at ProductionRunServices.java:2255-2256, when both config and configId are null, it immediately returns an error ("ManufacturingConfigurationNotAvailable"). This means the REST call for createProductionRunFromConfiguration always fails when the feature flag is on. While the fallback to the dispatcher at line 774-776 works correctly, each failure increments the shared CONSECUTIVE_FAILURES counter in ManufacturingRestClient.java:57. After 5 such failures, the circuit breaker opens globally, causing the separate createProductionRunForMktgPkg REST calls (which could succeed) to be short-circuited too.

Prompt for agents

The REST path for createProductionRunFromConfiguration in CheckOutHelper.java:756-777 is fundamentally broken because it cannot transmit the ProductConfigWrapper object (stored in inputMap as "config") over REST. The service at ProductionRunServices.java:2255-2263 requires either a config object or a configId - but the REST client passes null for configId and cannot pass the config object. This means every REST call always fails and falls back to the dispatcher.

The problem is compounded because ManufacturingRestClient uses a single shared circuit breaker (CONSECUTIVE_FAILURES / CIRCUIT_OPEN_SINCE at ManufacturingRestClient.java:57-58) for ALL endpoints. The always-failing createProductionRunFromConfiguration calls increment the failure count, which can open the circuit breaker and block the unrelated createProductionRunForMktgPkg calls.

Possible approaches:
1. Skip the REST path entirely for createProductionRunFromConfiguration until the configId-based loading is implemented on the service side (see the TODO at ProductionRunServices.java:2260)
2. Separate the circuit breaker state per endpoint so failures on one path don't affect the other
3. Both of the above

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🟡 Unvalidated quantity parameter enables JSON injection in REST request body

At line 89, the quantity string parameter is appended directly into the JSON body without quotes or validation: json.append(",\"quantity\":").append(quantity). Since this is a public static method, any caller could pass a malformed string like "1, \"injected\": \"val\"" to inject arbitrary JSON fields into the request body. While the current caller in CheckOutHelper.java:760-762 safely uses BigDecimal.toPlainString(), the public API has no guard against misuse.

Suggested change

	if (quantity != null) {
	json.append(",\"quantity\":").append(quantity);
	if (quantity != null) {
	json.append(",\"quantity\":").append(new java.math.BigDecimal(quantity).toPlainString());

---

Was this helpful? React with 👍 or 👎 to provide feedback.