Skip to content

Forwarding trusts only a person's edits, not a GitHub App with an instructor's login - #83

Merged
tomkaczocha merged 3 commits into
mainfrom
t3/factory-issue-75
Oct 17, 2026
Merged

tomkaczocha merged 3 commits into
mainfrom
t3/factory-issue-75

Conversation

@rawphp

@rawphp rawphp commented Oct 16, 2026

Copy link
Copy Markdown

Closes #75

Contract

  • done: ACTIVITY_QUERY asks for __typename on editor and on every userContentEdits editor; activitySince trusts a body edit only when the last editor and every edit after since pass isHumanInstructor (a non-User editor gives no body edit); checkEdits uses isHumanInstructor for the last editor and every history entry, so an App-edited comment is dropped once with reason edited-by-other; forwarding fixtures carry __typename: "User" on human editors; new tests cover a Bot body edit and a Bot comment edit whose login matches an instructor; REST comment authorship (user.login via isInstructor) is unchanged.
  • out: the approval path (already human-checked), the REST author check, any other query or adapter.
  • risk: none
  • test: cd apps/server && vp test run src/factory/tracker/github.test.ts src/factory/tracker/approvalHistory.test.ts → pass (277 tests)

Acceptance criteria

  • ACTIVITY_QUERY asks for __typename on editor and on every userContentEdits editor — test "the activity query asks only for the body edit" (github.test.ts), which names the top-level body lastEditedAt editor { __typename login } and the userContentEdits editor separately
  • activitySince checks body edits with isHumanInstructor: the last editor and every edit after since; a non-User editor gives no body edit — test "a body edit by an App with an instructor's login is not forwarded": App last, App in between, editor with no __typename, App last above a person-looking history; a person's edit still forwarded
  • checkEdits uses isHumanInstructor for the last editor and every history entry; an App-edited comment is dropped once with edited-by-other — test "a comment edited by an App with an instructor's login is dropped once": App last, App in between, App last above a cut-off history
  • Forwarding fixtures carry __typename: "User" on human editors — actorOf, bodyReplyRaw, editorReplyRaw and the two inline real-shape fixtures in github.test.ts
  • Comment authorship from the REST listing (user.login) is unchanged — github.ts activitySince still filters REST rows with isInstructor(c.user, …); test "activity keeps only instructors, case-insensitively" unchanged

Gate

  • factory/scripts/check.sh → exit 0 (seams, 41 factory test files / 1654 tests, server and web typecheck)

Models

  • maker: claude-sonnet-5-5 (host)
  • checker: claude-opus-5-5 (host)
  • No models block in .dw/config.yml; Tom's default roster used.

Review

  • cycles: 1
  • correctness: approve, no findings. security: approve, no findings.
  • testing (mutation pass, 7 named mutants, 4/7 killed at first tip): 2 actions and 1 nit, all test-only gaps for behavior that already held. Fixed in one test: commit; the 3 survivors (login-only lastOther, login-only last-editor guard, __typename dropped from the top-level editor) were re-run against the new tip and all die. 7/7 killed, 0 unreached.
  • fixed: query assertion anchored to the top-level editor field; App last editor above a cut-off history dropped as edited-by-other; App last editor above a person-looking history gives no body edit.
  • leftovers: none

Lessons

  • Read the branch name from the worktree, not from the launch snapshot. Symptom: the first push failed with src refspec t3/13995e6d does not match any. Cause: the session's git status snapshot named the worktree directory's id as the branch, but the factory had checked out t3/factory-issue-75. Default: at Isolate run git branch --show-current and use that name for every push and PR command.

Notes

  • Worktree was behind origin/main at Isolate (214c0e092, clean); fast-forwarded to 5df529b18 before provisioning.
  • All seven factory:* labels already existed; none created.
  • Red phase proven: at the test: commit, 3 tests fail (266 run); at the fix: commit, all green.

Commits

  • 132a19aa1 test(factory): forwarding fixtures carry __typename and Bot editors are refused — Make slice (red)
  • d73925caa fix(factory): forwarding trusts only a person's edits, not an App with an instructor's login — Make slice (green)
  • 0ca275b1a test(factory): pin the top-level editor typename and the App-last-editor cases — Check cycle 1 (tests only, no red phase)

Files

  • apps/server/src/factory/tracker/github.ts — ACTIVITY_QUERY asks for __typename; editor schemas use Actor; checkEdits and activitySince judge editors with isHumanInstructor
  • apps/server/src/factory/tracker/github.test.ts — fixtures carry the typename, botOf helper, Bot body-edit and Bot comment-edit tests, anchored query assertion

…re refused

Red against the old code: the activity query test (no __typename on editor), the body edit by an App test (bodyEdit forwarded), and the comment edited by an App test (comment kept).

Issue: #75
…h an instructor's login

ACTIVITY_QUERY asks for __typename on every editor. activitySince and checkEdits judge editors with isHumanInstructor, so a GitHub App whose login matches an instructor no longer passes when it edits the body or a comment. REST comment authorship is unchanged.

Issue: #75
…tor cases

The query assertion now names the top-level editor field, a comment whose App last editor sits above a cut-off history is dropped as edited-by-other, and a body whose last editor is an App is refused even when the history looks like a person's. Tests for behavior that already holds; no red phase.

Issue: #75
@rawphp
rawphp marked this pull request as ready for review October 16, 2026 23:13
@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 0ca275b merged with the base branch tip 5df529b)

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

@tomkaczocha
tomkaczocha merged commit 3152e75 into main Oct 17, 2026
2 checks passed
@rawphp
rawphp deleted the t3/factory-issue-75 branch October 28, 2026 04:06
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.

Forwarding trusts only a person's edits, not a GitHub App with an instructor's login

2 participants