Cloonar/coding-lab

Fork 0

feat(notify): done-signal trigger — notify when an AFK run opens its PR/change request #100

New issue

Closed

opened 2026-07-10 15:11:54 +02:00 by dominik.polakovics · 2 comments

dominik.polakovics commented

2026-07-10 15:11:54 +02:00

Owner

What to build

Push a notification when the reaper detects an AFK run's done-signal (a PR or change request whose head branch equals the run's branch) — "the agent finished; review and merge is on you now."

Hook at the reaper's done-signal detection site as an injected seam (same pattern as the needs-input seam and the metrics AFKRunEnded seam), covering both tracker bindings through the one Tracker contract: on a forge-bound repo the done-signal is a PR, on a builtin-bound repo a change request.
Payload: title <repo>~<label> opened PR #<n> (or the change-request equivalent), body = the PR/CR title, tag = <run-id>, tap deep-links to /runs/:id inside the PWA — lab's chat is where review/merge context lives; never bounce out to the forge URL.
Fires once per run (the done-signal is detected once and terminal); no notifications for deaths, timeouts, or the three-strikes pause in this slice.
AFK runs only — a manual instance has no done-signal.

Acceptance criteria

Reaper detecting a done-signal produces exactly one send with title, PR/CR title body, tag = run-id, and the /runs/:id route (hermetic: fake sender + the existing fake tracker)
Both tracker bindings covered: forge PR and builtin change request produce equivalent payloads
Deaths, timeouts, neutral Stop, and three-strikes pause produce no send
A blocked or failing sender never delays the reaper

Blocked by

## What to build Push a notification when the reaper detects an AFK run's **done-signal** (a PR or change request whose head branch equals the run's branch) — "the agent finished; review and merge is on you now." - **Hook at the reaper's done-signal detection site** as an injected seam (same pattern as the needs-input seam and the metrics `AFKRunEnded` seam), covering both tracker bindings through the one Tracker contract: on a `forge`-bound repo the done-signal is a PR, on a `builtin`-bound repo a change request. - **Payload**: title `<repo>~<label> opened PR #<n>` (or the change-request equivalent), body = the PR/CR title, `tag = <run-id>`, tap deep-links to `/runs/:id` inside the PWA — lab's chat is where review/merge context lives; never bounce out to the forge URL. - Fires once per run (the done-signal is detected once and terminal); no notifications for deaths, timeouts, or the three-strikes pause in this slice. - AFK runs only — a manual instance has no done-signal. ## Acceptance criteria - [ ] Reaper detecting a done-signal produces exactly one send with title, PR/CR title body, `tag = run-id`, and the `/runs/:id` route (hermetic: fake sender + the existing fake tracker) - [ ] Both tracker bindings covered: forge PR and builtin change request produce equivalent payloads - [ ] Deaths, timeouts, neutral Stop, and three-strikes pause produce no send - [ ] A blocked or failing sender never delays the reaper ## Blocked by - #98

dominik.polakovics added the

needs-triage

label

2026-07-10 15:11:54 +02:00

dominik.polakovics referenced this issue

2026-07-10 15:12:07 +02:00

feat(notify): terse lock-screen mode — content-free notification toggle #101

dominik.polakovics referenced this issue

2026-07-10 15:30:59 +02:00

feat(notify): Web Push plumbing end-to-end — VAPID key file, subscriptions, sender, service-worker handlers, settings enrollment #98

dominik.polakovics referenced this issue

2026-07-10 16:48:20 +02:00

feat(notify): needs-input trigger — notifier seam at the chat tailer's conversational-state edge #99

dominik.polakovics commented

2026-07-10 16:48:20 +02:00

Author

Owner

This was generated by AI during triage.

Agent Brief

