Skip to content

Forward instructor comments and edits into a running session - #73

Merged
tomkaczocha merged 6 commits into
mainfrom
t3/factory-issue-22
Oct 16, 2026
Merged

tomkaczocha merged 6 commits into
mainfrom
t3/factory-issue-22

Conversation

@rawphp

@rawphp rawphp commented Oct 16, 2026 •

Copy link
Copy Markdown

Closes #22

Contract

  • done: forward(ctx, run) in apps/server/src/factory/runner/building.ts sends each new instructor comment once and the newest instructor body edit once into the run's T3 thread, with softfac's message wording, command ids (softfac:<run>:comment:<id>, softfac:<run>:body:<edited_at>) and ledger records (comment_forwarded, step body-edit-forwarded <edited_at>), writing forward-wait <k> before a send into an idle thread. The GitHub tracker gains activitySince. The pass runs watch → forward → advance, holds the gate in a pass where forward sent, and starts the gate on every pass while a gate send is pending. A body or title edit by a non-instructor stopping the run is watch, ported before this issue; its tests still pass.
  • also done (review round 1): forward posts softfac's notice on the issue for each dropped instructor comment and records comment-dropped <id>, so a dropped comment is reported once and never forwarded.
  • out: feedback forwarding for a paused run, interruptions and the crash-point sections, Linear, any UI.
  • risk: none. The instructor filter in activitySince is the trust boundary; no auth, schema or secrets change.
  • test: vp test run apps/server/src/factory/runner apps/server/src/factory/tracker/github.test.ts apps/server/src/factory/pass.test.ts apps/server/src/factory/index.test.ts apps/server/src/factory/boundary.test.ts apps/server/src/factory/e2e.test.ts → pass (858 tests); vp run --filter t3 typecheck → pass

Acceptance criteria

  • Instructor comments and body edits on a building issue are sent into its thread with softfac's message wording and forward-wait steps — runner/forward.test.ts ("forward: comments", "forward: a body edit", "forward: waiting for the run a message starts"), runner/messages.test.ts (exact wording), runner/gating.test.ts (the gate half of the idle-session flow and the marker precedence tests), runner/progress.test.ts (forwarded, bodyEdits, lastWait).
  • A body or title edit by someone who is not an instructor makes the run stuck with softfac's reason — runner/building.test.ts describe("watch"), unchanged by this change and green.
  • The five softfac runner test sections are ported and pass — every case has a TS counterpart (checked by the correctness reviewer against src/runner/tests.rs at bf273a1). Two substitutions: RunStatus::Unknown("paused") became preparing/starting because the TS status type has no unknown variant; the_issues_impl_for_github_delegates_activity_since has no trait layer to delegate through, so activitySince is tested directly in tracker/github.test.ts with argv assertions.

Gate

  • factory/scripts/check-seams.sh → exit 0
  • vp test run apps/server/src/factory → exit 0 (41 files, 1514 tests)
  • vp run --filter t3 typecheck → exit 0
  • vp run --filter @t3tools/web typecheck → exit 0
  • factory/scripts/check.sh as a whole → exit 0 (.factory/gate-1.log)

Models

  • maker: claude-sonnet-5-5 (via host)
  • checker: claude-opus-5-5 (via host)

Review

  • cycles: 1
  • roster: correctness, security, reliability (read-only, cycle 1); testing (mutating, once on the ship tip, 12 named mutants, 12 killed, none unreached)
  • fixed: the pass had no pending-gate-send rule (correctness-1 and reliability-1, the same gap): forward skips a run whose gate-sending step has no gate_sent_back, and only the gate writes that line, so the pass must call the gate on every pass while a send is pending, as softfac's advance_and_gate does. Fixed in pass.ts/index.ts with hasPendingGateSend on PassWorld and pass tests for running, reminded, paused, ready-for-gate, stuck and needs-info with a pending send.
  • leftovers: none

Outside the issue, and why

  • apps/server/src/factory/installScripts.test.ts: PR fix(factory): build.sh smoke test passes on a clean build #72 changed build.sh to look for "no factory config" in the smoke server's log but left the test's fake server logging "factory started" and expecting the old failure message, so vp test run apps/server/src/factory, which is this repo's gate, was red on main. The test is aligned with the script (3 lines). Nothing else was done for it.
  • apps/server/src/factory/e2e.test.ts: the fake gh now answers the comments listing with ?since= and the ACTIVITY_QUERY, which forward reads on every pass of a building run.
  • tracker/approvalHistory.ts: isInstructor split out of isHumanInstructor so activitySince can reuse the login match.

