VDLP-50: verify the v2 client against a live FirecREST 2.6.0

README.md

@@ -25,14 +25,31 @@ The CLI uses an existing bearer token, so it never puts a client secret on a com
 
 ```bash
 export FIRECREST_URL=https://firecrest.example FIRECREST_SYSTEM=my-system FIRECREST_TOKEN=…
-firecrest-go submit job.sh /home/me
+firecrest-go systems
+firecrest-go submit -name myjob -out myjob.out -err myjob.err job.sh /home/me
 firecrest-go status 12345
-firecrest-go download /home/me/output.txt ./output.txt
+firecrest-go download /home/me/myjob.out ./output.txt
 ```
 
+Pass `-out`/`-err` when you intend to read a job's output back. Without them the
+scheduler picks the default `slurm-<jobid>.out`, and whether that file really appears
+where the API reports it is up to the site's Slurm configuration — see the caveat below.
+
 ## Verification
 
-Tests use `httptest`, require no running FirecREST stack, and are checked with `go vet`, `staticcheck`, and `go test -race`. No real Alps credentials or systems are exercised by this repository.
+Unit tests use `httptest`, require no running FirecREST stack, and are checked with
+`go vet`, `staticcheck`, and `go test -race`.
+
+Beyond that, this client has been **run end to end against a real FirecREST v2 server**
+(`f7t-appversion: 2.6.0`) on the project's local demo stack, on 2026-09-15: OAuth2
+client-credentials token, `systems`, `submit`, `status` polled to `COMPLETED`, and
+`download` of the job's stdout all succeeded. What that run found is recorded in
+`docs/verification-v2.md`.
+
+**No real Alps system and no real CSCS credentials have been exercised.** The demo
+stack's scheduler is a shell stand-in, not Slurm, and its tokens are not representative
+of Keycloak's short-lived ones — so timing, token refresh under load, and real
+scheduler states remain unverified.
 
 ## License
 

cmd/firecrest-go/main.go

