Skip to content

A forward that T3 refuses is dropped with a notice instead of retried forever - #84

Merged
tomkaczocha merged 8 commits into
mainfrom
t3/factory-issue-74
Oct 17, 2026
Merged

tomkaczocha merged 8 commits into
mainfrom
t3/factory-issue-74

Conversation

@rawphp

@rawphp rawphp commented Oct 16, 2026

Copy link
Copy Markdown

Closes #74

Contract

  • done: a MessageRejectedError on a forwarded comment posts the dropped-comment notice with the reason "T3 refused it", then writes comment-dropped <id>; the comment is never sent again. A refused body edit posts the fixed notice ("The factory did not send the edited issue body to the build session, because T3 refused it. Post the change as a new comment and the factory will send it.") then writes body-edit-dropped <edited_at>, which the ledger reader treats like body-edit-forwarded; a newer edit is still sent. A refusal does not stop the call. Refused items are left out of ForwardOutcome, so an all-refused call returns {comments: 0, bodyEdit: false} and the pass does not hold the gate. A forward-wait <k> under which nothing was sent is cancelled with forward-wait-cancelled <k>; the reader clears waitingAfter for that forward marker and restores lastWait, so advance does not answer running. Transport and timeout failures (SessionsError) and a gone thread keep today's behaviour.
  • out: the pass's notes wording; the reminder and gate send-back paths (they keep their own refusal handling); the tracker ever producing "refused"; softfac's docs.
  • risk: none
  • test: vp test run apps/server/src/factory/runner/forward.test.ts apps/server/src/factory/runner/progress.test.ts → pass (117). At the test: commit's tests against the base product code: 13 failed, 104 passed (red confirmed before ship).

Acceptance criteria

  • A refused comment is dropped, the notice posted with "T3 refused it", then comment-dropped <id>; never sent again; posted once — forward.test.ts "a send T3 refuses is dropped with a notice, and the call goes on"
  • A refused body edit is dropped with the fixed notice, then body-edit-dropped <edited_at>; not sent or reported again; a newer edit still forwarded — "a refused body edit is dropped with a notice and its step, and a newer edit is still sent"; progress.test.ts "keeps the time of every body-edit-dropped step with the forwarded ones"
  • A refusal does not stop the call; later comments and the edit go in order — "a refusal between two sendable comments stops nothing", "a refused comment followed by a sendable edit sends the edit and keeps the wait"
  • A refused item counts as not sent; an all-refused call does not hold the gate; its forward-wait does not keep advance at running — "a call whose every send was refused holds no gate and leaves no wait" (outcome {0,false}, forward-wait-cancelled 1, advance → ready-for-gate, then a fresh marker on the next send); progress.test.ts "a cancel for the idle marker clears the wait and restores the last wait", "a cancel does not undo a marker a send used"
  • Other send failures behave as today — "a failed send (transport) stops the call, and the rest is sent in order under the same ids", "a transport failure after a refusal still stops the call, and the refused item is told next time"; the status-read thread-gone tests are unchanged
  • The two ported retry-on-refusal tests are changed to the new rule and say they depart from softfac on purpose — both carry a "Departs from softfac on purpose (A forward that T3 refuses is dropped with a notice instead of retried forever #74)" comment
  • New unit tests: refused comment; refused body edit (+ newer edit); refusal between two sendable comments; all-refused pass; crash between notice and step — "a crash before the notice says it on the next call, and the step comes after the comment"; plus "a marker left idle by a crash is cancelled on the next call" and "a refused send under a marker another message used keeps the wait"

Gate

  • factory/scripts/check.sh → exit 0 (check-seams; vp test run apps/server/src/factory: 41 files, 1666 tests passed; vp run --filter t3 typecheck; vp run --filter @t3tools/web typecheck). Log: .factory/gate-0.log (not committed).

