CI's check:lint caught three real issues in the directive surface:
- TranscriptDirectiveLeaf called the contribution's render() inline in JSX
— the exact pattern no-restricted-syntax bans because the callback's hooks
land in the host and a plugin reload changes the host hook count
(React #310). The callback is now memoized and mounted via ContribRender.
- The inline frame mirrored its height state into a ref from the message
handler (the stale-read pattern no-restricted-syntax flags). Functional
setState reads current state directly; the shadow refs are gone.
- perfectionist import/export ordering in the frame and the SDK index.
The SDK doc still described the v1 frame (fixed height attribute, rail card
under the frame) — chrome that no longer exists. And plugin_storage's usage
example used `with plugin_db(...)`, which reads as auto-close but sqlite3's
context manager only scopes transactions; the example now closes explicitly.
The first inline frame was a full-width bordered box at a fixed height:
webpage-in-a-rectangle, not a widget. Now the frame disappears into the
message flow:
- Content-driven size. The injected measurer reports height (live) and
intrinsic width (adopted once, so %-width children can't feedback-loop the
frame toward zero). A sparkline shrink-wraps and sits flush left like an
inline image; a full-bleed page measures the whole viewport and stays
column-wide. The height attribute is now only a starting value.
- Theme bridge. A style prelude injects first with the app's resolved theme
tokens under stable names (--foreground, --muted-foreground, --accent,
--border, --card), the app font, zero body margin/padding, and a
transparent background — reference HTML written against those vars renders
native in any theme. Page styles override the prelude, so a page that
brings its own design keeps it.
- No chrome. Border, rounded box, and the rail-opener card under the frame
are gone; the fallback paths (non-HTML, remote gateway, unreadable file)
keep the classic card. The wheel gate went with the border — frames size
to content, so there is nothing to scroll inside, and widgets are fully
interactive.
- The desktop platform hint now teaches the default: an inline widget is
transparent, token-colored, flush left, no page chrome — only a standalone
page brings its own background. "Make me an inline sparkline" gets native
styling without the user spelling it out.
Some providers re-send the previous assistant text verbatim when a turn
continues past a tool call (a tool_calls row, then a stop row with identical
prose — both persisted). The turn merge folds both rows into one bubble, so
every paragraph in the reply rendered twice; inline ::preview frames made it
obvious. Repeated text parts now dedupe in the same pass as generated-image
echoes — the last occurrence wins.
The frame was a hardcoded 280px unless the model guessed a height attribute
— tall pages clipped (the flip-clock demo cut its last digit), short ones
floated in dead space. The opaque-origin sandbox means the parent can't
measure the document, but we own the srcdoc string: a tiny injected script
observes the document with ResizeObserver and posts its scrollHeight up via
postMessage, and the frame tracks it live within the 120-1200 clamp.
Reports are validated before they can move layout — per-mount random token
(two previews in one transcript, or a hostile page inventing messages,
can't move each other's frames), finite-number check, clamp. A 4px
tolerance stops vh-sized pages (which measure exactly what they're given)
from oscillating; an explicit height attribute still opts out of
auto-sizing entirely.
The first cut of the core ::preview consumer rendered the classic
preview-attachment card — a button into the right rail we already had, which
made the directive indistinguishable from an ordinary preview link. Now the
directive shows the thing itself: the workspace HTML file renders in a
sandboxed srcdoc iframe inline in the assistant message (opaque origin,
allow-scripts only — no reach into the app, its storage, or the bridge),
with an optional height attribute clamped to 120-1200px and the classic
card kept below as the rail escape hatch.
The frame waits for turn settle before reading the file (mid-stream it is
often mid-write), resolves relative paths against the session's own cwd,
and falls back to the plain card for non-HTML targets and remote gateways
(no local file door there).
Plugins that persist state have been writing into their own install tree
(<hermes home>/plugins/<name>/), which `hermes plugins update` git-pulls and
`hermes plugins remove` deletes — user data dies with the code that wrote it.
plugins/plugin_storage.py is the sanctioned home: plugin_data_dir(name) gives
one data root per plugin under <hermes home>/plugin-data/<name>/ (profile-
aware, created on first use, names validated against traversal), and
plugin_db(name) opens a WAL-mode SQLite database inside it. Secrets stay on
the existing secret-scope path — this is state, not credentials.
hermes-achievements, the in-tree offender, converts with a legacy-file
migration on first read.
The transcript becomes a contribution area (transcript.directives). A plugin
registers a named directive and the model addresses it by emitting
::name{key="value"} as its own paragraph; that leaf renders as the plugin's
component, wrapped in the contribution error boundary. Unclaimed or malformed
directives stay plain prose, so nothing changes for text that merely looks
like a directive (std::vector) or for users with the plugin disabled.
Core ships ::preview{file="..."} as the reference consumer (the existing
preview-attachment card), the desktop platform hint teaches the model the
syntax, and the SDK exports the area + types so runtime plugin.js files get
the surface through the normal plugins API.
Reconciles the salvaged Responses-API mandate with the bundled meta-ai
plugin that landed in #88565:
- meta-ai profile api_mode -> codex_responses (prompt caching engages
only on /v1/responses; 0% vs 93-99% measured). Custom endpoints with a
non-api.meta.ai base URL still fall through to chat_completions via
the host-driven mandate design.
- cli-config.yaml.example: point the example at MODEL_API_KEY (Meta's
documented env var) and note the bundled provider covers the default
endpoint
- tests/providers/test_meta_ai_profile.py updated for the new wire
Implement Claude Opus review findings for Meta API support:
- Document in agent/agent_init.py that provider="meta" without an api.meta.ai URL falls through to chat_completions by design (URL-driven wire selection).
- Comment on suppression guard in hermes_cli/runtime_provider.py noting api.meta.ai is handled by _detect_api_mode_for_url.
- Replace inline __import__ with top-of-module import in tests/hermes_cli/test_model_switch_openai_api_mode.py.
- Rename test_meta_retention_not_sent_when_overridden -> test_meta_retention_override_wins in tests/agent/transports/test_meta_codex_cache.py.
- Add test in tests/agent/test_meta_agent_init.py for provider="meta" fallback without api.meta.ai URL.
- Add test in tests/agent/test_auxiliary_client.py for prompt_cache_retention: "24h" under _CodexCompletionsAdapter.
Source: Claude Opus review findings for feat/meta-api-support.
Relocate host_mandated_api_mode check from top of api_mode cascade to
fallback else branch so URL-based provider-slug rewrites (e.g.
api.anthropic.com -> provider='anthropic') always run first. Previously
the mandate branch set api_mode for api.anthropic.com without rewriting
provider, leaving provider='' and causing credential_pool_matches_provider
to fail closed and discard anthropic-scoped pools (#63425 regression
introduced in 8f60e8263).
The mandate is now a true fallback for hosts without an elif branch
(api.meta.ai -> codex_responses for 93-99% prompt-cache hits vs 0% on
chat, plus future mandates) with lazy import + try/except preserved.
Add regression tests: provider=None + api.anthropic.com URL implies
provider='anthropic'/api_mode='anthropic_messages' and preserves an
anthropic credential pool; provider=None + api.meta.ai URL implies
codex_responses.
- hermes_cli/providers.host_mandated_api_mode: add exact-hostname clause for
api.meta.ai → codex_responses (measured 0% cache on /chat/completions vs
93-99% on /responses with retention); update docstring.
- hermes_cli/runtime_provider._detect_api_mode_for_url: mirror clause for
api.meta.ai (exact hostname, #32243) to keep runtime resolver in lockstep.
- agent/agent_init: call host_mandated_api_mode early in api_mode cascade
(after explicit api_mode wins, before provider-name specials) via lazy
import; single source of truth, preserves user override.
- agent/transports/codex._default_prompt_cache_retention_for_request: return
24h for api.meta.ai unconditionally; build_kwargs setdefault preserves
override; Bedrock branch untouched.
- cli-config.yaml.example: add commented providers.meta example (api_mode
auto-detected).
- website/docs/developer-guide/adding-providers.md: list Meta alongside
Codex/xAI as codex_responses native provider with retention note.
- tests: add hermetic behavior-contract suites for mandate, retention,
content-addressed prompt_cache_key, reasoning passthrough, AIAgent init,
usage cache reporting, model-switch override, and config roundtrip; extend
test_model_switch_openai_api_mode with meta cases.
Surface meta/muse-spark-1.2 in the Hermes model selector (CLI, desktop,
gateway) via the curated OpenRouter list and regenerated model-catalog.json.
The model is live on OpenRouter with tool calling; the picker intentionally
does not show the full OpenRouter catalogue.
Desktop shipped the same bug class four times in one week: a decorative
animation loop that never sleeps (bots face clock #88543, pixel egg #88406,
diffusion placeholder #88564, plus the #77651 hidden-renderer wave). Each fix
hand-rolled the same four behaviors. This extracts them into one helper in
src/lib/budgeted-loop.ts:
- fps budget (default 15) on top of rAF
- observability pause via the existing createRendererLoopPauseController
- idle dormancy: idleWhen() true after a draw parks the loop with zero
pending work until wake() — the piece every hand-rolled loop forgot
- teardown: dispose() cancels the frame, disposes the controller, and makes
wake() a no-op
DiffusionCanvas migrates onto it as the first consumer (net -40 lines at the
call site); its existing scheduling/budget/instance-cap tests pass unchanged
against the migrated implementation. Helper suite covers budget, pause,
park/wake, dormancy-survives-focus-churn, and dispose idempotency; sabotage
run (budget+dormancy stripped) fails 4/5.
- plugins/model-providers/meta-ai/__init__.py: drop out-of-tree install
instructions from the module docstring (now bundled)
- tests/providers/test_meta_ai_profile.py: port the plugin's test suite
into the repo (registry discovery instead of file-location import)
- website/docs/integrations/providers.md: meta-ai in the first-class
API-key provider list, META_BASE_URL override, contributor-tier
data-training note
Ships the meta-ai provider profile from
albertodepaola/hermes-meta-provider as a bundled model-provider plugin:
OpenAI-compatible chat completions at https://api.meta.ai/v1,
MODEL_API_KEY auth, top-level reasoning_effort dial with the Meta
none->minimal 400 guard, vision support, 16k default max_tokens, and
muse-spark-1.2 / muse-spark-1.2-contributor as the offline fallback
catalog.
When an external scheduler (Chronos on hosted deployments) cannot
deliver a fire — dead loopback hop at fire time, retry budget exhausted
— the job's next_run_at stays parked in the past and nothing ever runs
it: external providers have no local tick loop, so the day is silently
lost even if the gateway heals minutes later (4 consecutive nightly
misses in the field).
fire_overdue_jobs() in cron/scheduler_provider.py, called from the
gateway housekeeping loop every 5 minutes:
- No-op for the built-in ticker (its tick loop already self-heals
past-due jobs) and when cron.misfire_grace_minutes <= 0.
- Waits out a grace window (default 10 min) so the external scheduler's
own retry backoff gets first right to deliver.
- Claims via the provider's claim_fire (store CAS — a concurrent late
external retry is de-duplicated) and runs fire_claimed in a daemon
thread, mirroring the webhook admission pattern, so housekeeping
never blocks for the length of an agent run. Provider re-arm logic
(Chronos NAS one-shots) runs exactly as for a normal fire.
Docs: cron.md section + cron.misfire_grace_minutes reference.
Sessions started inside a git checkout now source skills from
<root>/.hermes/skills/ and <root>/.agents/skills/ (the cross-tool
convention shared with other agent harnesses) as the highest-precedence
skill tier: project > local > external_dirs.
Loading is trust-gated per repo (skills.trusted_project_dirs, managed by
'hermes skills trust'/'untrust') because skills are executable procedure
documents — auto-sourcing them from any cloned repo is a prompt-injection
vector. Untrusted repos with skills get a one-line banner notice instead.
- agent/skill_utils.py: find_project_root, get_project_skills_dirs,
get_untrusted_project_skills_root, get_scan_ordered_skills_dirs;
project dirs join the curator read-only ownership boundary
- agent/prompt_builder.py: project tier scanned first, entries tagged
[project], same-named local entries shadowed; cache key extended
- tools/skills_tool.py: skills_list scans project dirs first (first-wins);
skill_view resolves cross-tier collisions in favor of the project tier
(same-tier ambiguity still refuses); security warning recognizes the tier
- agent/skill_commands.py + hermes_cli/commands.py: /skill-name slash
commands and gateway slash menus include project skills
- tools/credential_files.py: project dirs mounted into remote backends
- cli.py: banner notice (loaded count / trust hint)
- hermes_cli/main.py + subcommands/skills.py: hermes skills trust/untrust
- config: skills.project_discovery (default on), skills.trusted_project_dirs
- docs: Project-Local Skills section in skills.md
- tests: tests/agent/test_project_skills.py (18 cases)
Session cwd is fixed at agent build time, so the resolved tier is stable
for the conversation and the system prompt stays byte-stable (cache-safe).
Follow-up to #88523/#88542 from a community report (main agent listed twice,
both rows unnamed @default handles). Two distinct bugs, both specific to
remote-gateway-primary desktops:
1. Phantom "This device" default (electron): the roster enumeration dialed
ensureRegistryBackend for the registry's local entry unconditionally. On a
remote-primary desktop that forces resolveRegistryLocalRoute into the
forced-local branch — SPAWNING a local backend the user never asked for.
That backend enumerates a `default` profile, so a second default agent
appears AND the duplicate-handle rule forces -device suffixes onto the
real one. New shouldDeferLocalEnumeration() treats the forced-local route
as connect-on-demand (same courtesy as undialed SSH sources): the local
entry only enumerates when it is the delegate route (local-primary
desktops, byte-identical behavior) or a forced-local child is already
pooled (the user opened one).
2. Main agent renamed to a connection label (plugin): displayName keyed the
"show the connection label for a default row" rule off sourceScoped —
which annotation also sets on ACTIVE-source rows. A remote-gateway user's
main agent rendered as an IP-derived label (or bare handle) instead of
"Hermes"/their title. Key it off remoteSource: only THIN rows from
another source trade the friendly name for their source label.
Tests: shouldDeferLocalEnumeration route matrix (delegate always enumerates;
forced-local defers until a conn:local:: child exists; bare-key remote
descriptor doesn't count), displayName regression (active default stays
Hermes/title; thin remote default still shows its source label).
Follow-up to the cherry-picked fix for #88391: the bare `streamdown` import
broke the plugin's side-load contract (vm-harness tests and the legacy-sdk
tmpdir loader can't resolve bare specifiers, and the runtime plugin door only
maps @hermes/plugin-sdk and react). Export Streamdown from the SDK instead,
feature-detect it in plugin.js like SkillsView/McpTab (plain-text fallback on
older desktops), and fix the indentation at the render site.
Extends the scheduling test suite from #88407 with behavioral coverage for
the salvaged #79327 work: frames inside the 1000/15 budget reschedule
without repainting, instances over MAX_ANIMATED_INSTANCES draw one static
frame and skip the loop, and the counter releases on unmount. Both tests
verified to fail against the pause-controller-only version.
- Throttle frame rate to ~15fps via timestamp check instead of
redrawing on every requestAnimationFrame callback
- Pause animation when document.hidden, resume on visibilitychange
- Cap concurrent animated instances at 2; extras render a single
static frame instead of starting another animation loop
- Fixes#79077
On hosted deployments a scheduled fire that cannot be forwarded to the
gateway api_server (dead 8642 listener, gateway down) was invisible
outside gui.log: no execution row is created because the claim never
happens, so `cronjob list` showed a healthy job that silently missed
days of scheduled runs (4 consecutive nightly misses in the field,
diagnosed only by log grep).
Changes:
- cron/jobs.py: note_fire_forward_failure() durably stamps
last_fire_error ({at, detail}) on the job record; mark_job_run clears
it on the next successful run so it always describes current
auto-fire health (mirrors preflight_alerted/drift_alerted).
- hermes_cli/web_routers/cron.py: the dashboard fire webhook stamps the
job on the gateway-unreachable path, best-effort (never disturbs the
503/Retry-After retry contract or the OOF-266 intentional-stop drop).
- tools/cronjob_tools.py: _format_job carries last_fire_error so the
agent-facing cronjob list surfaces it.
- hermes_cli/cron.py: `hermes cron list` prints a red
"Missed scheduled fire" line.
- web/: dashboard CronPage renders the miss; api.ts type updated.
- gateway/run.py: one-time startup warning when an external cron
provider is active but the api_server adapter is not running (the
fire path is dead-on-arrival; most common cause is API_SERVER_KEY
missing from an unsupervised gateway relaunch).
- website/docs: cron doc section on missed fires.
A URL-remote desktop whose PRIMARY profile has a per-profile remote
override lists the gateway's sub-profiles in the Bots pane, but
clicking one fell through the routing table's last case and spawned a
fresh local backend that shared nothing but the name (#88296).
resolveProfileBackendRoute now consults primaryRemoteActive: when the
primary's own backend is remote and the sub-profile has no stored
entry of its own, it routes through the primary gateway with profile
scoping (the same shared-primary flow global remote uses). Profiles
with their own local entries still pool locally.
Follow-up to the salvaged #88219 visibility/15fps work:
- Dormancy: the rAF loop stops scheduling frames when no faces are mounted
or none are visible, instead of running the 1Hz whole-document shadow-root
scan forever. A mounting BotFace or a face scrolling into view wakes it.
- Teardown: register() now hooks ctx.onDispose so disabling the Bots plugin
(or a hot reload) cancels the animation frame, disconnects the
IntersectionObserver, drops cached nodes, and clears window.__hbFaceClock.
Previously the loop ran until app restart even with the plugin disabled.
- Behavioral tests for park/wake/stop via a vm-extracted clock harness,
verified to fail against the pre-fix source.
The live-checkout git mutation guard blocked history-rewriting git ops
(checkout, reset --hard, rebase, cherry-pick, ...) in the running source
checkout and its worktrees on every platform. The hazard it protects
against is only real on Windows, where NTFS locks loaded module files and
an in-place rewrite can corrupt the running process. On POSIX, open file
handles pin the old inodes, so a checkout swap under a running process is
safe, and the guard mostly taxed normal dev/salvage workflows with clone
workarounds.
- tools/self_repo_guard.py: add guard_active() -> os.name == "nt"
- tools/terminal_tool.py: consult guard_active() before running the
detector; detector logic and block message unchanged for Windows
- tests: wiring tests force the guard on; new tests cover the POSIX
pass-through and the platform predicate
Layers on the salvaged #88489 (@29206394) and #88341 (@frizikk):
- sdk: host.activeConnectionId() — registry id of the LIVE active gateway.
The salvaged fix classifies against the registry primary; after the user
activates a non-primary source's agent, profiles.list answers from THAT
source and primary-based matching would duplicate the active source's
agents again. Live id wins, primaryConnectionId is the fallback, the
legacy kind==='local' rule covers older desktops.
- plugin: roster/chip/picker list keys are botRowKey(bot) — source-qualified
(connectionId, name) — so same-named agents on two genuine sources can
never collide as duplicate React keys (the render half of the dupe-bots
smear: name-keyed rows + duplicate names = repeated blocks every poll).
Annotated active-source rows keep the plain-name key, so nothing remounts
when a desktop gains the union roster.
- tests: live-id-beats-primary regression, botRowKey stability, source-shape
anchor refresh.
The union agent roster (host.agents) enumerates EVERY registered connection,
including the active gateway that already answered profiles.list. The plugin
merger treated the active gateway's own agents as rows from other sources
because a remote-primary desktop reports them with connectionKind 'remote',
so every bot appeared twice (baseline) and kept growing with each refetch.
Match union agents to the active gateway via the new primaryConnectionId
field on the roster RPC response and annotate the local rows in place
instead of appending phantom copies. Same-named profiles on genuinely
separate sources (This device, other remotes) still get their own tagged
rows, preserving the @name-device disambiguation rule.
Fall back to the legacy connectionKind==='local' rule when
primaryConnectionId is absent (older Electron builds), so single-source
behavior is byte-identical.
Fixes#88344
/simplify-code residual. The note hard-coded "'commits' and 'dirty' are
UNKNOWN", but the two probes fail independently: a bad base_commit fails
rev-list while `git status` still succeeds, so `dirty` is a REAL measurement
being reported as unknown. Safety was never affected (the worktree is preserved
either way), but telling the parent a measured value is untrustworthy is its own
kind of misreport — and it would push a human toward re-inspecting something
already proven.
`mark_worktree_payload_unproven()` now takes an `unmeasured` argument, and
finalize tracks which probe actually failed. The raising path still disclaims
both, because which probe raised is unknowable there.
Validation: 22/22 tests/tools/test_subagent_worktree.py; ruff + ty clean. New
guard mutation-checked (hard-coding "commits/dirty" back fails it).
Phase 2c fold. The schema guard added in the previous commit read and
AST-parsed delegate_tool's source, which AGENTS.md:1514 bans outright ("Never
read source code in tests" -- it passes when the implementation is subtly
broken and fails on a correct refactor). Extracting the shared factory the rule
prescribes removes the duplication the AST test was invented to police, so one
change resolves both.
- subagent_worktree: new module-level `mark_worktree_payload_unproven()` +
`unproven_worktree_payload()`. Both producers of this schema now call them,
so the payload cannot drift and the note string exists once.
- delegate_tool: the finalize-raised fallback calls the factory instead of
hand-building the dict (-16 lines). The re-import is guarded: the outer
`except` can be entered because the `from tools import subagent_worktree`
itself failed, in which case the name is unbound -- an inline fallback keeps
the flag rather than raising NameError and losing it.
- Test replaced with a BEHAVIORAL equivalent: it calls the real factory and
compares its key set against live `finalize_subagent_worktree()` output. Same
contract, no source reading, refactor-proof, and it actually executes the
code.
Also folded from the same review:
- Fail-closed on an unmeasurable commit count. With no `base_commit` the
rev-list probe never ran, `commits` kept its unproven 0 default, and a clean
tree still reached `git worktree remove --force` + `git branch -D` -- the
exact bug class #88113 is about, on a public function that takes a
caller-supplied dict. Now returns un-inspected instead, with a test driving a
real child commit.
- Per-probe diagnostics: the note said only "rev-list/status non-zero". It now
names WHICH probe failed, its exit code, and a bounded git stderr tail, so
the parent (and the human) can act on first read.
- Dropped the redundant `inspection_ok` bool for a `failed: list` of reasons;
removed the duplicated index-corruption block in favor of the existing
`_break_git_index()` helper.
Validation: 21/21 tests/tools/test_subagent_worktree.py; ruff clean; ty clean
on subagent_worktree.py and 64-vs-64 unchanged on delegate_tool.py (all
pre-existing, verified against the base commit). All 6 guards mutation-checked
twice -- neutering the flag fails 6, reverting production to pre-fix main fails
the same 6. E2E on real git: clean still prunes; corrupt index keeps the work
and reports the real stderr; empty base_commit keeps a committed child.
Review fold on the #88113 follow-up. The new guards asserted implementation
details that a strictly-better future change would break, and the second
producer of the payload schema had no coverage at all.
- The distinguishability test asserted the failure payload was byte-identical
to the genuinely-clean one (`for key in commits/dirty/pruned: assertEqual`).
That freezes the AMBIGUITY as a required property: emitting `commits: None`
for "unknown" would improve exactly what #88113 is about and fail the test.
Now asserts what the parent actually depends on -- both keep the worktree,
and only the flag separates them.
- `assertNotIn("inspection_failed", ok_payload)` pinned key ABSENCE on the
happy path, forbidding an always-present-but-False flag (a legitimately
better JSON contract: stable key set for serializers). Now
`assertFalse(...get("inspection_failed", False))` -- same coverage, tolerant
of that refactor.
- `assertIn("UNKNOWN", note)` coupled tests to one word of English prose, and
was not even a cross-producer contract: delegate_tool's note said "state
unknown" (lowercase), so a copy-edit broke the implied convention. Tests now
assert the note names the worktree AND branch -- the actionable part for a
human -- and both producers' notes were aligned to read as one contract.
- The raises test never proved its patched seam ran (a future short-circuit
before any git call would keep it green while proving nothing). Now checks
`call_count` and mirrors the branch-survival + note-names-path legs its
sibling had.
- NEW `WorktreePayloadSchemaTests`: commit 2's whole point is the schema the
parent reads, but delegate_tool's fallback -- the second producer -- was
verified only by reading. It now AST-parses the real fallback dict literal
and compares against live `finalize_subagent_worktree()` output, so the two
producers cannot drift and the pre-fix leak (repo_root/base_commit, missing
commits/dirty/pruned) cannot come back.
- Docs/docstring drift: the flag has a second trigger (finalization itself
raising, handled in delegate_tool), and the module docstring listed
`inspection_failed` without `note`. Both corrected.
- Extracted the duplicated 5-line "corrupt the index" setup into
`_break_git_index()` beside the file's other module-level helpers.
Validation: 19/19 tests/tools/test_subagent_worktree.py; ruff clean. New
schema guard mutation-checked -- reverting delegate_tool's fallback to the
pre-fix `dict(_worktree_info)` shape fails it. Restores checksum-verified.
The preserved worktree is invisible to the only consumer that can act on it.
Completes the #88113 fix. That change correctly stops the destructive prune
when a git probe fails, but still returns commits=0 / dirty=False -- values
that were never measured. Those are the defaults the prune used to delete on,
so the failure payload is byte-identical to "inspected fine, child left
nothing":
inspection FAILED, uncommitted work kept -> {commits: 0, dirty: False, pruned: False}
inspected OK, child produced nothing -> {commits: 0, dirty: False, pruned: False}
The only failure signal was a logger.warning, and the sole consumer of this
payload is the parent agent reading the serialized delegate_task entry -- it
cannot read logs (no in-repo code reads the key back). So the parent's rational
reading of the failure case is "the child produced no work", which is the exact
wrong conclusion: a worktree possibly full of uncommitted work is preserved and
then never looked at. The data survives but nobody is told to recover it.
Changes:
- subagent_worktree: one _unproven() helper stamps inspection_failed + a note
naming the worktree/branch, warns, and returns the payload. Both unproven
exits route through it, so they cannot drift apart again.
- subagent_worktree: the pre-existing exception path (timeout, OSError, a
non-numeric rev-list stdout) produced the same unproven payload but logged at
DEBUG -- effectively silent. It now takes the same flagged path as a non-zero
exit; identical outcomes get identical reporting.
- delegate_tool: the caller's finalize-raised fallback assigned the
creation-side metadata dict (path/branch/repo_root/base_commit) -- a disjoint
schema missing commits/dirty/pruned. It now emits the same flagged shape, and
logs at WARNING.
- Docs + docstring + module contract now state that pruning requires
affirmative proof, so a future cleanup doesn't "fix" the preserved worktree
by restoring the unconditional prune and reintroducing this P1.
Purely additive: the happy-path payload shape is unchanged, so no existing
reader can break.
Validation:
- 18/18 tests/tools/test_subagent_worktree.py; 127 passed across the delegation
suites (test_delegate, batch_validation, control_actions, timeout_diagnostic).
- 3 new guards mutation-checked: neutering the flag fails all three; reverting
the production file to pre-fix main fails all three. Restores checksum-verified.
- E2E on real git: inspection-failure now returns inspection_failed=true with
work intact on disk; proven-clean still prunes (pruned=true).
finalize_subagent_worktree() treated a non-zero exit from its rev-list
or status probes as proof of the payload defaults (commits=0, clean),
then pruned on them: git worktree remove --force plus branch -D
permanently deleted a child's uncommitted work whenever git could not
inspect the tree (e.g. a corrupted index) (#88113).
A destructive cleanup now requires affirmative proof of zero commits
plus a clean tree. Any non-zero inspection result keeps the worktree
and branch for manual review, with a warning naming both.
The rotation path flushes its un-persisted transcript to the parent (#47202)
and only then calls publish_compression_child. The abort handler rolls back
the in-memory transcript and keeps agent.session_id on the parent - its own
comment says "keep the parent live and discard the stale compacted snapshot" -
but the rows the flush just wrote are not part of what it discards. Every
failed rotation therefore leaves the parent transcript longer than it found
it, whatever the failure was.
That is survivable for a one-off failure and pathological for a sticky one.
A parent row carrying ended_at fails the publish on every attempt and nothing
in this path clears it, so each auto-compaction appends another copy of the
current turn to the transcript it was supposed to shrink. Worse, the growth
then satisfies conversation_compression's own len(durable_parent) >
len(messages) check, so the next attempt adopts the inflated snapshot as if it
were genuine concurrent activity and the in-memory transcript doubles too.
Check that one precondition before writing. It is a plain read of the row the
publish is about to read anyway, and it raises the publish's own message, so
split_status=aborted, failure_class=session_split_failed and the rollback path
are all unchanged; a live parent reaches the flush exactly as before.
Deliberately not extended to the compression lease, which is re-acquirable - a
transient miss there would abort a rotation that would otherwise have
committed. old_session_id moves above the flush so a failure raised from here
takes the same in-memory rollback as any other pre-publish failure.
Scope: this fixes the amplification for every abort cause. It does not fix
what marks a live session as ended in the first place (#88197 Bug 1), which
needs a maintainer decision on end-reason taxonomy and is tracked on the
issue; an affected session still aborts every attempt, it just stops making
itself larger while it does.
Refs #88197
The PR's guard used `(job.get('schedule') or {}).get('kind')` which
crashes with AttributeError when schedule is a raw string (e.g.
'every 5m'), as happens in test_parallel_pool.py fixtures and any
job created via create_job(schedule='every 1h'). Use the
isinstance guard pattern already used at lines 5181 and 5325.
/simplify-code finding: only one-shots carry a run_claim, yet the three
dispatch-failure paths called clear_run_claim unconditionally — each call
acquires _jobs_lock (blocking cross-process flock) and does a full
load_jobs read just to return False for any non-'once' job. The trigger
is exactly a failure storm (interpreter shutdown, EMFILE with N due
jobs): N serialized flock+file reads at the moment the process can least
afford I/O, all guaranteed no-ops for the majority job kind.
Gate at the call site on schedule.kind == 'once'; new mutation-checked
test proves recurring dispatch failures skip the claim I/O entirely.
9/9 tests green; ruff clean.
Brooklyn Nicholson