feat(spur-cli): support slurm-buildkite bridge compatibility

[shiv-tyagi]

Summary

Adds the missing Slurm CLI features required by CliMA/slurm-buildkite and similar CI bridges that script against Slurm's CLI surface.

• sbatch --parsable: prints bare job ID on stdout (no header/decoration)
• sbatch positional script args: trailing arguments after the script path are forwarded as $1, $2, etc. via set -- injection
• --comment readback: new comment field on JobInfo proto, surfaced in squeue %k and scontrol show
• squeue --format literals: refactored format engine to preserve literal characters between specifiers (%k,%A emits comment,42 with no extra spacing), added %% escape support
• squeue %A: resolves to per-element job ID (matches Slurm semantics)
• squeue --name: client-side filter by job name

Approach

• Proto: script_args (field 81) on JobSpec, comment (field 53) on JobInfo. Both default to empty, backward-compatible with existing Raft state via #[serde(default)].
• Format engine: introduced FormatToken enum (Field | Literal) replacing the old Vec<FormatField> + join(" ") pattern. All callers (squeue, sinfo, sacct) updated. Named-format path (sacct) emits literal space separators between fields.
• Agent: build_job_script injects set -- <args> after the shebang line (or at top if no shebang) when script args are present. Handles CRLF shebangs and shell-escapes args via shlex.

Test plan

• 123 spur-cli unit tests pass (format engine, sbatch, squeue, sacct)
• spurd build_job_script / inject_script_args tests (shebang, no-shebang, CRLF, quoting)
• Full workspace cargo test and cargo clippy clean
• Smoke tested on 4-node bare-metal cluster: sbatch --parsable, --comment readback via squeue %k,%A, scontrol show, squeue --name, positional script args

Closes #434

[codecov-commenter]

Codecov Report

❌ Patch coverage is 84.38662% with 42 lines in your changes missing coverage. Please review.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #443      +/-   ##
==========================================
+ Coverage   70.85%   70.92%   +0.07%     
==========================================
  Files         140      140              
  Lines       41849    42026     +177     
==========================================
+ Hits        29650    29804     +154     
- Misses      12199    12222      +23     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

Adds missing Slurm CLI compatibility features in spur-cli (and the underlying proto/controller/agent plumbing) to support Slurm CI bridges like CliMA/slurm-buildkite, including sbatch output/argument handling and squeue formatting/field support.

Changes:

• Extended the public proto + core job spec to carry script_args (positional script parameters) and comment (readback via squeue/scontrol show).
• Updated the agent to inject set -- <args> into batch scripts so positional parameters ($1, $@) work as expected.
• Refactored the CLI format engine to preserve literal characters (e.g. commas) and added new squeue behaviors (--name, %k, %A, %%).

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
proto/slurm.proto	Adds script_args to JobSpec and comment to JobInfo.
crates/spurd/src/agent_server.rs	Injects set -- ... into scripts to pass positional args safely.
crates/spurctld/src/server.rs	Maps script_args and comment between proto and core job types.
crates/spurctld/src/scheduler_loop.rs	Propagates script_args through controller-to-agent dispatch.
crates/spurctld/src/accounting/grpc.rs	Initializes new comment field for accounting responses.
crates/spur-k8s/src/job_controller.rs	Includes script_args in k8s job spec proto conversion.
crates/spur-core/src/job.rs	Persists script_args in core JobSpec with backward-compatible defaults.
crates/spur-cli/src/squeue.rs	Adds --name filtering and resolves new output fields (%k, %A).
crates/spur-cli/src/sinfo.rs	Updates to new format token API.
crates/spur-cli/src/scontrol.rs	Surfaces Comment= in scontrol show output.
crates/spur-cli/src/sbatch.rs	Adds --parsable output and captures trailing script args.
crates/spur-cli/src/sacct.rs	Adjusts tests for the new tokenized format engine output.
crates/spur-cli/src/format_engine.rs	Preserves literals between specifiers and adds %% escaping.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yansun1996]

Summary

This is a well-scoped, well-tested PR — the format-engine literal-preservation redesign in particular is nicely done. A couple of minor things worth a look before merging.

What's good

• format_engine.rs's FormatField → FormatToken refactor is the right way to solve "buildkite needs %k,%A to emit comment,42 with a literal comma, not a space-joined comment , 42" — preserving literal runs between specifiers rather than always space-joining fields. Genuinely nice: test_default_format_spacing_unchanged explicitly proves the existing default squeue/sinfo format output is byte-identical to before, which is exactly the backward-compat check I'd have asked for.
• script_args / set -- injection (agent_server.rs) uses shlex::try_join to shell-escape each argument before injecting set -- <args> after the shebang — correctly prevents a malicious/malformed script argument from escaping into shell metacharacter territory. Good edge-case test coverage (shebang, no shebang, #!/usr/bin/env bash, CRLF shebang).
• candidate_script_path's rewrite to a real clap pre-parse (instead of the old "assume the last positional arg" heuristic) is more robust, and — importantly — script_args is declared trailing_var_arg + allow_hyphen_values, which correctly avoids the exact class of bug found and fixed elsewhere in this repo recently (a flag like -V being misinterpreted as sbatch's own flag instead of being passed through to the wrapped script/command). Confirmed this is handled correctly here.
• The scheduler_loop.rs/server.rs/accounting/grpc.rs/spur-k8s/job_controller.rs touches are all legitimate, mechanical propagation of the two new fields (script_args, comment) through every JobSpec/JobInfo conversion site — not scope creep.
• proto/slurm.proto changes are clean appends (script_args = 81, comment = 53), no renumbering, package slurm; untouched.

Two things worth a look

1. %A (new ARRAY_JOB_ID format spec) resolves to job.job_id, not job.array_job_id. (squeue.rs: 'A' => job.job_id.to_string().) For a non-array job these are the same, but for an individual task within a job array, real Slurm's %A means "the array's master job ID" — which should be job.array_job_id (already available on JobInfo, used a few lines away for the array_job_id/array_task_id proto fields), not the task's own ID. Worth a quick fix: if job.array_job_id != 0 { job.array_job_id } else { job.job_id } (or whatever this codebase's existing convention is for "0 means not an array job").

2. squeue --name/-n filters client-side, unlike its sibling filters. --account/-A is passed through as a field on GetJobsRequest (server-side filter), but GetJobsRequest has no name field, so --name ended up implemented as a post-fetch .retain()-style loop in the CLI. Functionally correct, but inconsistent with the existing pattern, and means squeue -n foo on a cluster with many jobs fetches and discards everything server-side would otherwise have filtered. Given this PR already touches proto/slurm.proto for two other fields, adding string name = 6; to GetJobsRequest and filtering server-side (matching account/partition/user) would be a small, consistent addition — or if there's a reason to keep it client-side for now, worth a one-line note on why.

Neither of these is a blocker — happy to see this land with a fast-follow, or fixed inline if it's a quick change. Nice work on the format-engine piece especially.

(Reviewed by direct code inspection rather than a full build/test run, given the PR's modest size and the focused nature of the changes — happy to do a build-level pass too if useful.)

[shiv-tyagi]

Thanks for the thorough review, Yan.

On your two points:

[1] %A semantics — This is correct as-is. Slurm's man page defines %A as "Job id. This will have a unique value for each element of job arrays" (the per-element ID), while %F is "Job array's job ID. This is the base job ID" (the parent). So %A -> job_id is right. Verified on a live cluster with array jobs: elements with job_id 5,6 and array_job_id 4 correctly show their own IDs via %A. The header label ARRAY_JOB_ID is admittedly confusing, but the resolver is correct.

[2] squeue --name client-side filtering — Good catch. Fixed in the latest push: added string name = 6 to GetJobsRequest, wired server-side filtering through cluster.get_jobs() (comma-separated OR, matching the existing pattern for user/partition/account), updated the REST handler and JobsQuery, migrated scancel --name to server-side as well, and added a unit test for the name filter. All verified on the bare-metal cluster and via REST.

[yansun1996]

Both findings from my earlier review are resolved — the --name filter is now server-side (matches the sibling user/partition/account filters), and my %A concern was a false positive on my part (verified: real Slurm's %A is the per-element job ID, not the array parent — that's %F — thanks for catching that with the man page + live-cluster verification). LGTM.