Skip to content

chore(loop): drop the nonexistent sender.author_association claim test - #81

Merged
Alex1990 merged 1 commit into
mainfrom
fix/77-human-claim-payload
Oct 21, 2026
Merged

Alex1990 merged 1 commit into
mainfrom
fix/77-human-claim-payload

Conversation

@Alex1990

Copy link
Copy Markdown
Owner

What / why

Follow-up to #80, which fixed the right gap with one wrong field.

The claim predicate gated on event.sender.author_association. That field exists on the issue, not on the sender (sender is a simple-user, which has no such property), so the test was always undefined → null, and the very next real run still logged:

[loop] event=issues.labeled → run (issues.labeled ready-for-agent) · writeLevel=auto
[loop] skip: task #77 not claimable (status=waiting-human) — event handled idempotently

Change

  • humanClaimOf no longer inspects author_association. Permission needs no local test at all: editing labels requires triage/write access, so the existence of an issues.labeled run already is a maintainer's act (a GITHUB_TOKEN-raised one never runs). The predicate now excludes only bots (type === 'Bot' or a [bot] login).
  • scripts/loop/README.md D37 records the failure and the lesson: verify payload fields against a real event payload, not a hand-written one.

Proof

Predicate run against the real event shape (sender without author_association), plus the blocking cases:

real payload / waiting-human   "Alex1990"   <- accepted (was null before this PR)
real payload / ready           null
real payload / waiting-merge   null
bot sender                     null
bot login, no type             null
needs-triage label             null
issue_comment                  null
route: {"action":"run","taskId":77,"kind":"issue","stage":"feature","reason":"issues.labeled ready-for-agent"}

End-to-end proof is the post-merge run on #77: re-applying ready-for-agent must log human claim: task #77 waiting-human → ready followed by run ... started (... stage=feature).

Risk

Host scripts only, no package surface. The only widening versus #80 is that the requester's own permission is now taken on trust from GitHub's label-edit rule instead of re-checked; a bot cannot do it, and an outside contributor cannot edit labels at all.

@Alex1990
Alex1990 force-pushed the fix/77-human-claim-payload branch from c61a327 to 854b703 Compare October 21, 2026 15:45
@github-actions

Copy link
Copy Markdown
Contributor

Coverage

Metric % Covered / Total
Lines 91.73% 1121/1222
Statements 90.42% 1209/1337
Functions 92.76% 218/235
Branches 76.97% 575/747

@Alex1990
Alex1990 merged commit 04fe0a5 into main Oct 21, 2026
3 checks passed
@Alex1990
Alex1990 deleted the fix/77-human-claim-payload branch October 21, 2026 15:47
@github-actions github-actions Bot added the needs-triage Maintainer needs to evaluate (loop can't decide; too risky; external PR) label Oct 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Loop triage — needs-triage (owner review + merge).

This is an owner-authored, same-repo PR (fix/77-human-claim-payload, no loop-task: marker), and the change is already implemented, so there is no agent-actionable work left. The loop should also not be the checker of the predicate that gates its own entry. Routing to the human inbox rather than labelling a finished PR ready-for-agent.

Verified read-only on head 854b703 (tree == head, clean; base main 5c8d725; MERGEABLE, BLOCKED only by REVIEW_REQUIRED; CI Lint + Test pass in run 39628636476).

  • The defect is real, twice over. The pre-PR predicate re-run with a verbatim simple-user sender (no author_association) returns null, and only passed a hand-written sender.author_association. In production, the very next issues run after chore(loop): let a maintainer's claim label exit the waiting-human inbox #80 merged — 39626786589 (head 5c8d725, 15:34:27Z, from the 15:33:35Z label on Save every session created by agent in loop action #77) — logged skip: task #77 not claimable (status=waiting-human).
  • The fix behaves as claimed, exercised against the real module on this head: real payload / waiting-human → "Alex1990"; ready, waiting-merge, waiting-info, bot sender (type: Bot), bare [bot] login, missing sender, non-claim label, issue_comment and opened → null; route labeled → run(taskId 77, kind issue, stage feature); route comment → inbox.
  • Wiring is intact: entry.mjs:293 passes the raw payload (ctx built at :515), the hit sets status = 'ready' before claimability(), and no author_association reference is left under scripts/loop/**.
  • Scope is right: host-only (+20/−4), no package surface, no workflow/norms files, chore(loop) type.

After merging, the end-to-end proof still has to run: #77 currently carries ready-for-agent from 15:33:35Z, but that event is spent (its run was the skip above), so remove the label and add it back. Expect human claim: task #77 waiting-human → ready then run … started (… stage=feature).

Non-blocking, for later: D37's lesson is prose only — scripts/loop/** is outside every repo gate and has no test harness, so capturing a real issues.labeled payload as a fixture when one exists is worth doing; and the door remains issues-only (pull_request.labeled is still a noop).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-triage Maintainer needs to evaluate (loop can't decide; too risky; external PR)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant