* fix(llm): key the response cache on sampling parameters
--temperature and --max-output-tokens were sent to the adapter but left out
of the cache key, so changing either one replayed the previous answer
instead of calling the model. The same key drives --state-key reuse, so a
resumed workflow replayed the stale answer too.
Hash each parameter only when it is set, guarding on the same predicate
that decides whether it goes on the wire. stableStringify sorts keys, so an
invocation that sets neither serializes exactly as before and keeps its
current hash - the cache is content-addressed by filename and a blanket key
change would orphan every entry written by an earlier release.
metadata stays out: it is set unconditionally with --metadata-json and
carries correlation data rather than decode-time parameters, so hashing it
would invalidate existing caches and collapse the hit rate. --schema-version
already covers callers who want metadata to segment the cache.
* fix(llm): version the cache identity so old entries cannot replay sampling
Keying only the parameters that were supplied kept the omitted-parameter key
byte-identical to what earlier releases wrote. That preserved existing caches,
but it also left one collision: an entry those releases stored for a request
sampled at an explicit temperature sits under the key a request that omits
sampling computes, so after upgrading, an unsampled request could be served a
sampled answer.
Put both parameters in the payload unconditionally, with `null` as the identity
of an omitted one, and add a version field to the payload. Entries written under
the previous identity are unreachable rather than ambiguous: the cost is one
re-invocation per prompt after upgrade, never a wrong replay.
The key-stability test is replaced by its inverse — a cache entry written under
the pre-upgrade key is not returned to a request that omits sampling.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X6UVfFPP39RkoYx5jZ3KKM
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(state): normalize extended-length mkdir paths
On Windows fs.mkdir(recursive) reports the first created directory as an
extended-length path (\?\C:\...) while the requested directory is a plain
drive path. path.resolve keeps the prefix, so the two never compare equal
and path.relative between them yields an absolute path. The chain walk then
takes "C:" as its next segment and syncs a directory that does not exist,
so ensureDirectory throws ENOENT every time it actually creates something.
That breaks LLM cache writes, state.set, diff snapshots, and approval index
publication on the first Windows run.
Strip the prefix before resolving so both ends of the chain share one root
form. On POSIX the prefixes never occur and the walk is unchanged.
* fix(state): keep device namespaces out of the path normalization
Stripping every \?\ prefix also rewrote namespaces that have no plain
equivalent, so an explicitly configured LOBSTER_STATE_DIR such as
\?\Volume{GUID}\lobster\state became relative and resolved against the
current drive. That regressed a form main handles today.
Map only the drive-letter and UNC namespaces, which do have a plain
equivalent, and return anything else untouched. Cover the mapping directly
so the device-namespace case is pinned.
* fix(state): recognise the UNC namespace whatever its case
Windows compares path namespace components case-insensitively, so
`\?\unc\server\share` names the same share as `\?\UNC\server\share`.
Matching only the uppercase form left the lowercase spelling extended
while the directory it walks toward is plain, so the sync walk could
never reach the requested final path on such a setup.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gD5sJTbh2jn1uvgNCmkLq
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix: workflow run crashes when a step exits before reading its stdin
A step command that exits during its startup (for example a shell script
failing a preamble check under set -e) before draining a large piped stdin
leaves the engine's pending stdin write to fail with EPIPE. The stdin socket
had no 'error' listener, so Node raised an unhandled 'error' event and the
entire lobster process crashed -- losing the approval gate, the resume token,
and the step's real exit code and stderr.
Ignore stdin write errors in both shell-step and stdlib exec spawns: the
close handler already reports the true failure. Regression test drives a
300KB stdout through a fast-exiting step and asserts the run rejects with
'workflow command failed (1)' instead of crashing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* simplify: condense EPIPE inline comments to essential context
The 5-line comment blocks explaining EPIPE behavior were excessive for
1-line handlers. The test comment describing the 300KB mechanism was
similarly verbose. Trim each to the non-obvious why only.
* simplify: remove EPIPE inline comments
The child.stdin.on('error', () => {}) pattern is self-explanatory to
Node.js developers; the rationale is fully documented in the PR body.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
* fix(llm): stop spending an extra call on validation retries
The retry budget was compared against a 1-based attempt counter plus one,
so the initial request never counted against it. Every configured value
bought one more billed adapter call than requested, and
--max-validation-retries 0 still issued a second call after the first
response failed the output schema.
Compare the attempt counter against the retry count directly: attempt
counts calls, max-validation-retries counts retries, so calls = retries+1.
Only the failure bound moves; the success path is unchanged.
* docs(llm): say what --max-validation-retries counts
The flag's own description said only "retries when schema validation fails",
which is exactly the ambiguity this fix resolves: whether the budget includes
the first call. The compatibility question in front of a maintainer is easier to
answer when the option states that it allows N extra calls after the first, and
that its default is 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Bu6MvbjKGtfRiNesARMyZ
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Harden LLM cache files, diff snapshots, and approval ID indexes against truncated or malformed JSON after process termination.
Disposable cache and snapshot corruption now recovers as a miss; authoritative resume state still surfaces malformed JSON. Approval short-ID indexes use atomic no-overwrite publication and degrade to full resume-token approval when the index cannot be published durably.
Closes#111.
Closes#112.
Closes#113.
Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>
Fixes#108 and #109.
- Replace direct state writes with same-directory atomic temp-file writes.
- Preserve existing file modes and create new state files as 0600.
- Clean up temp files on failed replacement paths.
- Add state/SDK regression coverage plus changelog entry.
Proof:
- pnpm run typecheck
- node --test dist/test/state.test.js dist/test/resume.test.js dist/test/multi_approval_resume.test.js dist/test/approve_preview.test.js
- pnpm run lint
- pnpm run test
- built SDK write/read proof preserved 0600 across replacement
- autoreview clean: no accepted/actionable findings
Co-authored-by: Krasimir Kralev <krasi@idrobots.com>
* fix(retry): only propagate AbortError on external cancellation, not per-attempt timeout
Fixes#105.
withRetry unconditionally re-threw AbortErrors before calling shouldRetry,
causing timeout_ms + retry.max combinations to always result in a single
attempt regardless of retry configuration. Fix: check options?.signal?.aborted
before short-circuiting — external workflow cancellation still propagates
immediately, but per-attempt timeout AbortErrors now flow through shouldRetry.
* test(retry): update abort-error test to use aborted external signal; add timeout-retry unit test
Update withRetry test to properly simulate external cancellation (aborted
signal) rather than a bare AbortError without signal context.
Add unit test proving per-attempt timeout AbortErrors (no external signal)
are now retried as documented when timeout_ms + retry are combined.
* fix(retry): revert quote style to single-quote (match fork base)
* fix(retry): revert test quote style to single-quote (match fork base)
* test: add workflow-level proof that timeout_ms + retry retries on timeout
Integration test that runs a real step with timeout_ms=1500 + retry.max=3
where the command hangs past the timeout on attempts 1-2 (SIGKILLed) then
succeeds on attempt 3. Asserts status ok + attempt 3 + [RETRY] logs.
Verified the test fails against the pre-fix withRetry (short-circuit on
any AbortError) and passes with the fix. Addresses the review request for
real behavior proof at the workflow level, complementing the existing
withRetry unit tests.
* docs: credit timeout retry fix
* test: harden timeout retry workflow proof
* style: format timeout retry patch
---------
Co-authored-by: KrasimirKralev <krasi@idrobots.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
Adds retry field to workflow steps with exponential/fixed backoff,
jitter, and configurable max attempts. Retry delays are signal-aware
(abort cancels immediately). Abort errors never trigger retries.
- New src/core/retry.ts: withRetry utility with backoff calculation
- Step execution wrapped in retry loop when retry.max > 1
- Retry attempts logged to stderr as [RETRY] messages
- Dry-run renders retry config (attempts, backoff, base delay)
- Comprehensive validation for all retry fields
