feat: add Slurm command client and renderer

[nabinchha]

📋 Summary

Adds the first independently reviewable #868 slice: a structured Slurm command boundary and deterministic thin batch renderer. This gives M0 a plan-to-script proof and supplies the scheduler primitives needed by #867 without pulling allocation runtime or persistence policy into this PR.

🔗 Related Issue

Part of #868

🔄 Changes

• Add typed sbatch, squeue, sacct, scancel, and bounded sinfo operations with exact argv construction, minimal environments, timeouts, normalized errors, and returned-selector correlation.
• Add strict parsers for array submissions, active and terminal scheduler states, exit codes, accounting absence, and configured GPU GRES values.
• Add a deterministic generation-script renderer with typed directives, shell-safe values, pinned host-tool lookup, checksum verification, shard/attempt paths, and no user-provided executable shell text.
• Extend deterministic Slurm fakes and sanitized golden scripts to mirror the production command formats.
• Verify launcher behavior through focused boundary tests and isolated built-wheel imports.

🔍 Attention Areas

⚠️ Reviewers: Please pay special attention to the following:

• client.py — managed-selector correlation and the --export=NIL submission boundary.
• renderer.py — directive/resource semantics and checksum-before-source ordering.

🧪 Testing

• make test passes — 4,564 passed, 1 skipped
• Unit tests added/updated — 129 focused launcher/fake tests; 99% launcher coverage with all public paths covered
• E2E tests added/updated — N/A; this local/fake slice extends deterministic scheduler and golden-script coverage
• make check-all passes
• make test-slurm-wheel-install passes
• Five consecutive ten-pass review cycles completed with no additional findings

✅ Checklist

• Follows commit message conventions
• Commits are signed off (DCO)
• Architecture docs updated — N/A; this implements the existing reviewed plan and contracts

[greptile-apps]

Greptile Summary

The PR adds a structured Slurm command boundary and deterministic batch-script renderer.

• Adds typed clients and strict parsers for submission, queue, accounting, cancellation, and GPU inventory commands.
• Adds deterministic, shell-safe generation scripts with checksum verification and isolated attempt directories.
• Adds production runner behavior that preserves a nonempty caller PATH while restricting the remaining child environment.
• Adds focused launcher tests, deterministic Slurm fakes, and rendered-script golden fixtures.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the current runner preserves a nonempty ambient PATH and falls back to os.defpath when PATH is absent or empty.

Important Files Changed

Filename	Overview
packages/data-designer-slurm/src/data_designer/slurm/launcher/client.py	Adds typed Slurm command construction, output correlation, and normalized command failures without an eligible follow-up defect.
packages/data-designer-slurm/src/data_designer/slurm/launcher/parsing.py	Adds strict parsers for scheduler identities, states, exit codes, submission receipts, and GPU resources.
packages/data-designer-slurm/src/data_designer/slurm/launcher/renderer.py	Adds deterministic batch rendering with validated directives, quoted values, checksums, and attempt isolation.
packages/data-designer-slurm/src/data_designer/slurm/launcher/runner.py	Adds bounded shell-free process execution and now correctly preserves either the caller PATH or the platform fallback path.
packages/data-designer-slurm/tests/launcher/test_runner.py	Covers minimal environment handling, including absent and empty ambient PATH behavior.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Client as SlurmCommandClient
    participant Runner as SubprocessRunner
    participant Slurm as Slurm CLI
    participant Parser
    Caller->>Client: submit/query/cancel
    Client->>Runner: structured argv
    Runner->>Slurm: bounded subprocess with minimal environment
    Slurm-->>Runner: stdout/stderr/return code
    Runner-->>Client: completed process
    Client->>Parser: parse machine-readable output
    Parser-->>Caller: typed scheduler result

Loading

_{Reviews (9): Last reviewed commit: "simplify Slurm launcher structure" | Re-trigger Greptile}

[greptile-apps]

Default environment loses Slurm PATH

If a cluster installs Slurm outside the platform fallback path, the default runner replaces the configured environment with only LC_ALL=C while using bare executable names, causing every default client operation to fail with SlurmCommandError even though the tools are available through the caller's PATH.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/data-designer-slurm/src/data_designer/slurm/launcher/runner.py
Line: 43

Comment:
**Default environment loses Slurm PATH**

If a cluster installs Slurm outside the platform fallback path, the default runner replaces the configured environment with only `LC_ALL=C` while using bare executable names, causing every default client operation to fail with `SlurmCommandError` even though the tools are available through the caller's `PATH`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[nabinchha]

