fix(#1658): parallel + call doesn't split expanded positional params

[yottahmd]

Resolves #1658

Summary by CodeRabbit

• Bug Fixes

  • Improved parameter handling so expanded variable references are preserved during splitting, fixing incorrect parameter values in parallel steps.
  • Prevented progress updates from overwriting final run status and ensured stale "Running" states are detected and marked appropriately.

• Tests

  • Added end-to-end and unit tests covering conditional parameter quoting, parallel parameter expansion, and stale-running status detection.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds conditional quoting for parameter serialization via a new paramPair.SmartEscape() and uses it for positional params when building sub-DAG param strings. Adds unit and integration tests and lifecycle/staleness handling tweaks in runtime agent/manager code.

Changes

Cohort / File(s)	Summary
Param serialization & tests internal/core/spec/params.go, internal/core/spec/params_test.go	Adds paramPair.SmartEscape() that conditionally quotes values only when necessary; extends unit tests to cover named, positional, variable-reference, empty, spaced, and JSON-like values.
Sub-DAG param construction internal/core/spec/step.go	Uses SmartEscape() for unnamed (positional) params and retains Escaped() for named params when constructing string-style params: for call: sub-DAGs; clarifying comments added about runtime re-splitting.
Integration tests (parallel/call) internal/intg/parallel_test.go	Adds end-to-end test TestIssue1658_ParallelCallExpandedParamsSplitting covering various expanded-param scenarios. Note: the test function is duplicated in the file (two identical declarations), which will cause a compile error.
Agent progress draining internal/runtime/agent/agent.go	Adjusts progress goroutine lifecycle: introduces progressDrained flag and explicit drain/close sequence to avoid concurrent overwrites of final status by in-flight progress updates.
Stale-running status check internal/runtime/manager.go, internal/runtime/manager_test.go	When socket read fails but persisted status is Running, calls checkAndUpdateStaleRunningStatus to possibly downgrade to Failed; adds unit test subcase GetLatestStatusMarksStaleRunningAsFailed exercising this path.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat(cmd): add tree-style detailed result #1545 — touches internal/runtime/agent/agent.go; related to agent lifecycle/progress handling.
• fix(api): handle JSON params properly #1552 — modifies internal/core/spec/params.go handling for param parsing/serialization; related to paramPair behavior.

🚥 Pre-merge checks | ✅ 3 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (66 files): ⚔️ README.md (content) ⚔️ api/v1/api.gen.go (content) ⚔️ api/v1/api.yaml (content) ⚔️ internal/agent/api.go (content) ⚔️ internal/agent/bash.go (content) ⚔️ internal/agent/bash_test.go (content) ⚔️ internal/agent/hooks.go (content) ⚔️ internal/agent/hooks_test.go (content) ⚔️ internal/agent/loop.go (content) ⚔️ internal/agent/loop_test.go (content) ⚔️ internal/agent/navigate.go (content) ⚔️ internal/agent/navigate_test.go (content) ⚔️ internal/agent/patch.go (content) ⚔️ internal/agent/patch_test.go (content) ⚔️ internal/agent/session.go (content) ⚔️ internal/agent/system_prompt.go (content) ⚔️ internal/agent/system_prompt.txt (content) ⚔️ internal/agent/system_prompt_test.go (content) ⚔️ internal/agent/types.go (content) ⚔️ internal/auth/role.go (content) ⚔️ internal/auth/role_test.go (content) ⚔️ internal/auth/user_test.go (content) ⚔️ internal/cmd/context.go (content) ⚔️ internal/cmd/startall.go (content) ⚔️ internal/cmd/startall_test.go (content) ⚔️ internal/cmn/config/config.go (content) ⚔️ internal/cmn/config/config_test.go (content) ⚔️ internal/cmn/config/definition.go (content) ⚔️ internal/cmn/config/loader.go (content) ⚔️ internal/cmn/config/loader_test.go (content) ⚔️ internal/cmn/schema/config.schema.json (content) ⚔️ internal/core/spec/params.go (content) ⚔️ internal/core/spec/params_test.go (content) ⚔️ internal/core/spec/step.go (content) ⚔️ internal/intg/parallel_test.go (content) ⚔️ internal/runtime/agent/agent.go (content) ⚔️ internal/runtime/manager.go (content) ⚔️ internal/runtime/manager_test.go (content) ⚔️ internal/service/frontend/api/v1/api.go (content) ⚔️ internal/service/frontend/api/v1/audit.go (content) ⚔️ internal/service/frontend/api/v1/export_test.go (content) ⚔️ internal/service/frontend/api/v1/resources.go (content) ⚔️ internal/service/frontend/api/v1/services.go (content) ⚔️ internal/service/frontend/api/v1/webhooks.go (content) ⚔️ internal/service/frontend/api/v1/webhooks_test.go (content) ⚔️ internal/service/frontend/api/v1/workers.go (content) ⚔️ internal/service/frontend/auth/builtin.go (content) ⚔️ internal/service/frontend/auth/middleware_test.go (content) ⚔️ internal/service/oidcprovision/rolemapper.go (content) ⚔️ internal/service/oidcprovision/rolemapper_test.go (content) ⚔️ internal/service/resource/service.go (content) ⚔️ internal/service/resource/store.go (content) ⚔️ internal/service/resource/store_test.go (content) ⚔️ internal/test/helper.go (content) ⚔️ ui/src/App.tsx (content) ⚔️ ui/src/api/v1/schema.ts (content) ⚔️ ui/src/components/ProtectedRoute.tsx (content) ⚔️ ui/src/contexts/AuthContext.tsx (content) ⚔️ ui/src/features/dashboard/components/MiniResourceChart.tsx (content) ⚔️ ui/src/features/system-status/components/ResourceChart.tsx (content) ⚔️ ui/src/menu.tsx (content) ⚔️ ui/src/pages/api-keys/APIKeyFormModal.tsx (content) ⚔️ ui/src/pages/audit-logs/index.tsx (content) ⚔️ ui/src/pages/system-status/index.tsx (content) ⚔️ ui/src/pages/users/UserFormModal.tsx (content) ⚔️ ui/src/pages/webhooks/index.tsx (content) These conflicts must be resolved before merging into main.	Resolve conflicts locally and push changes to this branch.
Out of Scope Changes check	❓ Inconclusive	Changes to agent.go and manager.go address reliability of status tracking during concurrent operations and stale status detection, which are tangential to the main param-splitting issue. These appear to be related bug fixes but may exceed the primary scope.	Clarify whether concurrent status tracking improvements and stale status checks are part of issue #1658 or separate fixes that should be in a different PR.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically identifies the issue being fixed (parallel + call doesn't split expanded positional params) and references issue #1658, directly matching the main objective of the pull request.
Linked Issues check	✅ Passed	The code changes implement the core objective of correctly splitting expanded positional params in parallel+call scenarios. SmartEscape() handles conditional quoting to preserve variable references, buildStepSubDAG applies SmartEscape to unnamed params, and integration tests validate the splitting behavior.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 1658-fix

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch 1658-fix
• Create stacked PR with resolved conflicts
• Post resolved changes as copyable diffs in a comment

---

No actionable comments were generated in the recent review. 🎉

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[yottahmd]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[codecov]

Codecov Report

❌ Patch coverage is 78.26087% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.47%. Comparing base (228e4ae) to head (7bb1ee0).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/runtime/agent/agent.go	57.14%	2 Missing and 1 partial ⚠️
internal/runtime/manager.go	0.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1665      +/-   ##
==========================================
- Coverage   70.18%   69.47%   -0.71%     
==========================================
  Files         344      345       +1     
  Lines       38260    38796     +536     
==========================================
+ Hits        26854    26955     +101     
- Misses       9271     9391     +120     
- Partials     2135     2450     +315     

Files with missing lines	Coverage Δ
internal/core/spec/params.go	89.60% <100.00%> (+0.48%)	⬆️
internal/core/spec/step.go	85.43% <100.00%> (+0.07%)	⬆️
internal/runtime/manager.go	51.33% <0.00%> (-2.34%)	⬇️
internal/runtime/agent/agent.go	55.94% <57.14%> (-12.55%)	⬇️

... and 33 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5c68c5...7bb1ee0. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.