mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 03:15:46 -06:00
fix/github-hovercard-interactive
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Marvinthebored
|
bd897a6143 |
fix(exec): make timeout unit explicit (#119874)
* fix: exec timeout field does not state its unit, so callers pass milliseconds
`exec.timeout` is in seconds, but its sibling `yieldMs` is in milliseconds and
the `process` tool's identically named `timeout` is also in milliseconds. Those
two tools are used together in one workflow: exec backgrounds a long command,
then process polls it.
The unit exists only in the field description, and code mode deliberately defers
descriptions - the model sees `timeout?: number` and nothing else. A caller that
guesses milliseconds passes 900000 intending 15 minutes and gets ~10 days, so
the command never times out.
Add `timeoutSeconds` as the canonical field, matching the convention already
used by the config default (`tools.exec.timeoutSeconds`) and the sibling
(`yieldMs`). Keep `timeout` as a deprecated alias; `timeoutSeconds` wins when
both are supplied. Project the new field onto `nodeExecSchema`, which
hand-picks its properties.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XQojYRCUdXhcKFeu4QW5Gh
* fix(ci): satisfy test-type and dependency checks
Two CI failures from the test approach, not the change itself.
- check-test-types: TypeBox's TOptional does not surface `description` on its
static type, so the schema assertions failed under tsgo. Read it through a
narrow accessor instead.
- check-dependencies: knip flagged resolveExecTimeoutSeconds as an unused
export because only the test imported it. Move it beside the schema it
resolves, where the exec runtime imports it, so it is production-reachable.
Verified with the same commands CI runs: `run-tsgo.mjs -p
test/tsconfig/tsconfig.core.test.json` reports no errors in these files, and
knip --production reports no unused exports.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XQojYRCUdXhcKFeu4QW5Gh
* fix(ci): fit the exec description budget and satisfy oxlint
- `agent-tools.schema.test.ts` enforces a 550-char budget on the exec tool's
model-facing descriptions; the two new ones pushed it to 640. Shortened both.
The rename is what makes this possible: the unit now lives in the field name,
so the description no longer has to carry it. Extended that budget test to
pin `timeoutSeconds` alongside `timeout`.
- oxlint `no-base-to-string`: the test's description accessor typed the field as
`unknown` and stringified it. Typed as `string | undefined` instead.
Verified: budget test passes (137 tests green across both suites), and all six
behavior tests still fail against origin/main.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XQojYRCUdXhcKFeu4QW5Gh
* fix: remove the unit-ambiguous exec timeout field instead of deprecating it
Owner decision on the review's "ambiguous model-visible alias" finding: drop
`timeout` entirely rather than keep it as a deprecated alias.
Keeping it visible defeated the purpose. Code mode renders property names and
types and defers descriptions, so a model would still be offered a bare
`timeout` sitting next to a millisecond-based `yieldMs` and could still pick it.
A deprecation marker in a description the model never sees changes nothing.
Backward compatibility is not required here: every tool call is constructed
fresh by the model from the current schema, and a repository search found no
internal caller passing `timeout` to the exec tool. `timeoutSeconds` is now the
only exec timeout field, on both the main and node-only surfaces.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XQojYRCUdXhcKFeu4QW5Gh
* fix: migrate the internal exec callers to timeoutSeconds
Removing `timeout` from the exec schema broke three real internal callers that
the earlier survey missed, plus two test files and the runtime's own reads.
`check-test-types` caught it; my local search had filtered `timeout:` matches to
lines mentioning "exec" and discarded these.
- src/auto-reply/reply/bash-command.ts
- src/auto-reply/reply/commands-diagnostics.ts
- src/auto-reply/reply/commands-export-trajectory.ts
all passed `timeout: timeoutSec` and now pass `timeoutSeconds: timeoutSec`.
- bash-tools.exec-run.ts reads the field through the same typeof narrowing the
third call site already used; `params` is loosely typed, so a raw read is
`unknown`.
- Test fixtures updated. A vitest `waitFor({ timeout })` option in
background-abort.test.ts is NOT an exec param and was left alone.
`run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json` now reports zero errors
in changed files; the 5 remaining `rejectSymlinks` errors are pre-existing on
main. 148 tests pass across the affected suites.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XQojYRCUdXhcKFeu4QW5Gh
* docs: synchronize the exec config table with timeoutSeconds
Addresses the P2 finding. The tools.exec.timeoutSeconds row still described the
per-call override as `timeout` and `timeout: 0`, which no longer exist. The
only remaining bare `timeout` reference in this page is the deliberate
contrast with the process tool's millisecond field.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XQojYRCUdXhcKFeu4QW5Gh
* fix: migrate the QA lifecycle caller and the shared ExecToolArgs contract
Two findings from the re-review of
|
||
|
Peter Steinberger
|
73bdb4b924 |
feat(agents): record run-end worktree cleanup outcome; prove Workboard dirty retention (#120434)
* feat(agents): record run-end worktree cleanup outcome Persist removed, retained, and failed run-end cleanup outcomes on managed worktree records. Operators and QA can inspect the durable fact through worktrees.list and openclaw worktrees list --json. Release note: Managed worktree run-end cleanup now records why a checkout was removed or retained in worktree list JSON. * test(qa): prove dirty worktree retention outcome * chore(protocol): regenerate swift gateway models * fix(agents): harden worktree cleanup recovery Register run_end_cleanup_json as a lazy compatible column so same-version v6 index repair and read-only doctor migration can recover databases created before the column existed. Type removal contention at the registry boundary; unexpected claim failures now best-effort record a bounded failed outcome and rethrow the original error. * fix(ci): clear repo-wide lint debt blocking merge gates The red-main landing rule requires this PR to repair repository-wide merge-gate debt instead of bypassing it. Apply the current lint contracts mechanically and split turn-transition coverage into a concept-named sibling with per-file-safe test state. Exact line delta: +676/-574 (net +102) across 44 test/support files. * fix(ci): preserve cached health refresh proof Require the public refresh call to exist before accepting that sensitive fields were omitted, so the boundary proof cannot pass on a missing call. * fix(ci): correct test typing left by the lint sweep Literal-widened totalTokensVersion fixtures, a WebSocket RawData overload mismatch, and the protocol schema document cast broke check-test-types after the repo-wide lint repair. Aligns the fixtures with SessionEntry, narrows Buffer handling per RawData, and keeps the JSON-shaped undefined omission under structuredClone. * test(agents): reuse upstream resource-loader test support The session-loop split and #120463's helper extraction landed the same createResourceLoader/createCompactionHandlers twice; the rebase kept both, orphaning main's agent-session-loop-resource-loader.test-support.ts and failing the dead-code gate. Import the upstream helpers and delete the duplicates. * fix(agents): reject finalized rows at the worktree removal claim Address the accepted ClawSweeper late-claim finding by rereading and rejecting missing or finalized worktree rows inside the synchronous removal-claim transaction. Preserve the authoritative cleanup invariant: finalized contenders record nothing, while retained-busy is written only while the row remains live. * refactor(agents): reuse registry update for busy outcomes Keep the live-row conditional write in the canonical registry update path so the finalized-claim repair stays below the registry max-lines ratchet without weakening the authoritative-outcome invariant. * test(agents): drop session test duplicates after rebase Keep current main as the canonical owner of next-turn lifecycle coverage and correctness test support after replaying the older lint-debt split. * fix(agents): guard post-abort cleanup outcomes against finalization After abortWorktreeRemoval releases a stale remover's claim, its retained or failed write raced a finalizing remover and could overwrite the authoritative removed-lossless fact. Route every retained/failed write through the live-row condition; only the finalizing remover's own removed-lossless write stays unconditional. * fix(agents): persist the removal outcome atomically with finalization A delayed removed-lossless write after remove() finalized could race a restore plus newer cleanup and overwrite the newer operator-visible fact. The run-end outcome now rides remove()'s finalization update; every other cleanup write stays live-row conditional, so no post-finalize write path remains. * test(qa): restore strict cached-health contract assertions The lint sweep's Boolean() coercions let truthy non-booleans satisfy the wire-typed cached-meta contract. Assert the literal boolean for unknown-typed fields and use nullish-coalesced strict equivalents for boolean chains. * fix(agents): clear the stale cleanup outcome when restoring a worktree A restored checkout begins a new lifecycle; leaving the removed-lossless fact on the live row showed operators a stale result until the next cleanup. Restore clears the recorded outcome and the regression asserts the cleared state before the next cleanup records fresh truth. * fix(agents): scope stale cleanup outcomes to their observed lifecycle A stale remover's retained/failed write raced a concurrent remove-plus- restore: the revived row is live again, so the live-row condition alone could stamp a prior-lifecycle outcome. Condition those writes on the activity stamp the remover observed; restore bumps lastActiveAt, making any prior-lifecycle write a no-op. * fix(agents): advance the restore activity stamp within one millisecond Stale cleanup writes fence on the activity stamp they observed; a restore completing in the same millisecond could revive the row with an identical stamp and let the fence match. Restore now always advances past the stored value, and the ABA regression pins the clock to prove the same-millisecond case. |
||
|
Vincent Koc
|
3c858f7a92 | test(qa): cover OpenClaw exec process lifecycle (#118819) |