@@ -19,6 +19,9 @@ func main() {
 	baseURL := fs.String("url", os.Getenv("FIRECREST_URL"), "FirecREST base URL")
 	system := fs.String("system", os.Getenv("FIRECREST_SYSTEM"), "FirecREST system name")
 	token := fs.String("token", os.Getenv("FIRECREST_TOKEN"), "Bearer token")
+	name := fs.String("name", "", "submit: job name")
+	stdout := fs.String("out", "", "submit: standard output path, relative to the working directory")
+	stderr := fs.String("err", "", "submit: standard error path, relative to the working directory")
 	fs.Parse(os.Args[2:])
 	if *baseURL == "" || *system == "" || *token == "" {
 		fmt.Fprintln(os.Stderr, "-url, -system and -token (or matching environment variables) are required")
@@ -37,7 +40,13 @@ func main() {
 		if err != nil {
 			fatal(err)
 		}
-		id, err := client.Submit(context.Background(), firecrest.SubmitRequest{Job: firecrest.JobDescription{Script: string(script), WorkingDirectory: fs.Arg(1)}})
+		id, err := client.Submit(context.Background(), firecrest.SubmitRequest{Job: firecrest.JobDescription{
+			Script:           string(script),
+			WorkingDirectory: fs.Arg(1),
+			Name:             *name,
+			StandardOutput:   *stdout,
+			StandardError:    *stderr,
+		}})
 		if err != nil {
 			fatal(err)
 		}
@@ -46,11 +55,29 @@ func main() {
 		if fs.NArg() != 1 {
 			usage()
 		}
+		job, err := client.Job(context.Background(), fs.Arg(0))
+		if err != nil {
+			fatal(err)
+		}
 		status, err := client.JobStatus(context.Background(), fs.Arg(0))
 		if err != nil {
 			fatal(err)
 		}
-		fmt.Printf("job=%s stdout=%s stderr=%s\n", status.JobID, status.StandardOutput, status.StandardError)
+		exit := "?"
+		if job.Status.ExitCode != nil {
+			exit = fmt.Sprint(*job.Status.ExitCode)
+		}
+		fmt.Printf("job=%s name=%s state=%s exit=%s terminal=%t stdout=%s stderr=%s\n",
+			job.JobID, job.Name, job.Status.State, exit, job.Status.Terminal(),
+			status.StandardOutput, status.StandardError)
+	case "systems":
+		systems, err := client.Systems(context.Background())
+		if err != nil {
+			fatal(err)
+		}
+		for _, s := range systems {
+			fmt.Println(s.Name)
+		}
 	case "download":
 		if fs.NArg() != 2 {
 			usage()
@@ -78,6 +105,6 @@ func main() {
 }
 func fatal(err error) { fmt.Fprintln(os.Stderr, "error:", err); os.Exit(1) }
 func usage() {
-	fmt.Fprintln(os.Stderr, "usage: firecrest-go <submit|status|download> [flags] args")
+	fmt.Fprintln(os.Stderr, "usage: firecrest-go <submit|status|systems|download> [flags] args")
 	os.Exit(2)
 }

docs/verification-v2.md

@@ -0,0 +1,163 @@
+# End-to-end verification against FirecREST v2 (VDLP-50)
+
+Date: 2026-09-15. Server: the project's local demo stack, `f7t-appversion: 2.6.0`,
+one system `fakecluster`. Issuer: the demo launcher on `:8025` (it acts as the OIDC
+issuer; there is no separate Keycloak in this stack).
+
+This client was written against the v2 **specification** in VDLP-49 and had never
+been run against a server. This document records what that first contact found.
+It is a record of observations, not a changelog.
+
+## Result
+
+All four commands work. Three defects were found on the way, one in this client and
+two in the demo stack. Details below.
+
+| Command | Outcome |
+| --- | --- |
+| token, OAuth2 client credentials | works |
+| `systems` | works — `fakecluster` |
+| `submit` | works — HTTP 201, `{"jobId":"14"}` |
+| `status` | works, but only after adding the state call (see 2) |
+| `download` | works on an explicit output path; fails on the default one (see 4) |
+
+## 1. The submit body used snake_case; the schema is camelCase
+
+The client sent `working_directory`, `standard_output`, `standard_error`. The
+published `JobDescriptionModel` names these `workingDirectory`, `standardOutput`,
+`standardError`.
+
+FirecREST 2.6.0 accepts **both**. Submitting the same job twice, once each way,
+produced two jobs whose scheduler-side metadata was identical:
+
+```
+snake_case -> 201 {"jobId":"10"}   meta: 10|snake-test|/home/demo/tmp|/home/demo/tmp/snake.out
+camelCase  -> 201 {"jobId":"11"}   meta: 11|camel-test|/home/demo/tmp|/home/demo/tmp/camel.out
+```
+
+So this was never going to surface as a failed request — the fields were being
+honoured. It matters anyway: acceptance of the snake_case form comes from Pydantic
+models that populate by field name as well as by alias, which is an implementation
+detail of this server version, not a promise in the contract. A future release that
+sets `populate_by_name=False`, or a different v2 implementation, would silently drop
+`working_directory` and run every job in the user's home directory instead of the
+requested one — a wrong-directory job, not an error.
+
+**Changed**: the struct tags now carry the documented camelCase names. The unit test
+that asserted the snake_case wire format was asserting the wrong thing and was
+updated with it.
+
+## 2. `/metadata` carries no job state, so `status` could not answer "is it done?"
+
+`GET /compute/{system}/jobs/{id}/metadata` returns exactly:
+
+```json
+{"jobs":[{"jobId":"12","script":"…","standardInput":"/dev/null",
+          "standardOutput":"/home/demo/vdlp50-rt.out","standardError":"…"}]}
+```
+
+`jobId`, `script`, and the three stream paths. No state, no exit code. The issue
+asked to poll "until a terminal state", and with only this call there is nothing to
+poll — the response is identical for a queued job and a finished one.
+
+State lives on the sibling endpoint `GET /compute/{system}/jobs/{id}`:
+
+```json
+{"jobs":[{"jobId":"12","name":"vdlp50","status":{"state":"COMPLETED",
+          "stateReason":"None","exitCode":0,"interruptSignal":0}, …}]}
+```
+
+**Added**: `Client.Job` reading that endpoint, a `JobState.Terminal()` predicate over
+Slurm's terminal states, and `Client.Systems` for `/status/systems`. `status` now
+reports state, exit code and terminality alongside the paths. `Terminal()` treats an
+unrecognised state as non-terminal, so an unknown state makes a caller poll again
+rather than report a wrong outcome.
+
+Note for whoever writes the polling loop: `Terminal()` is the state-based stop
+condition, and it is the one to use. `exitCode` is not a substitute — the field is
+nullable in the schema, and this run only ever observed jobs that had already
+finished, so its value before terminality is unverified here.
+
+## 3. Stack defect: `standardOutput` is concatenated onto the working directory
+
+Submitting an **absolute** `standardOutput` produces a doubled path in the metadata
+response:
+
+```
+request : workingDirectory=/home/demo/tmp  standardOutput=/home/demo/tmp/snake.out
+metadata: "standardOutput": "/home/demo/tmp//home/demo/tmp/snake.out"
+```
+
+The file is written to the right place; only the reported path is wrong. The
+join is unconditional instead of respecting an already-absolute path. Not patched —
+it is the stack's defect, and per the issue's constraints it is documented, not
+worked around. Practical consequence: **pass output paths relative to the working
+directory**, which is what the CLI's `-out`/`-err` now do.
+
+## 4. Stack defect: the default output path reported does not exist
+
+Submitting without `standardOutput`, `status` reports `/home/demo/slurm-13.out`, and
+downloading it fails:
+
+```
+$ firecrest-go download /home/demo/slurm-13.out /tmp/final.out
+error: FirecREST API error: status=404 detail="{\"errorType\":\"error\",
+  \"message\":\"Remote process failed with exit status:1 and error message:
+  base64: /home/demo/slurm-13.out: No such file or directory\", …}"
+```
+
+The client is reporting faithfully what the server told it. The two halves of the
+`fakecluster` stand-in disagree about where default output goes:
+
+```
+sbatch  : [ -n "$OUT" ] || OUT_ABS="$JOBDIR/stdout"      # /var/spool/fakeslurm/13/stdout
+scontrol: [ -n "$out" ] || out="slurm-${jid}.out"        # reports /home/demo/slurm-13.out
+```
+
+`sbatch` writes the job's stdout into its spool directory; `scontrol`, which is what
+FirecREST reads the metadata from, reports the conventional Slurm name. The file at
+the reported path is never created. This is a defect of the demo stand-in and would
+not occur against real Slurm, where the default output genuinely lands in the working
+directory — so it should not be read as a v2 API defect.
+
+With an explicit output path the whole chain works:
+
+```
+$ firecrest-go submit -name vdlp50 -out vdlp50.out -err vdlp50.err job.sh /home/demo
+14
+$ firecrest-go status 14
+job=14 name=vdlp50 state=COMPLETED exit=0 terminal=true stdout=/home/demo/vdlp50.out stderr=/home/demo/vdlp50.err
+$ firecrest-go download /home/demo/vdlp50.out /tmp/final.out
+OK (23 bytes): VDLP-50 final run\ndone
+```
+
+## 5. Confirmed correct, no change needed
+
+- **Authentication.** `clientcredentials.Config` against the launcher's `/token`
+  endpoint yields a usable token; `/status/systems` returns 200 with it and 401
+  without. The issue flagged the issuer as a likely first discovery — it is not one,
+  the flow works as written.
+- **`ops/download` is a byte stream**, `content-type: application/octet-stream`, not
+  a JSON envelope. The client's raw-body handling is right. It is *not* `ops/view`,
+  so the documented `/ops/view` size-default defect does not apply to this client.
+- **`ops/upload`** with multipart field `file` and the directory in `?path=`
+  returns 204, matching the client's implementation.
+- **The `max_ops_file_size` limit is real and enforced**: downloading a 2 MiB file
+  against the configured 1 MiB limit returns `413 {"message":"Command output exceeded
+  buffer limit."}`. `Download` surfaces it as an `APIError` with the status intact.
+  No streaming/chunked path exists in this client — large files need the async
+  `filesystem/{system}/transfer/*` endpoints, still out of scope.
+- **`jobId` is returned as a JSON string** (`{"jobId":"14"}`), not the number the
+  original unit test assumed. The client already accepted both; a test now pins both.
+
+## Not verified
+
+No real Alps system, no real CSCS credentials. `fakecluster`'s scheduler is a shell
+script that runs the job inline and synchronously — there is no queue, so no job was
+ever observed in `PENDING` or `RUNNING`, and the terminal-state logic has been
+exercised only against `COMPLETED`. Demo tokens are long-lived; refresh behaviour
+under a realistic ~300 s TTL is untested.
+
+One further gap, unrelated to this client: `GET /status/{system}/userinfo` fails on
+this stack with `sacctmgr: command not found` — the stand-in does not provide that
+binary. The endpoint is not used here.

firecrest.go

@@ -43,14 +43,20 @@ type SubmitRequest struct {
 }
 
 // JobDescription is deliberately limited to portable submission fields.
+//
+// Field names follow FirecREST v2's published schema (JobDescriptionModel),
+// which is camelCase. FirecREST 2.6.0 also accepts the snake_case spellings
+// this client used previously, because its Pydantic models populate by field
+// name as well as by alias — but that is an implementation courtesy, not part
+// of the contract, so the documented names are what we send.
 type JobDescription struct {
 	Script           string            `json:"script"`
-	WorkingDirectory string            `json:"working_directory"`
+	WorkingDirectory string            `json:"workingDirectory"`
 	Name             string            `json:"name,omitempty"`
 	Account          string            `json:"account,omitempty"`
 	Partition        string            `json:"partition,omitempty"`
-	StandardOutput   string            `json:"standard_output,omitempty"`
-	StandardError    string            `json:"standard_error,omitempty"`
+	StandardOutput   string            `json:"standardOutput,omitempty"`
+	StandardError    string            `json:"standardError,omitempty"`
 	Environment      map[string]string `json:"env,omitempty"`
 }
 
@@ -83,6 +89,9 @@ type JobMetadata struct {
 }
 
 // JobStatus returns metadata for a job, including standard output and error paths.
+//
+// Note that /metadata carries no scheduler state: to learn whether a job is
+// still running, use Job, which reads GET /compute/{system}/jobs/{id}.
 func (c *Client) JobStatus(ctx context.Context, jobID string) (JobMetadata, error) {
 	var response struct {
 		Jobs []JobMetadata `json:"jobs"`
@@ -96,6 +105,88 @@ func (c *Client) JobStatus(ctx context.Context, jobID string) (JobMetadata, erro
 	return response.Jobs[0], nil
 }
 
+// JobState is the scheduler state of a job, as reported by sacct.
+type JobState struct {
+	State           string `json:"state"`
+	StateReason     string `json:"stateReason"`
+	ExitCode        *int   `json:"exitCode"`
+	InterruptSignal *int   `json:"interruptSignal"`
+}
+
+// Terminal reports whether the job has stopped changing state, so a caller can
+// end a polling loop. Slurm's terminal states are enumerated explicitly: an
+// unknown state is treated as non-terminal, which makes a caller poll again
+// rather than report a wrong outcome.
+func (s JobState) Terminal() bool {
+	switch s.State {
+	case "COMPLETED", "FAILED", "CANCELLED", "TIMEOUT", "NODE_FAIL",
+		"PREEMPTED", "BOOT_FAIL", "DEADLINE", "OUT_OF_MEMORY", "REVOKED":
+		return true
+	default:
+		return false
+	}
+}
+
+// Job describes a job's scheduler state and placement.
+type Job struct {
+	JobID            string   `json:"jobId"`
+	Name             string   `json:"name"`
+	Status           JobState `json:"status"`
+	Cluster          string   `json:"cluster"`
+	Partition        string   `json:"partition"`
+	Nodes            string   `json:"nodes"`
+	User             string   `json:"user"`
+	Account          string   `json:"account"`
+	WorkingDirectory string   `json:"workingDirectory"`
+}
+
+// JobState fetches a job's scheduler state via GET /compute/{system}/jobs/{id}.
+// This is the call to poll; JobStatus reports only paths and the script.
+func (c *Client) Job(ctx context.Context, jobID string) (Job, error) {
+	var response struct {
+		Jobs []Job `json:"jobs"`
+	}
+	if err := c.doJSON(ctx, http.MethodGet, "compute", path.Join("jobs", jobID), nil, &response); err != nil {
+		return Job{}, err
+	}
+	if len(response.Jobs) != 1 {
+		return Job{}, fmt.Errorf("decode job: expected one job, got %d", len(response.Jobs))
+	}
+	return response.Jobs[0], nil
+}
+
+// System is one HPC system exposed by this FirecREST installation.
+type System struct {
+	Name string `json:"name"`
+}
+
+// Systems lists the systems this installation serves. Unlike every other call
+// it is not scoped to c.system, so it is the cheapest way to check that a
+// configured system name actually exists before using it.
+func (c *Client) Systems(ctx context.Context) ([]System, error) {
+	var response struct {
+		Systems []System `json:"systems"`
+	}
+	u := *c.baseURL
+	u.Path = strings.TrimRight(u.Path, "/") + "/status/systems"
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil)
+	if err != nil {
+		return nil, err
+	}
+	resp, err := c.http.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("FirecREST request: %w", err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return nil, newAPIError(resp)
+	}
+	if err := json.NewDecoder(resp.Body).Decode(&response); err != nil {
+		return nil, fmt.Errorf("decode response: %w", err)
+	}
+	return response.Systems, nil
+}
+
 // Download fetches a small file through the synchronous filesystem ops endpoint.
 // The caller owns closing the returned body.
 func (c *Client) Download(ctx context.Context, remotePath string) (io.ReadCloser, error) {