RST `Job.GobDecode` silently swallows a Segments decode error and skips WorkResults

[Willard84]

I've been doing some code review with Claude and it flagged this potential issue.

Severity: Low (silent, but only triggers on malformed persisted data — an uncommon path)

Summary

Job.GobDecode — the custom encoding/gob decoder used when jobs are read back from the badger
pathStore — returns nil (success) when decoding the Segments field fails, instead of
returning the error. This does two things:

1. Swallows the decode error, so the caller believes the load succeeded.
2. returns early, skipping the very next line that decodes WorkResults.

Result: a single failed Segments decode produces a Job with both Segments and
WorkResults silently empty, reported as a successful load.

Location

rst/remote/internal/job/job.go, in GobDecode:

dec := gob.NewDecoder(bytes.NewReader(remainingData))
if err := dec.Decode(&j.Segments); err != nil {
    return nil                       // BUG: should be `return err`
}
return dec.Decode(&j.WorkResults)

The surrounding decodes are correct: proto.Unmarshal(...) above returns err, and the
WorkResults decode below returns its error. Only the Segments branch returns nil — the
asymmetry is internal to this one function.

Impact

Segments is the transfer-work decomposition; WorkResults tracks per-worker progress. A job
reloaded with both silently emptied looks like a valid job with nothing to do (or mis-resumes,
losing track of in-flight/completed work) — the fail-quiet-and-corrupt mode that is worst for a
data-management service.

Severity is Low because the buggy branch only runs when the persisted gob data is malformed
(on-disk corruption, truncation, or structurally invalid protobuf). Ordinary protobuf schema
evolution does not make Unmarshal error, so this effectively never fires on healthy data — but
when it does, it hides exactly the corruption that should surface loudly.

Fix

if err := dec.Decode(&j.Segments); err != nil {
    return err
}
return dec.Decode(&j.WorkResults)

The fix only changes error propagation, not the on-disk encoding, so no data migration is
involved.

Verification

Confirm that feeding GobDecode a payload whose Segments region is malformed now returns a
non-nil error instead of a zero-value Job.

Related (separate) — harden the length-prefix parsing against malformed input

The same function parses a length prefix that assumes a well-formed buffer and will panic
(rather than return an error) on truncated/garbage data:

jobFieldLength := binary.BigEndian.Uint16(data[:2])   // panics if len(data) < 2
jobData := data[2 : 2+jobFieldLength]                  // slice out of range if truncated

Bounds-checking len(data) and returning a descriptive error would make a corrupt row fail
loudly at load instead of crashing. This is a separate concern from the return nil bug above, and I can open a separate ticket for it if desired