th-fc07ac: the workflow step-attempt cap never fired — shadowed binding fed it 0

[brentrager]

The bug

handler.rs carried the step-attempt-cap capture block twice, back to back, with identical comments. The two copies differed in exactly one field:

let (cap_workflow, cap_step_before, cap_attempts) = match workflow.as_ref() {
    Some(wt) => (…, loaded_attempts),                          // durable conversation metadata
…
let (cap_workflow, cap_step_before, cap_attempts) = match workflow.as_ref() {
    Some(wt) => (…, state.session_step_attempts(session_id)),  // per-pod session map — shadows the above

Rust shadowing means the second won. That silently reverted th-c12df5, which had deliberately moved the workflow pointer and its attempt count onto durable conversation storage — the comment ten lines above the duplicate still explains why: the per-pod map "reset them to step 0 every turn, freezing the workflow at the first step so the judge/cap could never advance it".

It was worse than the pre-th-c12df5 behaviour. The two stores use different metadata keys: persist_workflow_step writes workflowStepAttempts, while the session accessor read stepAttempts — a key nothing ever writes. Its writer, set_session_step_attempts, had no callers anywhere in the repo. So apply_step_cap was fed a permanent 0, next_attempts never reached WORKFLOW_STEP_ATTEMPT_CAP, and a workflow step the judge never accepts could loop forever — precisely the pathological-visitor case (th-d57a1d) the cap exists to bound.

Which source is correct

The durable one, unambiguously. Not a judgement call:

• th-c12df5 exists specifically to move this off per-pod state, and its rationale is in the comment directly above the block.
• The per-pod value is not a live superset of the durable one (so no max(loaded, live) reconciliation is warranted) — it is never written at all, and reads a key no writer produces.
• Keying the cap to per-pod state would let a client reset its own cap by reconnecting.

The fix

Keep loaded_attempts, delete the duplicate block — and remove AppState::session_step_attempts / set_session_step_attempts rather than leave them. Both were vestigial from the pre-th-c12df5 design, neither had a live caller, and the getter's doc comment still advertised itself as feeding the cap. Leaving them in place is what let the wrong source get wired in; removing them makes the mistake unavailable rather than merely un-made. Also clears three unused variable warnings emitted on every build.

Test

step_attempt_cap_counts_across_reconnects_and_force_advances drives the real per-turn pipeline — load_workflow_step → apply_step_cap → persist_workflow_step — reloading from storage each turn as a reconnect or pod hop would, with a judge that always holds, and asserts the step force-advances exactly at the cap with the counter reset.

Verified it has teeth by mutation: replacing the loaded count with a hardcoded 0 (the bug) fails it with a step the judge never advances must force-advance on the capth turn. Reverted before committing.

The existing workflow_step_persists_to_conversation_metadata_and_resumes already covered the storage round-trip, so this adds the composition case it did not.

Verification

cargo fmt --all -- --check clean; cargo test --workspace 647 passed, 0 failed, and zero unused variable warnings. The dynamodb suite passed too (fixed on main by #366, so the rust lane is trustworthy again). anchor-guard passes naming @smooai/smooth-operator (stamped trees touched: rust, rust/smooth-operator-server).

Changeset is minor rather than patch — this removes two pub methods from AppState.

Found while triaging PR #333 under th-b093c7.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YbN45JeWDbcjvFqGJvmVD3

[changeset-bot]

🦋 Changeset detected

Latest commit: 09354f9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@smooai/smooth-operator	Minor
@smooai/smooth-operator-web-chat-example	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR