fix(workflows): namespace nested descendant step ids in loops/fan-out

[Noor-ul-ain001]

Summary

• while/do-while loop bodies and fan-out templates namespace nested step ids per iteration/item so logs and state.step_results entries stay unique — but the namespacing only rewrote the id of the immediate child step, not any descendant nested deeper (e.g. a shell step inside an if inside a while body, or inside a fan-out template's if/switch branch). That grandchild kept its bare, unnamespaced id across every iteration/item, so each iteration/item silently overwrote the previous one's entry in state.step_results under that same key — only the last iteration's or item's result for that nested step ever survived, and no per-iteration/per-item record of it ever existed.
• This is also a correctness gap beyond bookkeeping: nested/template step ids are deliberately exempted from the workflow's global id-uniqueness validation, on the assumption that runtime namespacing makes any collision safe. Since only the top-level child was actually namespaced, a step nested one level deeper could collide with an unrelated step of the same id elsewhere in the workflow and silently overwrite its result.
• Fix: add _rename_step_tree_ids, which recursively rewrites every id in a step's subtree (walking then/else/steps/default/cases.* — the same nesting keys overlays/merge.py walks for step-tree attribution) and returns a {new_id: original_id} map. Both the while/do-while loop body and fan-out's run_item now use this helper instead of renaming only the top-level id, and alias every renamed descendant's result back to its original id (mirroring the existing single-level aliasing) so sibling steps within the same iteration/item and code reading steps.<id>.output after the loop/fan-out still see that iteration's/item's value.

Test plan

• Added test_while_loop_namespaces_nested_descendant_steps and test_fan_out_namespaces_nested_descendant_steps to tests/test_workflows.py::TestWorkflowEngine: a shell step nested inside an if inside a while body (and inside a fan-out template) gets a distinct namespaced state.step_results entry per iteration/item, while the unprefixed key still holds the latest value.
• Verified both fail without the fix (test-the-test): the namespaced keys (retry-loop:leaf:1, fan:leaf:0, etc.) were simply absent, and step_results only ever held the last iteration's/item's bare-keyed entry — reproducing the exact bug.
• Ran the full tests/test_workflows.py suite: 926 passed, 20 pre-existing Windows symlink-elevation failures (need admin rights, unrelated to this change), 7 skipped. All While/DoWhile/FanOut/FanOutConcurrency tests pass, including the concurrent-execution and per-thread context isolation tests.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

https://claude.ai/code/session_01PJHJ2dHP2RVCNncHqN8Qm9

[Copilot]

🟡 Changes recommended

Initial loop iterations remain unnamespaced, while delayed and shared aliases break sibling references and concurrent fan-out isolation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds recursive runtime namespacing for nested workflow steps in loops and fan-out templates.

Changes:

• Adds recursive descendant ID rewriting and aliasing.
• Adds while and fan-out regression tests.
• Preserves namespaced execution results.

File summaries

File	Description
src/specify_cli/workflows/engine.py	Recursively namespaces nested step IDs.
tests/test_workflows.py	Tests nested loop and fan-out results.

Review details

Suppressed comments (1)

src/specify_cli/workflows/engine.py:1447

• These aliases are created only after the entire renamed subtree finishes. If a branch contains step a followed by step b that references steps.a, b executes before a is aliased and reads the previous iteration's value (or no value), whereas both steps previously used their bare IDs. Alias each descendant immediately after that descendant completes so intra-branch references retain their existing semantics.

                            for new_id, orig_id in id_map.items():
                                if new_id in context.steps:
                                    self._record_result(
                                        context, state, orig_id,
                                        context.steps[new_id],
                                    )

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[mnriem]

Please address Copilot feedback