Fixed in bb5bc8e. The default runner now forwards only the caller's PATH plus LC_ALL=C; an explicitly supplied environment remains an exact replacement. This preserves cluster command discovery without unrestricted ambient-environment forwarding. The environment regression coverage and the full Slurm suite pass (490 tests after the follow-up fix).

[greptile-apps]

Empty PATH bypasses fallback resolution

If the parent environment contains PATH="", os.environ.get preserves the empty value instead of using os.defpath. The default client's bare Slurm executable names then cannot resolve, causing every operation to fail with SlurmCommandError even when the tools are installed in a platform fallback directory.

Suggested change

	dict(environment) if environment is not None else {"PATH": os.environ.get("PATH", os.defpath)}
	dict(environment) if environment is not None else {"PATH": os.environ.get("PATH") or os.defpath}

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/data-designer-slurm/src/data_designer/slurm/launcher/runner.py
Line: 39

Comment:
**Empty PATH bypasses fallback resolution**

If the parent environment contains `PATH=""`, `os.environ.get` preserves the empty value instead of using `os.defpath`. The default client's bare Slurm executable names then cannot resolve, causing every operation to fail with `SlurmCommandError` even when the tools are installed in a platform fallback directory.

```suggestion
            dict(environment) if environment is not None else {"PATH": os.environ.get("PATH") or os.defpath}
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[nabinchha]

Fixed in ca308e2 by falling back to os.defpath when the ambient PATH is absent or empty. Added a regression test for an empty PATH; the full Slurm suite passes (490 tests), and Slurm lint and formatting checks pass.

[andreatnvidia]

Mapping SPECIAL_EXIT to FAILED makes a live held job appear terminal. Since this state parser is shared by squeue observations, reconciliation could start a replacement attempt while the original job is still held and releasable. This should remain nonterminal, or queue and accounting states should use separate normalization paths. A focused held-state test would pin the intended behavior. Slurm state definitions

[nabinchha]

Fixed in 7f84dcf. SPECIAL_EXIT now normalizes to the nonterminal PENDING state, consistent with a held and releasable job. Focused state tests now pin REQUEUE_HOLD, RESV_DEL_HOLD, and SPECIAL_EXIT as nonterminal.

[andreatnvidia]

JobIDRaw does not provide the logical <array_job_id>_<task_id> identity expected here. Real array elements are returned as numeric allocation IDs, which parse_accounting silently skips, turning terminal accounting evidence into an empty result. Switching this to JobID and updating the fake and goldens would align the command with the parser. It would also be safer for unexpected ID shapes to raise instead of being discarded. Slurm field definitions

[nabinchha]

Fixed in 7f84dcf. The accounting query now requests JobID, and the fake Slurm command contract was updated to match. parse_accounting still ignores the numeric array-parent row, but unexpected step-shaped or otherwise malformed IDs now raise instead of being silently discarded. The command, fake, and parser regression tests cover the new behavior.

[andreatnvidia]

One smaller state-coverage gap: RESV_DEL_HOLD currently falls through to UNKNOWN, although Slurm defines it as held. The fallback is safe, so this is not merge-blocking, but an explicit held or pending mapping would preserve the scheduler state more accurately.

[nabinchha]

Fixed in 7f84dcf. RESV_DEL_HOLD now maps explicitly to the nonterminal PENDING state, with focused coverage alongside the other held-state spellings.

[andreatnvidia]

Could the parent allocation reserve the CPUs required by client.cpus? That value defaults to 32, but the rendered directives do not request CPUs. On sites without whole-node allocation or a sufficient DefCpuPerGPU, the later client step cannot request resources the allocation does not hold. Another valid approach would be to encode and validate that site policy in SlurmProfile; either way, both GPU request modes need coverage.

[nabinchha]

Fixed in 7f84dcf. The parent batch allocation now renders #SBATCH --cpus-per-task=<client.cpus> from the resolved client configuration. The renderer tests cover both gres and visible GPU request modes, and the pinned goldens now record the default 32-CPU claim.

[andreatnvidia]

The follow-ups resolve the accounting identity, CPU allocation, and held-state issues from the earlier review. The ordinary-job and array-task observation paths now preserve the expected selector and accounting-lag behavior. I do not see any remaining merge blockers; the unreachable unthrottled-array branch is optional cleanup.