Category: enhancement
Summary: Fire a push notification when the reaper detects an AFK run's done-signal — a PR (forge binding) or change request (builtin binding) whose head branch equals the run's branch. The blocker (#98) has landed; this hooks the second trigger into the shipped sender.

Current behavior:
The reaper already detects the done-signal: it lists the repo's pulls through the Tracker contract, checks head-branch presence (open or merged counts, closed does not), classifies the run, and — exactly once per run, via an idempotent claim — executes the terminal reap. That reap site already carries one injected, nil-safe seam: the metrics hook that records how an AFK run ended (AFKRunEnded). Nothing there sends notifications. The landed push sender (Broadcast(Payload{Title, Body, Tag, Route})) is fire-and-forget and never blocks or errors at the call site.

Desired behavior:
When the reaper's terminal classification of an AFK run is success (done-signal present), one notification is broadcast:

Title: <repo>~<label> opened PR #<n> on a forge-bound repo, or the change-request equivalent on a builtin-bound repo. Repo and run fields (repo name, session name, branch, run ID) are all in scope at the reap site.
Body: the PR/CR title. Known gap to design around: the pull listing on the reaper's hot path carries number, head branch, state, and URL — but not the title; the title lives on the tracker's heavier per-pull detail fetch. Fetching the detail once on the success path is acceptable (it fires once per run); if that fetch fails, degrade the body to PR #<n> rather than blocking or skipping the notification. Also note the PR's number currently doesn't flow down to the terminal reap step — only a presence boolean does — so the matched pull ref must be threaded down or re-derived there.
Tag: the run ID (coalesces with any earlier needs-input item for the same run — the done notification replaces it on the lock screen).
Route: the run's chat route (/runs/<id>) — lab's chat is where review/merge context lives; never a forge URL, which would bounce the user out of the PWA.

Semantics:

Fires exactly once per run — the done-signal reap is already claimed idempotently, so no additional debounce state is needed; lean on that.
Success only: deaths, timeouts, a neutral Stop, and the three-strikes pause produce no send in this slice.
AFK runs only — a manual instance has no done-signal.
The reaper is never delayed: the seam call is fire-and-forget; any synchronous tracker detail fetch for the title happens on the success path only and must tolerate failure without affecting the reap.

Key interfaces:

Mirror the metrics seam exactly: the AFK service's options gain an optional notifier next to the existing nil-safe metrics hook, fired beside AFKRunEnded when the outcome is success. A nil notifier means no notifications, no other behavior change.
One Tracker contract covers both bindings: the same pull-listing/head-match logic serves forge PRs and builtin change requests, so there is no per-binding trigger logic — only payload wording differs.
push.Sender is a concrete struct (no interface); for hermetic tests either define a narrow local notifier interface to record payloads, or use the landed recipe (real sender + httptest fake gateway + Flush()), alongside the existing fake tracker.

Acceptance criteria:

A reap classifying an AFK run as success produces exactly one broadcast with the title, PR/CR-title body, tag = run ID, and the run's chat route (hermetic: fake/recorded sender + the existing fake tracker)
Both tracker bindings produce equivalent payloads: forge PR and builtin change request
A failed title-detail fetch degrades the body to the PR/CR number form and still sends
Deaths, timeouts, neutral Stop, and the three-strikes pause produce no send
A nil notifier disables the trigger with no other behavior change
The reap path is never delayed by gateway I/O (send is fire-and-forget at the call site)

Out of scope:

The needs-input trigger (#99) — separate seam at the chat tailer
Terse/content-free mode (#101)
Notifications for deaths, timeouts, or the three-strikes pause (possible future trigger categories)
Manual instances
Changes to the push package beyond (optionally) a narrow notifier interface — sender and subscription semantics are #98's landed contract
Forge-URL deep links (tap always routes inside the PWA)

> *This was generated by AI during triage.* ## Agent Brief **Category:** enhancement **Summary:** Fire a push notification when the reaper detects an AFK run's done-signal — a PR (forge binding) or change request (builtin binding) whose head branch equals the run's branch. The blocker (#98) has landed; this hooks the second trigger into the shipped sender. **Current behavior:** The reaper already detects the done-signal: it lists the repo's pulls through the Tracker contract, checks head-branch presence (open or merged counts, closed does not), classifies the run, and — exactly once per run, via an idempotent claim — executes the terminal reap. That reap site already carries one injected, nil-safe seam: the metrics hook that records how an AFK run ended (`AFKRunEnded`). Nothing there sends notifications. The landed push sender (`Broadcast(Payload{Title, Body, Tag, Route})`) is fire-and-forget and never blocks or errors at the call site. **Desired behavior:** When the reaper's terminal classification of an AFK run is **success** (done-signal present), one notification is broadcast: - **Title:** `<repo>~<label> opened PR #<n>` on a forge-bound repo, or the change-request equivalent on a builtin-bound repo. Repo and run fields (repo name, session name, branch, run ID) are all in scope at the reap site. - **Body:** the PR/CR title. **Known gap to design around:** the pull listing on the reaper's hot path carries number, head branch, state, and URL — but *not* the title; the title lives on the tracker's heavier per-pull detail fetch. Fetching the detail once on the success path is acceptable (it fires once per run); if that fetch fails, degrade the body to `PR #<n>` rather than blocking or skipping the notification. Also note the PR's number currently doesn't flow down to the terminal reap step — only a presence boolean does — so the matched pull ref must be threaded down or re-derived there. - **Tag:** the run ID (coalesces with any earlier needs-input item for the same run — the done notification replaces it on the lock screen). - **Route:** the run's chat route (`/runs/<id>`) — lab's chat is where review/merge context lives; never a forge URL, which would bounce the user out of the PWA. Semantics: - **Fires exactly once per run** — the done-signal reap is already claimed idempotently, so no additional debounce state is needed; lean on that. - **Success only:** deaths, timeouts, a neutral Stop, and the three-strikes pause produce no send in this slice. - **AFK runs only** — a manual instance has no done-signal. - **The reaper is never delayed:** the seam call is fire-and-forget; any synchronous tracker detail fetch for the title happens on the success path only and must tolerate failure without affecting the reap. **Key interfaces:** - Mirror the metrics seam exactly: the AFK service's options gain an optional notifier next to the existing nil-safe metrics hook, fired beside `AFKRunEnded` when the outcome is success. A nil notifier means no notifications, no other behavior change. - One Tracker contract covers both bindings: the same pull-listing/head-match logic serves forge PRs and builtin change requests, so there is no per-binding trigger logic — only payload wording differs. - `push.Sender` is a concrete struct (no interface); for hermetic tests either define a narrow local notifier interface to record payloads, or use the landed recipe (real sender + httptest fake gateway + `Flush()`), alongside the existing fake tracker. **Acceptance criteria:** - [ ] A reap classifying an AFK run as success produces exactly one broadcast with the title, PR/CR-title body, tag = run ID, and the run's chat route (hermetic: fake/recorded sender + the existing fake tracker) - [ ] Both tracker bindings produce equivalent payloads: forge PR and builtin change request - [ ] A failed title-detail fetch degrades the body to the PR/CR number form and still sends - [ ] Deaths, timeouts, neutral Stop, and the three-strikes pause produce no send - [ ] A nil notifier disables the trigger with no other behavior change - [ ] The reap path is never delayed by gateway I/O (send is fire-and-forget at the call site) **Out of scope:** - The needs-input trigger (#99) — separate seam at the chat tailer - Terse/content-free mode (#101) - Notifications for deaths, timeouts, or the three-strikes pause (possible future trigger categories) - Manual instances - Changes to the push package beyond (optionally) a narrow notifier interface — sender and subscription semantics are #98's landed contract - Forge-URL deep links (tap always routes inside the PWA)

dominik.polakovics added

enhancement

ready-for-agent

and removed

needs-triage

labels

2026-07-10 16:48:29 +02:00

dominik.polakovics referenced this issue from a commit

2026-07-10 18:16:49 +02:00

feat(notify): done-signal trigger — one push at the reaper's success reap (#100)

dominik.polakovics referenced this issue from a pull request that will close it,

2026-07-10 18:17:13 +02:00

feat(notify): done-signal trigger — notify when an AFK run opens its PR/change request (#100) #110

dominik.polakovics commented

2026-07-10 18:23:00 +02:00

Author

Owner

This was generated by AI while landing a PR.

Landing audit — PR #110 → PASS

Verification signal relied on: Forgejo Actions CI ci / native — success (4m7s, run #109). Not re-run.

Checks performed:

Conventions: Title feat(notify): done-signal trigger … is Conventional Commits ✓. AFK contract: body carries Closes #100 ✓.
Merge state: afk/100 contains main tip — no conflict.
Diff correctness (9 files, +453/−23):
- tracker.DonePull refactor is behaviour-preserving: rank < stateRank(PullMerged) gates out closed(1)/unrecognized(0), admits merged(2)/open(3) — byte-for-byte the old PRPresent open||merged semantics. PRPresent is now the boolean projection of DonePull from one shared bestPull loop, so "is there a done-signal" and "which pull" cannot diverge.
- Notify fires only on outcome == OutcomeSuccess && notify != nil. Classify returns OutcomeSuccess iff prPresent, and prPresent ⟹ DonePull ok, so donePull is always a real pull when the push fires — no zero-value payload risk.
- Import boundary (ADR-0038): afk does not import internal/push — notify.go imports only store + tracker; cmd/lab maps afk.Notification → push.Payload 1:1 (both are {Title, Body, Tag, Route}).
- Fire-and-forget: sender wired over pushSender.Broadcast (async), so the reaper never blocks on gateway I/O.

Verdict: PASS. Awaiting free-text merge confirmation.

> *This was generated by AI while landing a PR.* **Landing audit — PR #110 → `PASS`** **Verification signal relied on:** Forgejo Actions CI `ci / native` — **success** (4m7s, run #109). Not re-run. **Checks performed:** - **Conventions:** Title `feat(notify): done-signal trigger …` is Conventional Commits ✓. AFK contract: body carries `Closes #100` ✓. - **Merge state:** `afk/100` contains `main` tip — no conflict. - **Diff correctness (9 files, +453/−23):** - `tracker.DonePull` refactor is behaviour-preserving: `rank < stateRank(PullMerged)` gates out closed(1)/unrecognized(0), admits merged(2)/open(3) — byte-for-byte the old `PRPresent` `open||merged` semantics. `PRPresent` is now the boolean projection of `DonePull` from one shared `bestPull` loop, so "is there a done-signal" and "which pull" cannot diverge. - Notify fires only on `outcome == OutcomeSuccess && notify != nil`. `Classify` returns `OutcomeSuccess` **iff** `prPresent`, and `prPresent ⟹ DonePull ok`, so `donePull` is always a real pull when the push fires — no zero-value payload risk. - Import boundary (ADR-0038): `afk` does **not** import `internal/push` — `notify.go` imports only `store` + `tracker`; `cmd/lab` maps `afk.Notification` → `push.Payload` 1:1 (both are `{Title, Body, Tag, Route}`). - Fire-and-forget: sender wired over `pushSender.Broadcast` (async), so the reaper never blocks on gateway I/O. **Verdict: PASS.** Awaiting free-text merge confirmation.

dominik.polakovics closed this issue

2026-07-10 18:23:28 +02:00

dominik.polakovics referenced this issue from a commit

2026-07-10 18:23:30 +02:00

Merge pull request 'feat(notify): done-signal trigger — notify when an AFK run opens its PR/change request (#100)' (#110) from afk/100 into main

dominik.polakovics referenced this issue

2026-07-10 18:52:36 +02:00

feat(secrets): exposure detection — exposed flag, chat badge, mask-on-render #108