fix(fusion): retry the harness author once when the transport stalls

[BaoYunkai]

Problem

A fusion lane costs hours; an authoring call costs minutes. But an author whose
transport never delivered an answer -- a stream stalled mid-response, a
connection reset -- ended the lane on that first failure, discarding the whole
lane's remaining budget for a fault that one more call would very likely clear.

Fix

The classification this needs already exists one layer down. llm_failure
opens with exactly this proposition -- "Tell 'the model never answered' apart
from 'the model answered nothing'" -- and ships classify_llm_error,
RETRYABLE_KINDS, retry_delay and the env plumbing, which discover.py
already drives.

So the retry goes in _run_registered_author, where the exception object is
still in hand, rather than reading a log back and substring-matching it:

• RETRYABLE_KINDS -> retry with backoff, against a deadline.
• A provider safety stop -> never retried. Retrying one is the anti-pattern the
  session-resume allowlist already refuses, and _with_run_error deliberately
  appends the transport error to a safety verdict, so a text-matching
  classifier reads that case as retryable when it must not be.
• A timeout -> never retried. It has just spent a full attempt's budget.

Both run_author call sites are covered, the harness author and the kernel
author, and staging is never touched, so a retry cannot collide with a harness
the failed attempt already wrote.

The second commit sizes the deadline. The gate asks whether the next attempt
can plausibly finish: elapsed + delay + timeout_s >= deadline. Read from
llm_failure.DEFAULT_DEADLINE_SEC that is 1800s, sized for discover.py's short
calls, while an authoring attempt is allowed timeout_s, defaulting to 7200s
-- so the gate evaluated elapsed + delay + 7200 >= 1800 and the retry could
never start. The default budget is now what the configured attempts can
legitimately cost; an operator-set FORGE_LLM_RETRY_DEADLINE_SEC is still
enforced.

Test plan

• src/kernelforge/tests/fusion/test_author_extra.py (54 passed), asserting
  on AUTHOR_RC_*: a retryable failure retries, safety does not, timeout
  does not, a first attempt that works does not, and two failures stop
• A stalled transport is retried at timeout_s=7200, and an operator
  deadline too short to hold another attempt stops it -- the existing tests
  passed timeout_s=1, which is the only reason the dead gate went unseen
• src/kernelforge/tests/fusion/ (602 passed)
• ruff check . + ruff format --check . on the changed files

[ZhengGong-amd]

Request changes — please rewrite this in author.py rather than patch it here

The need is real: an author that dies on the transport should not cost the lane. But the mechanism is in the wrong module, and as written it does not fire in the scenario the PR describes — it makes that scenario worse.

1. In your own scenario the retry never happens, and the run is left wedged

The PR body says the author "had already written the harness" when the stream stalled. So the staging target exists. _prepare_author_harness refuses when the target exists — it does not clean up; _finish_author_harness owns that:

if target.exists() or target.is_symlink():
    return False, "author harness staging target already exists", True

So the retry path returns early, skipping _finish_author_harness and leaking the staged file. Since _author_harness_target is a stable sha256 of the output dir, the leak persists across runs.

Measured on this head vs origin/main, real staging helpers, only run_author mocked:

	main	this PR
stall after the author staged a harness	harness author exited 1, staging cleaned	no retry (attempts=[0]), author harness staging target already exists, staging leaked
second run, transport healthy	succeeds	fails identically — run_author is never called

One stall used to cost a lane. Now it costs the lane and wedges every rerun against that output dir until someone deletes .forge_fusion/kernel_harness_<digest>.py by hand. The reason string also changes from the transport failure to a staging failure, and that string is what FusionAbort carries into the manifest.

CI is green because all five tests stub the one new branch:

monkeypatch.setattr(cli, "_prepare_author_harness", lambda *_a, **_k: (True, "", False))

2. The classifier already exists, one layer down

fusion/llm_failure.py opens with exactly this PR's proposition:

"""Tell \"the model never answered\" apart from \"the model answered nothing\"."""