Models

  • maker: claude-sonnet-5-5 (via host; no models block in .dw/config.yml, so Tom's roster)
  • checker: claude-opus-5-5 (via host)
  • orchestrator: this session, claude-fable-5-1

Review

  • cycles: 1
  • roster: correctness, reliability (read-only), testing (12 named mutants, all reached, 11 killed at 3aeeda02a)
  • fixed: testing-1 (blocker; surviving mutant: deleting !editSent from the cancel condition) and testing-2 (nit) → e353b8d51, a test on an idle session that pins the marker stays when the edit went in; the mutant now fails that test
  • leftovers (non-blockers, dropped on purpose):
    • correctness-1 / reliability-1 (nit): if a call crashes between a forward-wait and its cancel, and the refused comment then disappears before the next pass (deleted, or now reported as a tracker drop), the nothing-to-send early return never cancels the idle marker and the run waits until the time limit. Both reviewers note the same stale wait exists today for a crash between the marker and the first send. Out of this issue's scope; a follow-up could cancel seen.idleForwardWait on the early-return path when the session is still idle at that ordinal.
    • reliability-2 (nit): while GitHub comment posts fail, each pass re-sends every not-yet-noticed refused item once (T3 answers the same refusal), writes a marker/cancel pair, and the failed forward holds the gate for the outage. Bounded by the number of refused items and self-healing; acceptable as is.
  • not verified live: the change was exercised through the runner's fakes only. No live T3 session or GitHub issue was driven to a refusal in this run.

Lessons

  • Read the worktree's branch at Isolate; the harness snapshot can name another. Symptom: the maker's brief said the branch was t3/c19a1b2a (from the session's git status snapshot) while the factory's worktree was on t3/factory-issue-74; the maker noticed and stayed put. Cause: the factory names its worktree branch after the issue, and the snapshot was taken before that. Default: run git branch --show-current at Isolate and put that name in every packet and push.
  • In a pnpm monorepo, prove a test: commit red in place, not in a detached worktree. Symptom: a detached worktree needs its own node_modules and a fresh install to run vitest. Default: with the maker and reviewers idle and git status clean, git checkout <base> -- <the product files> (name each path; zsh does not split an unquoted $VAR), run the contract's test files, then git checkout HEAD -- <the same paths> and run once more for green. Record both counts on the receipt.

Commits

  • 413e6231e test(factory): pin that a forward T3 refuses is dropped with a notice — Make slice (red: 13 failing)
  • 3aeeda02a fix(factory): drop a forward T3 refuses with a notice instead of retrying forever — Make slice (green)
  • e353b8d51 test(factory): a sent edit keeps the forward-wait its refused comments shared — Check cycle 1

Files

  • apps/server/src/factory/runner/building.ts — forward catches a refusal per send, goes on, counts only what was sent, cancels an idle forward-wait, and tells the issue through one tellIssue helper (replaces reportDrops)
  • apps/server/src/factory/runner/progress.ts — BODY_EDIT_DROPPED, FORWARD_WAIT_CANCELLED, Seen.idleForwardWait; the reader treats body-edit-dropped like body-edit-forwarded and applies the cancel
  • apps/server/src/factory/runner/messages.ts — the "T3 refused it" reason text and REFUSED_BODY_EDIT
  • apps/server/src/factory/tracker/github.ts — DropReason gains "refused" (the runner's own; the tracker never produces it)
  • apps/server/src/factory/runner/forward.test.ts — rig can refuse a command id; two pinned softfac tests moved to the new rule; new describe "forward: a send T3 refuses"
  • apps/server/src/factory/runner/progress.test.ts — reader tests for the two new steps and idleForwardWait

Run notes

  • The worktree was fast-forwarded from 214c0e092 to origin/main (5df529b18) at Isolate before any read or write.
  • Repo labels: all seven factory:* labels already existed; none created.

Red: 9 refusal tests in forward.test.ts (the replaced pinned test and
the new "forward: a send T3 refuses" describe) and 4 readers in
progress.test.ts (body-edit-dropped, idle forward-wait, cancel, new
session) fail against the code that treats every send error as
transient.

Issue: #74
…ying forever

A MessageRejectedError on a forwarded comment or body edit is final: T3
keeps the refusal under the command id. forward now drops the item, tells
the issue once (comment-dropped <id> / body-edit-dropped <edited_at>),
goes on with the rest, counts the item as not sent so the gate is not
held, and cancels a forward-wait nothing was sent under
(forward-wait-cancelled <k>) so advance does not wait for a run that
nothing started. Transport failures still stop the call and retry.

Issue: #74
…s shared

Review round 1 of #74: deleting `!editSent` from the cancel condition in
forward survived the suite. The test now runs on an idle session, so the
marker is written, and pins that the edit's send keeps it: no
forward-wait-cancelled, waitingAfter stays 1, advance answers running.

Issue: #74
@rawphp
rawphp marked this pull request as ready for review October 16, 2026 23:22
@rawphp

rawphp commented Oct 16, 2026

Copy link
Copy Markdown
Author

Factory gate: passed (attempt 1, run by the factory in a fresh checkout of the pull request's head e353b8d merged with the base branch tip 5df529b)

Command Exit Time
factory/scripts/check.sh 0 142.4 s

@tomkaczocha
tomkaczocha merged commit 84b09f0 into main Oct 17, 2026
2 checks passed
@rawphp
rawphp deleted the t3/factory-issue-74 branch October 27, 2026 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A forward that T3 refuses is dropped with a notice instead of retried forever

2 participants