Notes

  • The worktree was cut from a stale main (one merge behind origin/main); fast-forwarded to 676303a69 before provisioning.
  • No repo labels were created: all seven factory:* labels already existed.
  • vp i ran as provisioning.

Lessons

  • A reviewer's tree copy under the worktree joins the gate's test run. Symptom: the gate reports dozens of failed test files with every test passing; the failed files are under .factory/<reviewer>/apps/.... Cause: vp test run apps/server/src/factory filters by path substring, so a copy of the tree anywhere under the worktree matches too, and the reviewer could not remove its copy because the harness denied rm -rf. Default: the copy's path always contains the filtered substring, so it must be gone before the gate; when rm -rf is denied, remove it with node -e "fs.rmSync(..., {recursive:true})", and run the gate only after ls .factory/ shows no tree copy.
  • The worktree's branch is what git branch --show-current says, not what the launch snapshot says. Symptom: git push -u origin <name from the session's git status snapshot> fails with "src refspec does not match any". Cause: the harness's status snapshot named a different branch from the one the factory checked out. Default: read the branch with git branch --show-current at Isolate and use that on every push.

Commits

  • f40601a5c test: port softfac's forwarding tests — Make slice (red: the tests import forward, activitySince and the new Seen fields, none of which existed)
  • 782fbc4ab feat: forward instructor comments and body edits into the session — Make slice
  • 01efd0894 fix(test): the build smoke test expects "no factory config" like build.sh — gate (main was red)
  • 514caa2d2 test: the pass starts the gate while a gate send is pending — Check cycle 1 (red: hasPendingGateSend did not exist)
  • c90edd9be fix: the pass starts the gate while a gate send is pending — Check cycle 1

Files

  • apps/server/src/factory/runner/building.ts — forward, ForwardOutcome, SkipReason, gateSendPending exported
  • apps/server/src/factory/runner/run.ts — applyActions carries out forward-comment and forward-body-edit with a ForwardPayload
  • apps/server/src/factory/runner/progress.ts — Seen.forwarded, bodyEdits, lastWait; BODY_EDIT_FORWARDED
  • apps/server/src/factory/runner/messages.ts — cleanInstruction, instructionText, loginText, commentMessage, bodyEditMessage
  • apps/server/src/factory/tracker/github.ts — activitySince, ACTIVITY_QUERY, COMMENT_EDITOR_QUERY, checkEdits, the Activity types
  • apps/server/src/factory/tracker/approvalHistory.ts — isInstructor
  • apps/server/src/factory/pass.ts — watch → forward → advance; gate hold when sent; gate on a pending send
  • apps/server/src/factory/index.ts — wires forward and hasPendingGateSend
  • apps/server/src/factory/runner/forward.test.ts — the ported sections (new)
  • apps/server/src/factory/runner/{messages,progress,gating,building}.test.ts, tracker/github.test.ts, pass.test.ts, e2e.test.ts, installScripts.test.ts — tests

The tests need forward, activitySince, the new Seen fields, the two
message builders and the forward actions of applyActions, none of which
exist yet: they do not compile and fail until the next commit.

Issue: #22
forward sends the instructors' new comments and their edit of the issue
body into a building run's session, once each, under softfac's message
wording, command ids and forward-wait steps. The pass runs it between
watch and advance and holds the gate in a pass where it sent something.
activitySince reads the activity from GitHub.

Issue: #22
…d.sh

PR #72 changed build.sh to look for "no factory config" in the smoke
server's log but left the test's fake server logging "factory started"
and expecting the old failure message, so the factory suite was red on
main. The gate for this branch runs that suite.

Issue: #22
Fails: a running, reminded or paused run with a pending gate send does not
start the gate, a pending send does not win over the gate waiting for a sent
comment, and a ledger read error for the pending check is not recorded.

Issue: #22
@rawphp
rawphp marked this pull request as ready for review October 16, 2026 15:05
@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 c90edd9 merged with the base branch tip 676303a)

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

Port softfac's dropped-comment notice into `forward`: after the sends, each
comment `activitySince` drops gets one comment on the issue in softfac's fixed
words and a `comment-dropped <id>` step, and a comment told about as dropped
is never forwarded later (f10).

Look up the editors of edited comments in one GraphQL call per 100 ids
instead of one per comment (f5). Rename `GatePayload` to `ActionPayload`,
since forwards carry their message on it too (f7). Pin the live
`hasPendingGateSend` wiring (f3) and a failed pending-send read for a
finished session (f4) with tests.

Fixes f3, f4, f5, f7, f10.
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.

Forward instructor comments and edits into a running session

2 participants