fix(sltb): stop schedule-pickup stalling when the trader sends no response - #38
Open
Aravinda-HWK wants to merge 1 commit into
Open
fix(sltb): stop schedule-pickup stalling when the trader sends no response#38Aravinda-HWK wants to merge 1 commit into
Aravinda-HWK wants to merge 1 commit into
Conversation
…ponse
The `trader_response_gateway` edges both referenced `suggest.response`:
e_trader_proposes schedule.schedule_outcome != 'propose_alternative' || suggest.response != 'accept'
e_trader_accepts schedule.schedule_outcome == 'propose_alternative' && suggest.response == 'accept'
On the first pass through `trader_suggest` the `response` control is hidden
(it only shows once the officer has proposed an alternative), so the submit
payload can arrive without it. `response?` and `trader_notes?` are both
optional mappings, so when neither is present the `suggest` namespace is
never created at all.
The engine compiles each condition with `expr.Env(WorkflowVariables)`, which
rejects an unknown *root* name outright — `unknown name suggest` — before the
`&&`/`||` can short-circuit. `handleGatewayNode` returns that error, and the
workflow is left Running with no pending activity and no way forward. Every
later submit then fails with "cannot find pending activity", because the
activity was already consumed by the submit that stalled it.
Restructure the gateway so no condition can reference a namespace that may be
absent, and so it always has somewhere to go:
e_trader_first_pass schedule.schedule_outcome != 'propose_alternative' -> officer_review
e_trader_accepts suggest.response == 'accept' -> end
e_trader_proposes (no condition — default passthrough) -> officer_review
`schedule` always exists at this gateway (the form requires suggested_date /
suggested_time whenever response is absent or 'propose', and officer_review
writes schedule_outcome unconditionally), and a missing *key* on an existing
map evaluates to nil rather than erroring. `suggest.response` is now only
reached on the second pass, where the form requires it. The unconditional
third edge means an exclusive gateway can no longer stall here.
Verified against the real evaluator (go-temporal-workflow) across all five
reachable states: first pass with and without `response`, and second pass with
`accept`, `propose`, and `response` missing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
Shall we keep this without merging? Because the previous logic was also right. Once we get a response to this issue https://github.com/LSFLK/lsf-govtech-tnsw/issues/36, we can either close or merge this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Observed in
nsw-stagingon tasksltb_3b_schedule_pickup:94650116-e7a4-40bb-9fdb-9c6ac2e56231(consignment "SLTB Sampaling Date Change Option"). The trader's submit appeared to fail, and every retry returned:That error is a symptom. The first submit actually succeeded and completed the activity — then the workflow stalled and never scheduled anything else:
Status
Running, Pending Activities0, forever. All later submits hit the already-consumed activity, which is where the misleading error comes from.Root cause
Note the payload has no
responsekey.On the first pass through
trader_suggesttheresponsecontrol is hidden — the uiSchema only shows it onceofficer_outcome == 'propose_alternative', i.e. after an officer review. So a first-pass submit legitimately carries noresponse. Bothresponse?andtrader_notes?are optional output mappings, so when neither is present thesuggestnamespace is never created.Both edges out of
trader_response_gatewaythen referenced it:EvaluateConditioncompiles withexpr.Compile(cond, expr.Env(WorkflowVariables), expr.AsBool()). expr rejects an unknown root name at compile time, before&&/||can short-circuit:handleGatewayNodedoesreturn erron that, so the gateway can neither route nor recover.A missing key on a map that does exist is fine — it evaluates to nil. Only the absent root name is fatal. That is why the flow works whenever the form happens to send
response(JSONForms materialising the"default": "propose") and breaks when it doesn't. Same artifact version, same deployment — purely payload-dependent, which is what made it look intermittent.The fix
Restructure the gateway so no condition can reference a possibly-absent namespace, and so it always has an exit:
e_trader_first_passschedule.schedule_outcome != 'propose_alternative'officer_reviewe_trader_acceptssuggest.response == 'accept'ende_trader_proposesofficer_reviewWhy this is safe:
schedulealways exists at this gateway. The form requiressuggested_date+suggested_timewheneverresponseis absent or'propose'(both map intoschedule.*), andofficer_reviewwritesschedule_outcomevia a non-optional mapping.schedule, sosuggestis never compiled against a first-pass context.schedule_outcomebeing an absent key is fine — nil !='propose_alternative'-> true.response(allOf->if officer_outcome == 'propose_alternative' then required: [response]), sosuggestexists.EXCLUSIVE_SPLITcan no longer hitno matching conditions found at exclusive gateway. Belt and braces: even a second-pass payload that somehow omitsresponsenow routes back toofficer_reviewinstead of hanging.Behaviour for every valid path is unchanged — this only removes the stall.
versionbumped2->3.Verification
Driven by the patched
workflow.jsonitself, evaluated through the realEvaluateConditionfromgo-temporal-workflow, across all five reachable states:Against the pre-fix edges, case 1 gives
ERROR: unknown name suggest— reproducing the stall exactly.Not fixed here — worth separate follow-ups
schedule_gatewayhas the same shape. Its two edges (== 'approve',== 'propose_alternative') exactly cover the officer form'soneOf, andschedule_outcomeis required with a non-optional mapping, so it is safe today — but it has no fallback edge, so adding a third outcome would stall the same way. Left alone to keep this PR to the actual defect; happy to harden it if you'd prefer.Runningwith no pending activity and no error surfaced to the user. Worth an issue ongo-temporal-workflow— it affects every flow, not just this one.nsw-staging:94650116-..., plus6f32b868-...(2026-07-07) andc979d3b8-...(2026-07-31).audit_logsis empty (0 rows) innsw_stagingand the app logs carry no actor field, so submits had to be attributed via consignment ownership. A real gap for triage.🤖 Generated with Claude Code