and ships classify_llm_error, RETRYABLE_KINDS ("Kinds a retry can still fix"), _error_chain, retry_delay, DEFAULT_ATTEMPTS, DEFAULT_DEADLINE_SEC, env_setting. discover.py already drives the full loop with them. author.py:28 already imports from it, and _run_registered_author holds the exception object while classifying into AUTHOR_RC_SAFETY / AUTHOR_RC_TIMEOUT / AUTHOR_RC_FAILED.

This PR serialises that exception to a log, writes it to disk, reads the last 4 KB back in another module, and substring-matches it. The information is destroyed in one place and reconstructed by grep in another.

3. The reconstruction is wrong — three "never retry" cases come back retryable

Called directly on this head:

log (real path that produces it)	retry?	_author_failure_is_transient
...; the agent run also failed: APIConnectionError: Connection error. → AUTHOR_RC_SAFETY	no	True
TimeoutError: Request timed out after 7200s → AUTHOR_RC_TIMEOUT	no	True
...the tool returned an api error earlier so I stopped (task refusal)	no	True
API Error: Response stalled mid-stream. → AUTHOR_RC_FAILED	yes	True

Neither the safety nor the timeout case is hypothetical. _with_run_error deliberately appends the transport error to a safety verdict, so that path reliably carries a marker — and author.py:1217 says retrying a safety stop "is exactly the anti-pattern the session-resume allowlist already refuses". And _agent_timeout_sec() defaults to 7200 s, so retrying a timeout burns another two hours — the very budget this PR argues it is protecting. "The author costs minutes" only holds on the happy path.

What I'd like instead

Put it where the exception still exists, in _run_registered_author, after the two existing branches:

llm_failure.py   classify_llm_error / RETRYABLE_KINDS / retry_delay   ← one vocabulary, already there
  └─ author.py   SAFETY  -> never retry (branch + rationale already written)
                 TIMEOUT -> never retry (it just spent the whole per-attempt budget)
                 RETRYABLE_KINDS -> retry with backoff and a deadline
       └─ command.py   unchanged

Smaller than this diff, and it drops out for free:

• no marker table, no log reading, no harness_author.stalled.log (each attempt writes its own log at that layer);
• staging is never touched, so §1 cannot happen;
• safety and timeout already returned, so §3 cannot happen;
• both run_author call sites are covered — this PR only fixes command.py:307, not the kernel author at :855.

Two more things worth carrying over: there is no backoff (a stalled upstream usually stalls again immediately) and no deadline, while discover.py has both and test_task_preparer_retry_budget.py already encodes the rule — "A retry that cannot plausibly finish must not be started."

Please also fix, in the rewrite

• Move the tests to test_author_extra.py (28 run_author scenarios, AUTHOR_RC_* assertions already there) and assert on rc: FAILED+retryable retries, SAFETY does not, TIMEOUT does not, OK does not, two failures stop. Don't monkeypatch the function under test, and do assert the returned (ok, error) — none of the five tests currently does.
• return attempts if ok or True else attempts is dead — ok or True is constant.
• test_the_retry_is_not_repeated asserts the same thing as test_a_stalled_stream_is_retried_once.
• Two comments state things the code does not do: # The failed attempt may have left a partial harness staged. (the next line gives up instead of handling it — this is the cause of §1) and "a model that cannot write a conforming harness ... must not be retried" (row 3 of the table above).
• contextlib.suppress(OSError) around the log copy loses the failed attempt's log silently, right before the retry overwrites it — that is the one piece of evidence this change exists to keep.

Please don't patch around these

Each of these would turn a test green and leave the cause in place:

1. deleting the stale staging file in the retry path;
2. adding negative markers to exclude safety verdicts;
3. adding a "log mentions timeout" exception.

They all compensate for the same root cause: the decision is made in the caller, one layer up from where the exception still exists. Move it down into author.py and all three disappear.

[haishuok0525]

What this PR does

HEAD stacks two retry mechanisms for fusion authoring:

1. author.py _run_registered_author: exception / end_reason classification, default 2 attempts, backoff, FORGE_LLM_RETRY_DEADLINE_SEC.
2. command.py _author_baseline_harness: after a failed run_author, if the log tail matches _TRANSIENT_AUTHOR_MARKERS, call run_author again.

Commit graph: cf4bab81 (exception path) then c0212fe (log-marker path on top). Neither removes the other.

Checked against HEAD c0212fe: author.py retry loop + classification, command.py outer retry + _prepare_author_harness, llm_failure.DEFAULT_DEADLINE_SEC=1800, _agent_timeout_sec default 7200, both test files.

Blocking issues

1. Two retry layers on HEAD. Outer _author_baseline_harness always calls run_author, which already loops. Keep one path (the author.py one matches the second commit intent) and delete the log-marker loop in command.py.

2. Inner retry is dead under defaults. Gate is elapsed + delay + timeout_s >= deadline with timeout_s default 7200 and DEFAULT_DEADLINE_SEC 1800, so after a retryable first failure the second attempt is never started. Non-harness run_author call sites therefore get no effective retry unless the deadline env is raised above one full attempt.

3. Motivating stall after a written harness fails the outer retry. On retry, _prepare_author_harness returns "author harness staging target already exists" when the staging file is still there (command.py ~206-207, ~368-371), then returns before _finish_author_harness. Session shape "harness written, then stream stall" does not get a second author run; staging can leak. test_harness_author_retry.py stubs _prepare_author_harness to always succeed, so CI misses this.

4. Outer marker classification retries cases the inner path correctly refuses. Tail markers include request timed out (rc 124 still retried) and generic connection error / sdk error / api error, which can match safety/_with_run_error text or a task refusal that mentions an earlier API error.

5. PR body matches only the log-marker approach, not the stacked author.py retry. Please align the description with the single remaining design after the fix.

6. Missing CHANGELOG.md Unreleased entry.

[xiaofei-zheng]

What this PR does

The retry moved down to where the exception object still exists. _run_registered_author (author.py:1066-1126) now loops over _run_registered_author_once, which returns (rc, retryable); classification uses classify_llm_error / RETRYABLE_KINDS / retry_delay from llm_failure. The log-marker layer in command.py is gone -- command.py is not in this diff at all. The second commit sizes the retry budget to what an attempt actually costs.

No blocking issues -- LGTM

Verified rather than skimmed:

• One retry layer. command.py unchanged; the loop lives inside run_author -> _run_registered_author (author.py:1376), so both call sites (command.py:307 harness author and command.py:855 kernel author) are covered without either touching _prepare_author_harness / _finish_author_harness. The "staging target already exists" wedge and the leaked kernel_harness_<digest>.py cannot occur.
• The gate is live under defaults. default_deadline = attempts * (timeout_s + max_delay) (author.py:1088) = 2 * (7200 + 120) = 14640s, so after a first retryable failure elapsed + delay + 7200 >= 14640 is false and the second attempt starts. An operator-set FORGE_LLM_RETRY_DEADLINE_SEC is still enforced; deadline > 0 as the disable sentinel matches discover.py:1177. test_author_extra.py covers both directions at a real timeout_s=7200.
• Safety and timeout never retry. author.py:1256/1276/1305 return (AUTHOR_RC_SAFETY, False); author.py:1315 returns (AUTHOR_RC_TIMEOUT, False), and that branch is checked ahead of the retryable one, so a safety verdict carrying an appended transport error from _with_run_error cannot be read as retryable.
• Non-Exception escapes are preserved. author.py:1290 re-raises when run_error is not an Exception, so KeyboardInterrupt / SystemExit are not swallowed as retryable transport faults.
• The marker table, the log tail read, harness_author.stalled.log, test_harness_author_retry.py, the ok or True dead expression and the duplicate test are all gone (grep-confirmed). New tests assert on AUTHOR_RC_* and real call counts with a scripted backend, monkeypatching only author.time.sleep.
• PR description now matches the author.py design, CHANGELOG.md has its Unreleased entry, CI is green.