From 4934befb98301dbf1748bff84371c8cdf3a099e0 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Tue, 20 Oct 2026 21:24:30 -0700 Subject: [PATCH 1/7] Add the Slack focus board as the dashboard's Slack tab Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 30 ++ README.md | 42 ++ bin/assistant-llm.py | 1 + bin/render-assistant-page.py | 221 +++++++++- bin/slack-focus.py | 170 ++++++++ bin/todo-server.py | 88 ++++ src/assistant/slack_focus/__init__.py | 14 + src/assistant/slack_focus/board.py | 51 +++ src/assistant/slack_focus/client.py | 147 +++++++ src/assistant/slack_focus/collect.py | 151 +++++++ src/assistant/slack_focus/judge.py | 158 ++++++++ src/assistant/slack_focus/settings.py | 85 ++++ src/assistant/slack_focus/store.py | 119 ++++++ src/assistant/slack_focus/triage.py | 189 +++++++++ tests/drive_dashboard_overview.py | 187 ++++++++- tests/slack_focus_fixtures.py | 155 +++++++ tests/test_renderer_slack_tab.py | 156 +++++++ tests/test_slack_focus.py | 559 ++++++++++++++++++++++++++ tests/test_slack_focus_cli.py | 172 ++++++++ tests/test_todo_server.py | 85 ++++ 20 files changed, 2777 insertions(+), 3 deletions(-) create mode 100755 bin/slack-focus.py create mode 100644 src/assistant/slack_focus/__init__.py create mode 100644 src/assistant/slack_focus/board.py create mode 100644 src/assistant/slack_focus/client.py create mode 100644 src/assistant/slack_focus/collect.py create mode 100644 src/assistant/slack_focus/judge.py create mode 100644 src/assistant/slack_focus/settings.py create mode 100644 src/assistant/slack_focus/store.py create mode 100644 src/assistant/slack_focus/triage.py create mode 100644 tests/slack_focus_fixtures.py create mode 100644 tests/test_renderer_slack_tab.py create mode 100644 tests/test_slack_focus.py create mode 100644 tests/test_slack_focus_cli.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ed70fe2..635d889 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,36 @@ The version is carried in `pyproject.toml` and `src/assistant/__init__.py` ## [Unreleased] +### Added +- Add a **Slack** tab to the dashboard, ported from the standalone focus-board + tool. It lists the Slack threads where the next move is yours, in Do first, + Promise, This week, and Optional, plus Done by you, Just read, Set aside, and + Auto-resolved. + - **Refresh from Slack** POSTs `/slack-focus/refresh` to `todo-server.py`, + which starts `bin/slack-focus.py` in the background under zsh with + `~/.zprofile` sourced, so `$SLACK_USER_TOKEN` and `gh` resolve under + launchd. The page polls `/slack-focus/status` and shows each step. A second + click while a refresh runs starts nothing. + - **Done** and **Undo** POST `/slack-focus/done` and `/slack-focus/undone`. + A checked-off row comes back when its thread gets a new reply or edit. + - The pipeline lives in `src/assistant/slack_focus/`. Collect and triage are + rules only; the judge is one model call over the surfaced candidates, + tools off, metered as `caller="slack_focus"`, with verdicts cached by + content. If the model fails, the raw rows show. + - The Slack client refuses any method outside a read allowlist before a + request leaves the machine, and a test checks no module names a write + method. + - Everything about the user is in the new `slack_focus` block of + `~/.assistant/comms/config.json`: watched channels, bot names, skipped + channel names, GitHub repos and hosts, and Jira projects. The Slack ID, + name, and workspace come from the token. Jira project keys and GitHub + hosts used to be hard-coded. + - Files live in `~/.assistant/slack-focus/`. + - `bin/assistant-llm.py` accepts `--feature slack_focus`. +- A row's text now quotes the other person's latest message, not yours, and + the short-ack hint ignores your own replies. The focus-board compared + author names with the label "you", which never matched a real name. + ### Changed - Read agent sessions from cmux first, with mukul-memory's registry as a fallback. cmux 0.64.25 records agent sessions itself, and diff --git a/README.md b/README.md index 78d8786..2b5008b 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,8 @@ The whole thing optimizes one number downward, computed mechanically each day: * **🧠 Turn corrections into rules, then into memory.** `lesson-extractor.py` scans your recent Claude Code transcripts and the action ledger for corrections, confirmations, and recurring questions, and distills lesson candidates into a proposals queue for you to review. Confirm one and it's routed to the right store (one of `claude` / `assistant` / `ffp` / `archffp` / `assistant-repo`), then mirrored into the Obsidian vault and synced to the cross-machine memory repo that feeds Mem0 semantic memory. Nothing is added without your confirmation. +**πŸ“Œ See which Slack threads need you.** The dashboard's **Slack** tab lists the threads where the next move is yours: direct asks, @-mentions, DMs, promises you made, and unread posts in channels you watch. Rules sort every thread for free; one bounded model call writes short titles and sets noise aside. Threads whose linked pull request merged or ticket closed drop to **Auto-resolved**. Select **Done** to check a row off; it comes back if the thread gets a new reply. It's read-only: it never posts, reacts, or marks anything read. See [Slack focus list](#slack-focus-list). + **πŸ‘‹ Nudge stalled work and move the safe stuff forward.** Each pulse the Observer emits one verdict per workspace, and Python β€” not the model β€” turns it into an action. `ready_for_merge` queues `/merge-when-ready`; `ready_for_cleanup` sends `/cleanup` (only on workspaces *it* queued the merge for, and only once a work receipt exists); `stranded` nudges the idle agent with what failed and a retry; `needs_user` surfaces an awaiting card and does nothing else. Autonomy is fenced by a back-off list, the work-receipt gate, the assistant-merge ledger, and `NO_INGEST_GUARD`. ## Setup β€” connect your sources (opt-in) @@ -168,6 +170,43 @@ Default ingestion is **@-mentions + DMs only**. For full channel/group message i +### Slack focus list + +The Slack tab needs a Slack **user** token, because only user tokens can search messages, and a `slack_focus` block that says what to watch. Nothing about you is hard-coded: your Slack ID, name, and workspace come from the token. + +1. Create or reuse a Slack app with these **user** token scopes: `search:read`, `channels:history`, `groups:history`, `im:history`, `mpim:history`, `channels:read`, `groups:read`, `im:read`, `mpim:read`, `users:read`, and `usergroups:read`. Install it and copy the `xoxp-` token. +2. Add `export SLACK_USER_TOKEN=xoxp-…` to `~/.zprofile`. The Refresh button sources that file, so the token never lands in a config file or plist. +3. Add a `slack_focus` block to `~/.assistant/comms/config.json`: + +```json +{ + "slack_focus": { + "days": 7, + "title": "Slack focus", + "watched_channels": {"C0123456789": "my-team"}, + "bot_authors": ["Deploy Bot"], + "skip_channels": ["deployments"], + "github_repos": ["my-org/my-repo"], + "github_hosts": ["github.com"], + "jira": {"enabled": false, "base_url": "https://jira.example.com", "project_keys": ["ABC"], + "token_env": "JIRA_API_TOKEN", "email_env": "JIRA_EMAIL"} + } +} +``` + +| Setting | What it does | +|---|---| +| `days` | How far back to look, 1–30. Default 7. | +| `title` | The tab's heading. | +| `watched_channels` | Channel ID β†’ your label. Every thread there counts, not only the ones that mention you. Mentions and DMs are always checked. | +| `bot_authors` | Display names whose last word means the thread is a bot post. | +| `skip_channels` | Channel-name fragments to treat as bot noise, such as `deployments`. | +| `github_repos` | Repos where your merged PRs can retire an ask ("already done"). Uses `gh`. | +| `github_hosts` | Hosts whose pull request links count, for GitHub Enterprise. Default `github.com`. | +| `jira` | Optional. With `enabled` and `project_keys`, a thread whose linked tickets are all done is auto-resolved. The token comes from the env var named in `token_env`; with `email_env` set, it uses Jira Cloud basic auth, otherwise a bearer token. `base_url` falls back to `$JIRA_BASE_URL`. | + +Open the **Slack** tab and select **Refresh from Slack**. The first pull of 7 days takes a minute or two. To refresh from a terminal, run `python3 bin/slack-focus.py` (`--triage` reuses the last pull, `--no-judge` skips the model). Files live in `~/.assistant/slack-focus/`. To route the model step to Droid, run `bin/assistant-llm.py set droid --feature slack_focus`. + ## Config knobs you own | Knob | Default | Where | What it does | @@ -177,6 +216,7 @@ Default ingestion is **@-mentions + DMs only**. For full channel/group message i | Planner autoDispatch | **OFF** | `~/.assistant/comms/config.json` β†’ `{"planner":{"autoDispatch":true}}` | OFF (safe default) = a stalled goal *stages a decision* for your review. ON = whitelisted steps dispatch unattended overnight. Flip it on only after watching the planner behave. | | JIRA scope | your own issues | `JIRA_JQL` env var | Widens the JQL beyond `assignee/reporter/watcher = you`. | | Slack channel ingest | mentions + DMs only | `SLACK_INGEST_CHANNELS=1` env (+ manifest edit) | Opts into full-channel message ingestion. | +| Slack focus list | not set up | `~/.assistant/comms/config.json` β†’ `slack_focus` block + `SLACK_USER_TOKEN` | Which channels, bots, repos, and Jira projects the Slack tab uses. See [Slack focus list](#slack-focus-list). | | Connector cadence / caps | 60s Β· 200 events Β· 10 pages | `~/.assistant/comms/config.json` β†’ `connectors` block | Per-connector poll cadence and batch caps (module constants are only the fallback). | **Strategist (M6, the LLM drafter).** In `~/.assistant/comms/config.json` β†’ `{"strategist": {…}}`: `enabled` (default `true`), `dailyCostCeilingUsd` (the day's LLM budget β€” the Strategist is *shed first* when it's hit, never the Observer), plus per-goal `budget.maxStrategistCallsPerDay` (default `1`) in `assistant-goals.json`. It ships enabled, but on the safe `autoDispatch`-off default it only upgrades *staged-decision* text you review β€” never unattended work. @@ -187,6 +227,7 @@ Open the dashboard at **http://127.0.0.1:9876** (localhost only; served by `bin/ - **Sessions tab** β€” your default view counts open cmux workspaces, not GitHub alerts. It groups workspaces into **Needs your answer**, **Assistant tasks**, **Ready to close**, and **Paused**. Each card shows one next action. Click **Go to session** to focus its workspace, or expand the card for details. The finish prompt highlights older pending work without interrupting productive sessions or automatically closing anything. - **Notifications tab** β€” a separate inbox for GitHub pull requests and other alerts. Its counts don't add to your session workload. Expand a group's history for its existing controls; receipts, the FYI digest, and health remain available here. +- **Slack tab** β€” the threads where the next move is yours, in **Do first**, **Promise**, **This week**, and **Optional**. Select **Refresh from Slack** to pull again; the button shows each step. **Done** checks a row off until its thread gets a new reply. - **Connections tab** β€” every connector as **Connected** / **Available, not connected** / **Needs attention**, with the how-to-connect hint inline. - **`/goal` skill** β€” add / list / rerank / pause goals (`/goal add "…" --outcome "…"`). Goals feed the planner and boost decision ranking. Automation can only *propose* goal changes; only you (this skill) edit the store in place. - **One-tap accept** on a decision routes through the todo-server, executes the recommended action class (draft-only for any external send in this tree), and ledgers the transition. Reject / snooze do the same. @@ -276,6 +317,7 @@ expanded original questions, and unchanged controls. - `bin/build-morning-brief.py` β€” rebuild today's brief on demand; `bin/plan-next-actions.py` β€” run the goals planner on demand - `bin/interrupt-gate.py` β€” the sole push chokepoint (consulted for every would-be notification) - `bin/todo-server.py` β€” the localhost dashboard + JSON API at http://127.0.0.1:9876 +- `bin/slack-focus.py` β€” refresh the Slack tab's list on demand (the dashboard's Refresh button runs it) - `bin/assistant-curator.py` β€” lesson/rule management across all five stores - `bin/tool-dispatch.py` β€” named tool dispatcher (`bin/tools-manifest.json`) - `install.sh --apply` β€” wires everything up; symlinks skills; writes/copies plists (never loads them) diff --git a/bin/assistant-llm.py b/bin/assistant-llm.py index ec3cae5..1ef0d17 100755 --- a/bin/assistant-llm.py +++ b/bin/assistant-llm.py @@ -23,6 +23,7 @@ "strategist", "narrator", "lessons", + "slack_focus", ) diff --git a/bin/render-assistant-page.py b/bin/render-assistant-page.py index b0286c9..ec8ad9c 100755 --- a/bin/render-assistant-page.py +++ b/bin/render-assistant-page.py @@ -2457,6 +2457,106 @@ def _render_connections_panel_inner(world): return html, n_connected +def _slack_href(url): + try: + parts = urlsplit(url) if isinstance(url, str) else None + except ValueError: + return "#" + if parts and parts.scheme == "https" and parts.hostname and not any(c.isspace() for c in url): + return e(url) + return "#" + + +def _slack_focus_row(item, action=None): + tag = '@you' if item.get("mention") else "" + meta = [x for x in (item.get("why"), item.get("channel"), item.get("who"), + (item.get("last") or "")[5:].replace("T", " ")) if x] + if action == "undo" and item.get("done_at"): + meta.append("checked off " + str(item["done_at"])[5:16].replace("T", " ")) + effort = item.get("effort") + pill = f'{e(effort)}' if effort in ("Quick", "Small", "Medium") else "" + button = "" + if action: + done = action == "done" + button = (f'') + return (f'
{tag}{e(item.get("text") or "")}' + f'{e(" Β· ".join(meta))}{pill}{button}
') + + +def render_slack_focus_tab(): + """The Slack tab: the threads where the next move is yours, ported from the + focus board. Reads ~/.assistant/slack-focus/ through slack_focus.board. + Every Slack-derived string is escaped. Any failure degrades to one message. + Returns (html, n_do_first or None).""" + try: + return _render_slack_focus_tab_inner() + except Exception as exc: # noqa: BLE001 β€” the tab must never break the page + return (f'
The Slack list is unavailable: {e(str(exc)[:200])}.
'), None + + +def _render_slack_focus_tab_inner(): + from assistant.slack_focus import board # noqa: PLC0415 + + view = board.load(HOME) + state = view["state"] + running = bool(state.get("running")) + button = (f'') + focus = view["focus"] + if focus: + when = parse_iso(focus.get("generated")) + pulled = when.strftime("%b %d, %H:%M UTC") if when else "at an unknown time" + how = ("A model wrote the titles and set aside noise." if view["judged"] + else "These are the raw rows; the model step didn't run.") + status = f'Last pulled {e(pulled)} from the last {e(str(focus.get("window_days", "?")))} days. {how}' + else: + status = "Not checked yet. Select Refresh from Slack to build your list." + if running: + status = f'Refreshing: {e(state.get("phase") or "Starting")}.' + error = "" + if state.get("ok") is False and not running: + error = (f'

The last refresh didn\'t finish: ' + f'{e(str(state.get("error") or "unknown error"))}

') + count = f' {view["do_first"]} to do first' if focus else "" + head = (f'

{e(view["title"])}{count}

' + f'
{button}{status}
' + f'
{error}') + if not view["configured"]: + body = ('
Set up the Slack list: add a slack_focus block to ' + '~/.assistant/comms/config.json and export SLACK_USER_TOKEN in ' + '~/.zprofile. The README lists every setting.
') + elif not focus: + body = '
Nothing pulled from Slack yet.
' + else: + lanes = [] + for name, cls, hint, items in view["lanes"]: + rows = "".join(_slack_focus_row(i, "done") for i in items) \ + or '

Nothing here.

' + lanes.append(f'

{e(name)} {len(items)}

' + f'

{e(hint)}

{rows}
') + extras = ( + ("sf-done", "Done by you", "Comes back if its thread gets a new reply.", + [_slack_focus_row(i, "undo") for i in view["done_by_you"]]), + ("sf-read", "Just read", "Worth knowing. No reply needed.", + [_slack_focus_row(i) for i in view["read"]]), + ("sf-aside", "Set aside", "The model judged these not actionable. Scan them to be sure.", + [_slack_focus_row(i) for i in view["set_aside"]]), + ("sf-resolved", "Auto-resolved", "A linked pull request or ticket closed.", + [_slack_focus_row(i) for i in view["handled"]]), + ) + details = "".join( + f'
{e(label)} ' + f'{len(rows)}

{e(hint)}

' + f'
{"".join(rows) or "

Nothing here.

"}
' + for key, label, hint, rows in extras) + body = "".join(lanes) + details + html = (f'
' + f'{head}{body}
') + return html, (view["do_first"] if focus else None) + + def render(): if not WORLD_PATH.exists(): DASHBOARD_HTML.write_text("

world.json not present yet β€” Scanner hasn't run.

") @@ -2471,6 +2571,8 @@ def render(): fleet_html, fleet_n = render_fleet_tab() brief_html, brief_n = render_brief_tab() connections_html, connected_n = render_connections_panel(world) + slack_html, slack_n = render_slack_focus_tab() + slack_count = f' {slack_n}' if slack_n is not None else "" pulse_health_html = render_pulse_health() pulse_alert_html = render_pulse_alert() counts = world.get("counts", {}) @@ -3602,6 +3704,37 @@ def render(): button:focus-visible, summary:focus-visible, input:focus-visible, a:focus-visible { outline: 2px solid var(--blue); outline-offset: 3px; } @media (max-width: 1100px) { .attention-board { grid-template-columns: repeat(2,minmax(0,1fr)); } } @media (max-width: 620px) { .attention-board { grid-template-columns: 1fr; } .attention-intro { flex-direction: column; gap: 12px; } .attention-filter { align-self: stretch; } .attention-filter input { width: 100%; } .attention-hint { min-height: auto; } } + +/* ─── Slack tab ─── */ +.sf-head { display: flex; flex-wrap: wrap; align-items: baseline; justify-content: space-between; gap: 10px; margin-bottom: 18px; } +.sf-head h2 { margin: 0; border: 0; padding: 0; } +.sf-controls { display: flex; flex-wrap: wrap; align-items: center; gap: 10px; } +.sf-controls .meta { margin: 0; } +.sf-error { color: var(--amber); margin: 0 0 16px; } +.sf-lane { margin-bottom: 24px; } +.sf-lane h3 { font: 600 13px/1.4 var(--sans); margin: 0 0 2px; color: var(--text); } +.sf-lane h3 .count, .sf-more summary .count { color: var(--muted); font: 500 11px/1 var(--mono); margin-left: 6px; } +.sf-lane .meta, .sf-more .meta { margin: 0 0 8px; } +.sf-list { background: var(--panel); border: 1px solid var(--line); border-radius: 10px; overflow: hidden; } +.sf-do .sf-list { border-left: 3px solid var(--green); } +.sf-promise .sf-list { border-left: 3px solid var(--pink); } +.sf-week .sf-list { border-left: 3px solid var(--blue); } +.sf-optional .sf-list { border-left: 3px solid var(--purple); } +.sf-row { display: flex; align-items: center; gap: 10px; padding: 10px 14px; border-top: 1px solid var(--line); } +.sf-row:first-child { border-top: 0; } +.sf-row:hover { background: var(--hover); } +.sf-link { flex: 1; min-width: 0; color: inherit; text-decoration: none; display: flex; flex-direction: column; gap: 2px; } +.sf-title { color: var(--text); overflow-wrap: anywhere; } +.sf-meta { color: var(--muted); font-size: 12px; overflow-wrap: anywhere; } +.sf-tag { font: 600 10px/1 var(--mono); color: var(--green); background: var(--green-bg); border-radius: 4px; padding: 2px 5px; margin-right: 6px; } +.sf-effort { flex: none; font: 500 10px/1 var(--mono); border-radius: 4px; padding: 3px 6px; color: var(--blue); background: var(--blue-bg); } +.sf-medium { color: var(--amber); background: var(--amber-bg); } +.sf-mark { flex: none; } +.sf-none { color: var(--muted); margin: 0; padding: 10px 14px; } +.sf-more { margin-bottom: 14px; } +.sf-more summary { cursor: pointer; color: var(--text-2); font: 600 13px/1.4 var(--sans); padding: 4px 0; } +.sf-more[data-context-key="sf-done"] .sf-title { text-decoration: line-through; opacity: 0.7; } +@media (max-width: 620px) { .sf-row { flex-wrap: wrap; } .sf-link { flex-basis: 100%; } } """ js = r""" @@ -3846,6 +3979,7 @@ def render(): } document.getElementById('refresh-error').textContent = ''; updateFreshness(); + if (document.querySelector('#slack-focus[data-running="true"]')) slackFocusPoll(); } catch (error) { document.getElementById('refresh-error').textContent = 'Refresh failed: ' + error.message; } finally { @@ -3873,10 +4007,88 @@ def render(): } window.addEventListener('DOMContentLoaded', () => { const initial = (window.location.hash || '#overview').replace('#', ''); - showTab(['overview', 'brief', 'decisions', 'workspaces', 'fleet', 'connections', 'todos'].includes(initial) ? initial : 'overview'); + showTab(['overview', 'brief', 'decisions', 'workspaces', 'fleet', 'connections', 'slack', 'todos'].includes(initial) ? initial : 'overview'); updateFreshness(); setInterval(refreshDashboard, 15000); + if (document.querySelector('#slack-focus[data-running="true"]')) slackFocusPoll(); }); +// Slack tab: the Refresh button starts a background pull through the +// todo-server, then polls its state every 2 seconds and reloads the page +// content when the pull ends. Done and Undo save a check-off the same way. +let slackFocusPolling = false; +function slackFocusPost(path, body) { + return fetch(path, body === undefined ? {method: 'POST'} + : {method: 'POST', headers: {'Content-Type': 'application/json'}, body: JSON.stringify(body)}); +} +function slackFocusShow(state, message) { + const button = document.getElementById('sf-refresh'); + const status = document.getElementById('sf-status'); + if (!button || !status) return; + button.disabled = Boolean(state.running); + button.textContent = state.running ? 'Refreshing…' : 'Refresh from Slack'; + if (message) status.textContent = message; + else if (state.running) status.textContent = 'Refreshing: ' + (state.phase || 'Starting') + '.'; +} +async function slackFocusPoll() { + if (slackFocusPolling) return; + slackFocusPolling = true; + try { + let state; + for (;;) { + const response = await slackFocusPost('/slack-focus/status'); + if (!response.ok) throw new Error('the server returned ' + response.status); + state = await response.json(); + if (!state.running) break; + slackFocusShow(state); + await new Promise(resolve => setTimeout(resolve, 2000)); + } + slackFocusShow(state, 'Loading the new list…'); + // The run records that it finished, then re-renders the page. Reload + // briefly until the page is at least as new as the finish. + const finished = Date.parse(state.finished || '') / 1000; + for (let tries = 0; tries < 15; tries++) { + await refreshDashboard(true); + const rendered = Number(document.getElementById('dashboard-content').dataset.renderedAt); + if (!(rendered < finished)) break; + await new Promise(resolve => setTimeout(resolve, 1000)); + } + const status = document.getElementById('sf-status'); + if (status.textContent === 'Loading the new list…') { + status.textContent = state.ok === false + ? "The refresh didn't finish: " + (state.error || 'unknown error') + : 'Refresh finished. The list updates within a minute.'; + } + } catch (error) { + slackFocusShow({running: false}, "Couldn't check the refresh: " + error.message); + } finally { + slackFocusPolling = false; + } +} +async function slackFocusRefresh() { + slackFocusShow({running: true, phase: 'Starting'}); + try { + const response = await slackFocusPost('/slack-focus/refresh'); + if (!response.ok) throw new Error('the server returned ' + response.status); + } catch (error) { + slackFocusShow({running: false}, "Couldn't start the refresh: " + error.message); + return; + } + await slackFocusPoll(); +} +async function slackFocusMark(button) { + button.disabled = true; + try { + const path = button.dataset.done === 'true' ? '/slack-focus/done' : '/slack-focus/undone'; + const response = await slackFocusPost(path, {fp: button.dataset.fp}); + if (!response.ok) throw new Error(await response.text()); + button.textContent = 'Saved'; + await refreshDashboard(true); + } catch (error) { + button.disabled = false; + button.textContent = "Couldn't save. Try again"; + } +} + async function openWs(btn) { const requests = btn.closest('#assistant-requests'); if (requests) { @@ -4123,6 +4335,9 @@ def render(): + @@ -4152,6 +4367,10 @@ def render(): {connections_html} +
+{slack_html} +
+
{todos_html}
diff --git a/bin/slack-focus.py b/bin/slack-focus.py new file mode 100755 index 0000000..99cc302 --- /dev/null +++ b/bin/slack-focus.py @@ -0,0 +1,170 @@ +#!/usr/bin/env python3 +"""slack-focus.py β€” refresh the Slack focus board behind the dashboard's Slack tab. + + python3 bin/slack-focus.py pull Slack, triage, judge, re-render + python3 bin/slack-focus.py --triage re-triage and judge the last pull (no Slack calls) + python3 bin/slack-focus.py --no-judge skip the model; show the raw mechanical rows + +Read-only toward Slack, GitHub, and Jira. The pure logic lives in +src/assistant/slack_focus/; this file is the one place that spawns the model +(the narrator's subprocess pattern: tools off, metered as caller="slack_focus"). +The dashboard's Refresh button starts this through bin/todo-server.py, which +sources ~/.zprofile first so $SLACK_USER_TOKEN and PATH resolve under launchd. +Each step is recorded in ~/.assistant/slack-focus/state.json for the button. +""" +from __future__ import annotations + +import argparse +import json +import os +import signal +import subprocess +import sys +import time +from datetime import datetime +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +BIN = REPO / "bin" +SRC = REPO / "src" +for path in (SRC, BIN): + if str(path) not in sys.path: + sys.path.insert(0, str(path)) + +import comms_lib # noqa: E402 +import llm_runner # noqa: E402 +import metering # noqa: E402 +from assistant import model_tiers # noqa: E402 +from assistant.slack_focus import client as slack_client # noqa: E402 +from assistant.slack_focus import collect, judge, settings, store, triage # noqa: E402 + +RENDER_SCRIPT = BIN / "render-assistant-page.py" +JUDGE_TIMEOUT_SEC = int(os.environ.get("SLACK_FOCUS_JUDGE_TIMEOUT_SEC", "420")) +CLAUDE_BIN = os.environ.get("CLAUDE_BIN", str(Path.home() / ".local/bin/claude")) + + +def _home() -> Path: + return Path(os.environ.get("HOME", str(Path.home()))) + + +def run(cmd: list[str], *, input_text: str | None = None, timeout: int = 30, + merge_bedrock: bool = False) -> tuple[int, str, str]: + """Run a subprocess in its own session; a timeout kills the whole group.""" + env = dict(os.environ) + if merge_bedrock: + env.update(comms_lib.load_bedrock_env(_home())) + try: + proc = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + text=True, env=env, start_new_session=True) + except OSError as e: + return 127, "", f"spawn failed: {e}" + try: + out, err = proc.communicate(input=input_text, timeout=timeout) + return proc.returncode, out or "", err or "" + except subprocess.TimeoutExpired: + try: + os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + except (OSError, ProcessLookupError): + proc.kill() + try: + out, err = proc.communicate(timeout=10) + except subprocess.TimeoutExpired: + out, err = "", "" + return 124, out or "", (err or "") + f"\ntimeout after {timeout}s" + + +def call_model(prompt: str) -> str: + """The judge's one model call. Tools are off, so Slack text can't steer it + into acting. Returns "" on any failure, which keeps the raw rows.""" + route = llm_runner.load_route_config(llm_runner.config_path(home=_home()), "slack_focus") + provider = llm_runner.select_provider(route, []) + model = route.droid_model if provider == "droid" else model_tiers.model_for("balanced") + run_dir = store.base_dir(_home()) / "judge-run" + run_dir.mkdir(parents=True, exist_ok=True) + result = llm_runner.invoke( + provider=provider, prompt=prompt, model=model, run_dir=run_dir, timeout=JUDGE_TIMEOUT_SEC, run=run, + claude_bin=CLAUDE_BIN, droid_bin=route.droid_bin, reasoning_effort=route.droid_reasoning_effort, + disable_tools=True, tag="assistant-slack-focus") + try: + usage = metering.observer_usage(result.stdout, len(prompt), model) + metering.append_cost_row(caller="slack_focus", model=model, usage=usage, wall_ms=result.wall_ms, + status="ok" if result.usable else "failed", provider=provider) + except Exception as e: # noqa: BLE001 β€” metering never blocks the board + print(f"metering failed (ignored): {e}", file=sys.stderr) + if not result.usable: + print(f"judge model rc={result.rc}: {result.stderr.strip()[-300:]}", file=sys.stderr) + return result.result_text if result.usable else "" + + +def rerender() -> None: + subprocess.run([sys.executable, str(RENDER_SCRIPT)], timeout=60, check=False, + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + + +def phase(home: Path, text: str) -> None: + print(f"PHASE: {text}", flush=True) + store.write_state(home, phase=text) + + +def refresh(home: Path, *, pull: bool, use_judge: bool, token: str, + transport=slack_client.http_transport, model=call_model, + resolver: triage.Resolver | None = None) -> dict: + """Run the pipeline and return the board it wrote.""" + root = store.base_dir(home) + cfg = settings.load(home) + if not cfg.configured: + raise RuntimeError("Add a slack_focus block to ~/.assistant/comms/config.json first.") + if pull: + phase(home, "Pulling Slack") + client = slack_client.Client(token, transport) + threads_doc, mine = collect.collect( + client, cfg, progress=lambda i, n: store.write_state(home, phase=f"Pulling Slack ({i} of {n} threads)")) + store.write_json(root / "threads.json", threads_doc) + store.write_json(root / "mine.json", mine) + threads_doc = store.read_json(root / "threads.json") + mine = store.read_json(root / "mine.json", []) + if not isinstance(threads_doc, dict): + raise RuntimeError("There's no Slack pull to triage yet. Run a full refresh first.") + resolver = resolver or triage.Resolver(cfg) + phase(home, "Sorting threads") + focus = triage.triage(threads_doc, mine, cfg, resolver) + if use_judge: + phase(home, "Asking the model which threads need you") + threads = {t["id"]: t for t in threads_doc["threads"]} + evidence = judge.done_evidence(cfg, resolver, focus["window_days"], datetime.now()) + focus, cache = judge.judge(focus, threads, evidence, store.read_json(root / "judge-cache.json"), model) + store.write_json(root / "judge-cache.json", cache) + store.write_json(root / "focus.json", focus) + return focus + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description="Refresh the Slack focus board.") + ap.add_argument("--triage", action="store_true", help="re-triage the last pull without calling Slack") + ap.add_argument("--no-judge", action="store_true", help="skip the model step") + args = ap.parse_args(argv) + home = _home() + root = store.base_dir(home) + with store.locked(root / "refresh.lock", blocking=False) as got: + if not got: + print("A refresh is already running.", file=sys.stderr) + return 0 + store.write_state(home, running=True, pid=os.getpid(), started=store.now_iso(), finished=None, + ok=None, error=None, phase="Starting") + started = time.time() + error = None + try: + focus = refresh(home, pull=not args.triage, use_judge=not args.no_judge, + token=os.environ.get("SLACK_USER_TOKEN", "")) + print(json.dumps(focus["counts"])) + except Exception as e: # noqa: BLE001 β€” every run must record how it ended + error = str(e)[:300] or type(e).__name__ + print(f"refresh failed: {error}", file=sys.stderr) + store.write_state(home, running=False, finished=store.now_iso(), ok=error is None, error=error, + phase=None, seconds=round(time.time() - started)) + rerender() + return 0 if error is None else 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/bin/todo-server.py b/bin/todo-server.py index e790a99..fe3e049 100755 --- a/bin/todo-server.py +++ b/bin/todo-server.py @@ -46,6 +46,14 @@ unseen-degradation pass knows the brief was looked at + POST /slack-focus/status Slack tab refresh state + (JSON) + POST /slack-focus/refresh start a background + Slack focus refresh + (no-op while one runs) + POST /slack-focus/done | /slack-focus/undone check an item off or + back on (body = + {"fp": ""}) GET / health check ("ok") On every successful TODO mutation, the JSON file is atomically rewritten and the @@ -107,6 +115,10 @@ RENDER_SCRIPT = Path(__file__).resolve().parent / "render-assistant-page.py" RENDER_DASHBOARD_SCRIPT = RENDER_SCRIPT PROPOSALS_DIR = HOME / ".architect" / "orchestrator-proposals" +SLACK_FOCUS_SCRIPT = Path(__file__).resolve().parent / "slack-focus.py" +SLACK_FOCUS_LAUNCH = '[ -f "$HOME/.zprofile" ] && . "$HOME/.zprofile" >/dev/null 2>&1; exec "$0" "$@"' +SLACK_FOCUS_FP_RE = re.compile(r"^[a-f0-9]{64}$") +_slack_focus_proc = None ALLOWED_FLAGS = {"autoDispatch", "closeOnMerge"} PORT = 9876 HOST = "127.0.0.1" @@ -517,6 +529,66 @@ def brief_seen(params): return False, f"brief store unavailable: {e}" +def _slack_focus_store(): + if str(SRC_DIR) not in sys.path: + sys.path.insert(0, str(SRC_DIR)) + from assistant.slack_focus import store # noqa: PLC0415 + return store + + +def _reap_slack_focus(): + """Collect a finished refresh. If it exited before recording its own + state, the state still names this server, so record the failure here.""" + global _slack_focus_proc + if _slack_focus_proc is None or _slack_focus_proc.poll() is None: + return + _slack_focus_proc = None + store = _slack_focus_store() + state = store.read_json(store.base_dir(HOME) / "state.json", {}) + if isinstance(state, dict) and state.get("running") and state.get("pid") == os.getpid(): + store.write_state(HOME, running=False, ok=False, phase=None, finished=store.now_iso(), + error="The refresh didn't start. See ~/.assistant/slack-focus/refresh.log.") + + +def slack_focus_status(): + """POST /slack-focus/status: the refresh state the Slack tab's button polls.""" + _reap_slack_focus() + return json.dumps(_slack_focus_store().read_state(HOME)) + + +def slack_focus_refresh(): + """POST /slack-focus/refresh: start a refresh in the background, unless one + is already running. It runs under zsh with ~/.zprofile sourced, because + launchd doesn't give this server $SLACK_USER_TOKEN or the gh PATH.""" + global _slack_focus_proc + _reap_slack_focus() + store = _slack_focus_store() + root = store.base_dir(HOME) + with store.locked(root / "refresh.lock", blocking=False) as free: + if not free or _slack_focus_proc is not None: + return json.dumps({"started": False}) + store.write_state(HOME, running=True, pid=os.getpid(), started=store.now_iso(), + finished=None, ok=None, error=None, phase="Starting") + with open(root / "refresh.log", "w") as log: + _slack_focus_proc = subprocess.Popen( + ["/bin/zsh", "-c", SLACK_FOCUS_LAUNCH, sys.executable, str(SLACK_FOCUS_SCRIPT)], + stdin=subprocess.DEVNULL, stdout=log, stderr=subprocess.STDOUT, start_new_session=True) + return json.dumps({"started": True}) + + +def slack_focus_mark(body, done): + """POST /slack-focus/done | /slack-focus/undone (body = {"fp": ""}).""" + data, err = _parse_json_body(body) + if err: + return False, err + fp = data.get("fp") + if not isinstance(fp, str) or not SLACK_FOCUS_FP_RE.match(fp): + return False, "fp must be a 64-character hex fingerprint" + _slack_focus_store().set_done(HOME, fp, done) + rerender_dashboard() + return True, "marked done" if done else "marked not done" + + class WorkspaceIdentityConflict(ValueError): """The requested workspace identity cannot be verified.""" @@ -682,6 +754,7 @@ def do_POST(self): or (len(parts) == 3 and parts[0] == "goal" and parts[1] == "update") or (len(parts) == 3 and parts[0] == "decision" and parts[1] == "act") or parts == ["brief", "seen"] + or parts in (["slack-focus", "refresh"], ["slack-focus", "done"], ["slack-focus", "undone"]) ) if _mutation and not self._origin_allowed_for_mutation(): self._reply(403, "cross-origin mutation refused") @@ -779,6 +852,21 @@ def do_POST(self): self._reply(200 if ok else (404 if "no brief" in msg else 400), msg) return + # POST /slack-focus/{status,refresh,done,undone} β€” the Slack tab. + if parts in (["slack-focus", "status"], ["slack-focus", "refresh"]): + body = (slack_focus_status() if parts[1] == "status" else slack_focus_refresh()).encode() + self.send_response(200) + self._cors() + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + return + if parts in (["slack-focus", "done"], ["slack-focus", "undone"]): + ok, msg = slack_focus_mark(self._read_body(), parts[1] == "done") + self._reply(200 if ok else 400, msg) + return + # POST /focus/ if len(parts) == 2 and parts[0] == "focus": identity = urllib.parse.parse_qs( diff --git a/src/assistant/slack_focus/__init__.py b/src/assistant/slack_focus/__init__.py new file mode 100644 index 0000000..fbc7af1 --- /dev/null +++ b/src/assistant/slack_focus/__init__.py @@ -0,0 +1,14 @@ +"""slack_focus β€” the Slack focus board: which Slack threads need your action. + +Ported from the standalone focus-board tool. The pipeline is read-only and +mostly mechanical: + + collect β†’ pull your mentions, DMs, and watched channels; bucket every thread + triage β†’ map buckets to lanes, resolve threads whose linked PRs or tickets + closed, pick out your own promises + judge β†’ one bounded model call over the surfaced candidates only + board β†’ the view the dashboard's Slack tab renders + +Your settings live in the `slack_focus` block of ~/.assistant/comms/config.json. +The Slack user token comes from $SLACK_USER_TOKEN, never from config. +""" diff --git a/src/assistant/slack_focus/board.py b/src/assistant/slack_focus/board.py new file mode 100644 index 0000000..8ab8630 --- /dev/null +++ b/src/assistant/slack_focus/board.py @@ -0,0 +1,51 @@ +"""Board: what the dashboard's Slack tab shows, read from the stored files.""" +from __future__ import annotations + +from pathlib import Path + +from . import settings as settings_mod +from . import store + +LANES = ( + ("Do first", "do", "Someone is waiting on your reply or decision."), + ("Promise", "promise", "You said you would. Deliver it or confirm it's done."), + ("This week", "week", "Needs you, but not first thing."), + ("Optional", "optional", "Unread in a channel you watch. Skim it if you have time."), +) + + +def load(home: Path | None = None) -> dict: + root = store.base_dir(home) + cfg = settings_mod.load(home) + focus = store.read_json(root / "focus.json") + focus = focus if isinstance(focus, dict) and isinstance(focus.get("active"), list) else None + view = { + "configured": cfg.configured, + "title": cfg.title, + "state": store.read_state(home), + "focus": focus, + } + if focus is None: + return view + threads_doc = store.read_json(root / "threads.json", {}) + threads = {t["id"]: t for t in (threads_doc.get("threads") or []) if isinstance(t, dict) and "id" in t} \ + if isinstance(threads_doc, dict) else {} + marks = store.read_json(root / "done.json", {}) + marks = marks if isinstance(marks, dict) else {} + open_items, done_items = [], [] + for item in focus["active"]: + item = dict(item, fp=store.fingerprint(item, threads)) + if item["fp"] in marks: + done_items.append(dict(item, done_at=marks[item["fp"]])) + else: + open_items.append(item) + view.update( + lanes=[(name, cls, hint, [i for i in open_items if i.get("lane") == name]) for name, cls, hint in LANES], + done_by_you=done_items, + read=focus.get("read") or [], + set_aside=focus.get("judge_dropped") or [], + handled=focus.get("handled") or [], + judged=bool((focus.get("judge") or {}).get("ran")), + ) + view["do_first"] = len(view["lanes"][0][3]) + return view diff --git a/src/assistant/slack_focus/client.py b/src/assistant/slack_focus/client.py new file mode 100644 index 0000000..efa5e12 --- /dev/null +++ b/src/assistant/slack_focus/client.py @@ -0,0 +1,147 @@ +"""Read-only Slack Web API client for the focus board. + +It never posts, reacts, or marks anything read: `call` refuses any method +outside READ_METHODS before a request leaves the machine. The token is your +xoxp- user token from $SLACK_USER_TOKEN, because search needs a user token. +""" +from __future__ import annotations + +import http.client +import json +import re +import time +import urllib.error +import urllib.parse +import urllib.request +from datetime import datetime, timezone +from typing import Callable + +READ_METHODS = frozenset({ + "auth.test", + "conversations.history", + "conversations.info", + "conversations.replies", + "search.messages", + "usergroups.users.list", + "users.info", +}) +# A transport is (method, params, token) -> parsed JSON. Injectable for tests. +Transport = Callable[[str, dict, str], dict] +# Errors that mean "this one thread or channel is gone or private", not that the +# pull is broken. Any other error stops the refresh, so the board is never +# silently built from a partial pull. +SKIPPABLE = frozenset({"thread_not_found", "channel_not_found", "not_in_channel", "user_not_found", + "user_not_visible", "no_such_subteam"}) + + +class SlackError(RuntimeError): + pass + + +def http_transport(method: str, params: dict, token: str) -> dict: + """POST one form-encoded Web API call, retrying rate limits and flaky reads.""" + for _ in range(20): + req = urllib.request.Request( + f"https://slack.com/api/{method}", + data=urllib.parse.urlencode(params).encode(), + headers={"Authorization": f"Bearer {token}"}, + ) + try: + with urllib.request.urlopen(req, timeout=60) as resp: + return json.loads(resp.read()) + except urllib.error.HTTPError as err: + if err.code != 429: + raise + time.sleep(int(err.headers.get("Retry-After", "5")) + 1) + except (http.client.IncompleteRead, urllib.error.URLError, ConnectionError, TimeoutError): + time.sleep(3) + raise SlackError(f"{method}: Slack didn't answer after 20 tries") + + +def iso(ts) -> str: + return datetime.fromtimestamp(float(ts), timezone.utc).isoformat() + + +class Client: + def __init__(self, token: str, transport: Transport = http_transport, sleep=time.sleep): + if not token: + raise SlackError("Set SLACK_USER_TOKEN to a Slack user token (xoxp-).") + self._token = token + self._transport = transport + self._sleep = sleep + self._users: dict[str, str] = {} + self._channels: dict[str, dict] = {} + self._groups: dict[str, bool] = {} + auth = self._transport("auth.test", {}, token) + if not auth.get("ok"): + raise SlackError(f"Slack refused the token: {auth.get('error', 'unknown error')}") + self.me = auth["user_id"] + self.workspace_url = str(auth.get("url") or "").rstrip("/") + self.me_name = self.user_name(self.me) + + def call(self, method: str, params: dict) -> dict: + """One read call. A `ratelimited` answer is retried; a skippable error + returns the answer as is; any other error raises.""" + if method not in READ_METHODS: + raise SlackError(f"{method} isn't a read method; the focus board only reads Slack") + for _ in range(4): + answer = self._transport(method, params, self._token) + error = answer.get("error") + if answer.get("ok", True) or error in SKIPPABLE: + return answer + if error != "ratelimited": + raise SlackError(f"Slack {method} failed: {error or 'unknown error'}") + self._sleep(30) + raise SlackError(f"Slack {method} stayed rate limited; try again in a few minutes") + + def user_name(self, uid: str) -> str: + if not uid: + return "" + if uid not in self._users: + u = self.call("users.info", {"user": uid}).get("user", {}) + self._users[uid] = u.get("real_name") or u.get("name") or uid + return self._users[uid] + + def channel(self, cid: str) -> dict: + if cid not in self._channels: + ch = self.call("conversations.info", {"channel": cid}).get("channel", {}) + if ch.get("is_im"): + name, kind = f"DM {self.user_name(ch.get('user'))}", "dm" + elif ch.get("is_mpim"): + name, kind = "group DM", "mpim" + else: + name, kind = "#" + ch.get("name", cid), "channel" + self._channels[cid] = {"name": name, "kind": kind, "last_read": float(ch.get("last_read") or 0)} + return self._channels[cid] + + def in_group(self, gid: str) -> bool: + if gid not in self._groups: + users = self.call("usergroups.users.list", {"usergroup": gid}).get("users", []) + self._groups[gid] = self.me in users + return self._groups[gid] + + def mentions_me(self, text: str) -> str | None: + if f"<@{self.me}>" in (text or ""): + return "direct" + if any(self.in_group(g) for g in re.findall(r" str: + text = re.sub(r"<@([UW][A-Z0-9]+)>", lambda m: "@" + self.user_name(m.group(1)), text or "") + text = re.sub(r"]+)?>", "@group", text) + text = re.sub(r"<(https?://[^|>]+)\|([^>]+)>", r"\2", text) + text = re.sub(r":[a-z0-9_+'-]+:", "", text) + return re.sub(r"[ \t]{2,}", " ", text).strip() + + def search(self, query: str, max_pages: int = 80): + for page in range(1, max_pages + 1): + r = self.call("search.messages", {"query": query, "count": 100, "page": page, "sort": "timestamp"}) + messages = r.get("messages", {}) + yield from messages.get("matches", []) + if page >= messages.get("paging", {}).get("pages", 1): + return + + def permalink(self, cid: str, ts: str, thread_ts: str) -> str: + link = f"{self.workspace_url}/archives/{cid}/p{ts.replace('.', '')}" + return link + (f"?thread_ts={thread_ts}&cid={cid}" if thread_ts != ts else "") diff --git a/src/assistant/slack_focus/collect.py b/src/assistant/slack_focus/collect.py new file mode 100644 index 0000000..efab78a --- /dev/null +++ b/src/assistant/slack_focus/collect.py @@ -0,0 +1,151 @@ +"""Collect: pull the threads that may need you, and every message you sent. + +Purely mechanical. Every thread lands in one bucket by rules, no model. +""" +from __future__ import annotations + +import sys +import time +import urllib.parse +from datetime import datetime, timedelta, timezone +from typing import Callable + +from .client import Client, iso +from .settings import Settings + +SKIP_SUBTYPES = ("channel_join", "channel_leave") +Progress = Callable[[int, int], None] + + +def bucket(t: dict, settings: Settings) -> str: + if t["last_author"] in settings.bot_authors or any(s in t["channel"] for s in settings.skip_channels): + return "bot" + if t["last_is_me"]: + return "waiting_on_others" + if t["i_posted_after_in_channel"] or t["i_reacted_to_last"]: + return "replied_elsewhere" + if t["kind"] in ("dm", "mpim"): + return "ball_mine_dm" + if t["mentioned_after_my_last_reply"] == "direct": + return "ball_mine_mention" + if t["i_participated"] and t["from_others_after_my_last"] > 0: + return "thread_moved_on" + if t["mentioned_after_my_last_reply"] == "group": + return "group_mention" + if t["unread"]: + return "unread_watched" if t["watched"] else "unread_other" + return "read_fyi" + + +def _shape(client: Client, m: dict) -> dict: + author = client.user_name(m["user"]) if m.get("user") else m.get("username", "bot") + return {"author": author, "at": iso(m["ts"])[:16], "text": client.clean(m.get("text", ""))[:400]} + + +def _summarize(client: Client, settings: Settings, cid: str, msgs: list[dict], thread_ts: str, + parent_last_read: float | None = None) -> dict: + ch = client.channel(cid) + mine = [float(m["ts"]) for m in msgs if m.get("user") == client.me] + my_last = max(mine) if mine else 0.0 + last = msgs[-1] + after_me = [m for m in msgs if float(m["ts"]) > my_last and m.get("user") != client.me] + mention_after = next((client.mentions_me(m.get("text", "")) for m in after_me + if client.mentions_me(m.get("text", ""))), None) + last_read = parent_last_read if parent_last_read is not None else ch["last_read"] + return { + "cid": cid, "root": msgs[0]["ts"], "channel": ch["name"], "kind": ch["kind"], + "watched": cid in settings.watched_channels, + "permalink": client.permalink(cid, last["ts"], thread_ts), + "started": iso(msgs[0]["ts"]), "last_activity": iso(last["ts"]), + "last_author": _shape(client, last)["author"], "last_is_me": last.get("user") == client.me, + "i_participated": bool(mine), "messages": len(msgs), "from_others_after_my_last": len(after_me), + "mentioned_after_my_last_reply": mention_after, + "i_reacted_to_last": any(client.me in r.get("users", []) for r in last.get("reactions", [])), + "unread": float(last["ts"]) > last_read and last.get("user") != client.me, + "first": _shape(client, msgs[0]), + "latest": [_shape(client, m) for m in msgs[-4:]] if len(msgs) > 1 else [], + } + + +def collect(client: Client, settings: Settings, now: float | None = None, + progress: Progress | None = None) -> tuple[dict, list[dict]]: + """Return (threads document, your messages) for the last `settings.days`.""" + now = time.time() if now is None else now + since = now - settings.days * 86400 + after = (datetime.fromtimestamp(now) - timedelta(days=settings.days + 1)).strftime("%Y-%m-%d") + candidates: dict[tuple[str, str], None] = {} + dm_channels: set[str] = set() + + for query, kind in ((f"<@{client.me}> after:{after}", "mention"), (f"is:dm after:{after}", "dm"), + (f"to:me after:{after}", "dm")): + for m in client.search(query, max_pages=59): + if float(m["ts"]) < since: + continue + link_query = urllib.parse.urlparse(m.get("permalink", "")).query + thread_ts = urllib.parse.parse_qs(link_query).get("thread_ts", [m["ts"]])[0] + if kind == "dm" and thread_ts == m["ts"]: + dm_channels.add(m["channel"]["id"]) + else: + candidates.setdefault((m["channel"]["id"], thread_ts), None) + + for cid in settings.watched_channels: + cursor = "" + while True: + r = client.call("conversations.history", + {"channel": cid, "oldest": since - 14 * 86400, "limit": 200, "cursor": cursor}) + for m in r.get("messages", []): + if float(m.get("latest_reply") or m["ts"]) >= since \ + and m.get("subtype") not in ("channel_join", "bot_message"): + candidates.setdefault((cid, m.get("thread_ts") or m["ts"]), None) + cursor = r.get("response_metadata", {}).get("next_cursor", "") + if not cursor: + break + + threads = [] + # A DM surfaced any way gets its full in-window history, so a DM ask isn't + # cut down to the one message search matched. + dm_hits = set(dm_channels) + for cid, thread_ts in list(candidates): + if client.channel(cid)["kind"] in ("dm", "mpim"): + dm_hits.add(cid) + del candidates[(cid, thread_ts)] + for cid in sorted(dm_hits): + h = client.call("conversations.history", {"channel": cid, "oldest": since, "limit": 200}) + top = sorted((m for m in h.get("messages", []) if not m.get("subtype")), key=lambda m: float(m["ts"])) + if not top: + continue + for m in top: + if m.get("reply_count"): + candidates.setdefault((cid, m["ts"]), None) + threads.append(_summarize(client, settings, cid, top, top[-1]["ts"])) + + total = len(candidates) + for i, (cid, thread_ts) in enumerate(candidates): + if progress and i % 10 == 0: + progress(i + 1, total) + r = client.call("conversations.replies", {"channel": cid, "ts": thread_ts, "limit": 200}) + msgs = [m for m in r.get("messages", []) if m.get("subtype") not in SKIP_SUBTYPES] + if not msgs or float(msgs[-1]["ts"]) < since: + continue + parent = msgs[0] + parent_read = float(parent.get("last_read") or 0) if parent.get("subscribed") else None + threads.append(_summarize(client, settings, cid, msgs, thread_ts, parent_read)) + + mine = [{"cid": m["channel"]["id"], "ts": m["ts"], "cname": m["channel"].get("name"), + "text": client.clean(m.get("text", "")), "permalink": m.get("permalink", ""), + "dm": bool(m["channel"].get("is_im") or m["channel"].get("is_mpim"))} + for m in client.search(f"from:me after:{after}")] + mine_ts = [(x["cid"], float(x["ts"]), x["permalink"], x["dm"]) for x in mine] + for t in threads: + last = datetime.fromisoformat(t["last_activity"]).timestamp() + t["i_posted_after_in_channel"] = any( + c == t["cid"] and ts >= last - 60 and (dm or f"thread_ts={t['root']}" in link) + for c, ts, link, dm in mine_ts) + + threads.sort(key=lambda t: t["last_activity"], reverse=True) + for i, t in enumerate(threads): + t["id"], t["bucket"] = i + 1, bucket(t, settings) + print(f"collected {len(threads)} threads, {len(mine)} of your messages", file=sys.stderr) + doc = {"generated": datetime.fromtimestamp(now, timezone.utc).isoformat(), "days": settings.days, + "me": client.me, "me_name": client.me_name, "threads": threads} + return doc, mine diff --git a/src/assistant/slack_focus/judge.py b/src/assistant/slack_focus/judge.py new file mode 100644 index 0000000..21b8a55 --- /dev/null +++ b/src/assistant/slack_focus/judge.py @@ -0,0 +1,158 @@ +"""Judge: one bounded model call that turns mechanical candidates into board rows. + +It reads only the candidates triage surfaced, never the full thread set. For +each it keeps or sets aside the row, picks a lane, and writes a short title. +Verdicts are cached by content, so an unchanged thread is never re-judged. +If the model is unavailable, the raw mechanical rows stay, so the board +always renders. +""" +from __future__ import annotations + +import hashlib +import json +import re +from datetime import datetime, timedelta +from typing import Callable + +from .settings import Settings +from .triage import LANE_ORDER, Resolver + +LANES = ("Do first", "This week", "Promise", "Read", "Done") +EFFORTS = ("Quick", "Small", "Medium") +# llm(prompt) -> the model's text answer, or "" when it couldn't run. +LLM = Callable[[str], str] + + +def rubric(owner: str) -> str: + return f"""You are triaging a focus dashboard for {owner}. Below is a JSON array of candidate items that +mechanical rules already surfaced from Slack. For EACH item decide if it is genuinely something {owner} +must act on - a direct question to them, a decision only they make, a review/approval they owe, or a +commitment they made and must deliver. Social chatter, FYIs, things owned by others, or already-answered +items are NOT actionable. + +Most of these do NOT need a reply β€” the thread is complete, it's an FYI, an ack, or someone else +owns the next step. Be strict about what truly needs {owner} to act. + +A request from someone for new work or what to pick up next (e.g. "any small features I can help +with?", "what should I work on?") IS a "Do first" ask β€” {owner} owes them an answer. + +If a candidate is clearly ALREADY DONE by one of {owner}'s merged PRs listed below, mark its lane +"Done" (do not tell them to do it again). + +Return ONLY a JSON array, one object per item you keep (omit pure noise/social chatter): + {{"id": , "lane": "Do first" | "This week" | "Promise" | "Read" | "Done", "title": "<=12-word action or summary>", + "why": "<=15-word reason", "effort": "Quick" | "Small" | "Medium"}} +A direct request in a 1:1 DM (dm=true) β€” a question, a decision, or a personal ask like a promotion +recommendation β€” is "Do first": a specific person is waiting on {owner}. Do NOT demote a 1:1 DM ask to +"This week"; This week is for broader work with nobody blocked on an immediate reply. + +Lanes: "Do first" = someone is waiting on {owner}'s reply or decision now. "This week" = needs them, not urgent. +"Promise" = they said they'd do something and it is NOT already done. "Read" = worth knowing but needs NO +response (thread complete, FYI, decision already made). "Done" = already completed per the merged PRs below. +When unsure between a response-lane and "Read", choose "Read". Keep titles concrete. No prose outside the array.""" + + +def done_evidence(settings: Settings, resolver: Resolver, window_days: int, today: datetime) -> list[str]: + """Your PRs merged in the window, so the judge can retire finished asks.""" + since = (today - timedelta(days=window_days + 2)).strftime("%Y-%m-%d") + evidence = [] + for repo in settings.github_repos: + out = resolver.gh(["pr", "list", "-R", repo, "--author", "@me", "--state", "merged", + "--search", f"merged:>={since}", "--json", "number,title", "--limit", "40"]) + try: + evidence += [f'{repo.split("/")[-1]}#{p["number"]} {p["title"]}' for p in json.loads(out or "[]")] + except (ValueError, KeyError, TypeError): + continue + return evidence + + +def candidates(focus: dict, threads: dict[int, dict]) -> list[dict]: + out = [] + for i, item in enumerate(focus["active"]): + t = threads.get(item.get("thread_id")) + context = f'opener {t["first"]["author"]}: {t["first"]["text"][:160]}' if t else "" + c = {"id": i, "lane": item["lane"], "channel": item["channel"], "from": item["who"], + "dm": item["channel"].startswith("DM "), "text": item["text"][:240], "context": context, + "link": item["link"]} + key = json.dumps([c["link"], c["text"], c["lane"], c["channel"], c["from"], c["dm"], c["context"]], + sort_keys=True) + c["fp"] = hashlib.sha256(key.encode()).hexdigest() + out.append(c) + return out + + +def usable_cache(saved, evidence: list[str], rubric_sig: str) -> dict: + """Prior verdicts, or nothing when the merged-PR set or the rubric changed.""" + if not isinstance(saved, dict) or set(saved.get("done_evidence", [])) != set(evidence) \ + or saved.get("rubric") != rubric_sig or not isinstance(saved.get("verdicts"), dict): + return {} + return {k: v for k, v in saved["verdicts"].items() if v == "DROP" or isinstance(v, dict)} + + +def parse(raw: str) -> dict[int, dict] | None: + m = re.search(r"\[.*\]", raw or "", re.S) + if not m: + return None + try: + data = json.loads(m.group(0)) + return {int(d["id"]): d for d in data if isinstance(d, dict) and "id" in d and d.get("title")} + except (ValueError, TypeError): + return None + + +def judge(focus: dict, threads: dict[int, dict], evidence: list[str], saved_cache, llm: LLM) -> tuple[dict, dict]: + """Return (focus with judged lanes, cache to save). Mutates nothing passed in.""" + focus = json.loads(json.dumps(focus)) + owner = focus.get("me_name") or "you" + text = rubric(owner) + rubric_sig = hashlib.sha256(text.encode()).hexdigest() + cache = usable_cache(saved_cache, evidence, rubric_sig) + cands = candidates(focus, threads) + # Only new or changed candidates go to the model, renumbered 0..k so it + # echoes dense ids. + to_judge = [c for c in cands if c["fp"] not in cache] + local = {c["fp"]: j for j, c in enumerate(to_judge)} + payload = [{"id": j, "lane": c["lane"], "channel": c["channel"], "from": c["from"], "dm": c["dm"], + "text": c["text"], "context": c["context"]} for j, c in enumerate(to_judge)] + prompt = (f"{text}\n\nALREADY DONE β€” {owner}'s merged PRs this window:\n" + f"{chr(10).join(evidence) or '(none found)'}\n\nCANDIDATES:\n{json.dumps(payload, indent=0)}\n") + model = parse(llm(prompt)) if to_judge else {} + + if to_judge and not model: + focus["judge"] = {"ran": False, "note": "The model didn't return usable rows, so these are the raw ones."} + return focus, saved_cache if isinstance(saved_cache, dict) else {} + + judged, new_cache = {}, {} + for c in cands: + if c["fp"] in cache: + v = cache[c["fp"]] + else: + mv = model.get(local[c["fp"]]) + v = {k: mv[k] for k in ("lane", "title", "why", "effort") if k in mv} if mv else "DROP" + new_cache[c["fp"]] = v + if v != "DROP": + judged[c["id"]] = v + + kept, read, dropped = [], [], [] + for i, item in enumerate(focus["active"]): + v = judged.get(i) + if not v: + dropped.append({"text": item["text"], "channel": item["channel"], "who": item.get("who", ""), + "link": item["link"], "why": "not something you need to act on"}) + continue + item = dict(item) + lane = v.get("lane") if v.get("lane") in LANES else item["lane"] + item.update(lane=lane, text=v["title"], why=v.get("why", ""), + effort=v["effort"] if v.get("effort") in EFFORTS else "") + if lane == "Done": + focus["handled"].append({"text": item["text"], "channel": item["channel"], "link": item["link"], + "why": v.get("why", "already done")}) + else: + (read if lane == "Read" else kept).append(item) + kept.sort(key=lambda i: (LANE_ORDER.index(i["lane"]) if i["lane"] in LANE_ORDER else 9, -i.get("rank", 0))) + read.sort(key=lambda i: -i.get("rank", 0)) + focus.update(active=kept, read=read, judge_dropped=dropped) + focus["counts"]["handled"] = len(focus["handled"]) + focus["judge"] = {"ran": True, "respond": len(kept), "read": len(read), "dropped": len(dropped), + "cached": len(cands) - len(to_judge), "rejudged": len(to_judge)} + return focus, {"done_evidence": sorted(evidence), "rubric": rubric_sig, "verdicts": new_cache} diff --git a/src/assistant/slack_focus/settings.py b/src/assistant/slack_focus/settings.py new file mode 100644 index 0000000..e809f5a --- /dev/null +++ b/src/assistant/slack_focus/settings.py @@ -0,0 +1,85 @@ +"""The user's Slack focus settings: the `slack_focus` block of comms/config.json. + +Everything specific to one person lives here: the channels they watch, the +bots they ignore, the repos their PRs land in, and their Jira projects. Their +Slack identity and workspace come from the token, so they're never configured. +""" +from __future__ import annotations + +import json +import os +from dataclasses import dataclass, field +from pathlib import Path + +DEFAULT_DAYS = 7 +DEFAULT_TITLE = "Slack focus" + + +@dataclass(frozen=True) +class JiraSettings: + enabled: bool = False + base_url: str = "" + token_env: str = "JIRA_API_TOKEN" + email_env: str = "JIRA_EMAIL" + project_keys: tuple[str, ...] = () + + +@dataclass(frozen=True) +class Settings: + configured: bool = False + days: int = DEFAULT_DAYS + title: str = DEFAULT_TITLE + watched_channels: dict[str, str] = field(default_factory=dict) + bot_authors: tuple[str, ...] = () + skip_channels: tuple[str, ...] = () + github_repos: tuple[str, ...] = () + github_hosts: tuple[str, ...] = ("github.com",) + jira: JiraSettings = field(default_factory=JiraSettings) + + +def config_path(home: Path | None = None) -> Path: + return (home or Path(os.environ.get("HOME", str(Path.home())))) / ".assistant/comms/config.json" + + +def _strings(value) -> tuple[str, ...]: + if not isinstance(value, list): + return () + return tuple(str(v) for v in value if isinstance(v, str) and v.strip()) + + +def parse(block) -> Settings: + """Build Settings from the raw `slack_focus` block. Wrong-typed fields fall + back to their defaults, so a hand-edit typo degrades one knob, not the tab.""" + if not isinstance(block, dict): + return Settings() + days = block.get("days") + watched = block.get("watched_channels") + jira = block.get("jira") if isinstance(block.get("jira"), dict) else {} + hosts = _strings(block.get("github_hosts")) + return Settings( + configured=True, + days=days if isinstance(days, int) and not isinstance(days, bool) and 1 <= days <= 30 else DEFAULT_DAYS, + title=block.get("title") if isinstance(block.get("title"), str) and block["title"].strip() else DEFAULT_TITLE, + watched_channels={str(k): str(v) for k, v in watched.items()} if isinstance(watched, dict) else {}, + bot_authors=_strings(block.get("bot_authors")), + skip_channels=_strings(block.get("skip_channels")), + github_repos=_strings(block.get("github_repos")), + github_hosts=hosts or ("github.com",), + jira=JiraSettings( + enabled=jira.get("enabled") is True, + base_url=str(jira.get("base_url") or "").rstrip("/"), + token_env=str(jira.get("token_env") or "JIRA_API_TOKEN"), + email_env=str(jira.get("email_env") or "JIRA_EMAIL"), + project_keys=tuple(k.upper() for k in _strings(jira.get("project_keys"))), + ), + ) + + +def load(home: Path | None = None) -> Settings: + """Read the live settings. A missing or unreadable config means the board + isn't set up yet, which the tab shows as setup steps, never as an error.""" + try: + doc = json.loads(config_path(home).read_text()) + except (OSError, json.JSONDecodeError): + return Settings() + return parse(doc.get("slack_focus") if isinstance(doc, dict) else None) diff --git a/src/assistant/slack_focus/store.py b/src/assistant/slack_focus/store.py new file mode 100644 index 0000000..59f6957 --- /dev/null +++ b/src/assistant/slack_focus/store.py @@ -0,0 +1,119 @@ +"""Files under ~/.assistant/slack-focus/ and the small stores built on them. + + threads.json every collected thread, bucketed + mine.json every message you sent in the window + focus.json the triaged and judged board + judge-cache.json verdicts keyed by candidate content + done.json items you checked off: {fingerprint: when} + state.json the current or last refresh: running, phase, ok + refresh.log the last refresh's output + +Every write is atomic (temp file + rename), so a dashboard render never reads +a half-written file. +""" +from __future__ import annotations + +import fcntl +import hashlib +import json +import os +import tempfile +from contextlib import contextmanager +from datetime import datetime, timezone +from pathlib import Path + + +def base_dir(home: Path | None = None) -> Path: + return (home or Path(os.environ.get("HOME", str(Path.home())))) / ".assistant/slack-focus" + + +def now_iso() -> str: + return datetime.now(timezone.utc).replace(microsecond=0).isoformat() + + +def read_json(path: Path, default=None): + try: + return json.loads(path.read_text()) + except (OSError, ValueError): + return default + + +def write_json(path: Path, data) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(prefix=f".{path.name}.", dir=str(path.parent)) + try: + with os.fdopen(fd, "w") as f: + json.dump(data, f, indent=1, ensure_ascii=False) + os.replace(tmp, path) + finally: + if os.path.exists(tmp): + os.unlink(tmp) + + +@contextmanager +def locked(path: Path, blocking: bool = True): + """An exclusive flock on `path`. Yields False when non-blocking and taken.""" + path.parent.mkdir(parents=True, exist_ok=True) + with open(path, "a+") as f: + try: + fcntl.flock(f.fileno(), fcntl.LOCK_EX | (0 if blocking else fcntl.LOCK_NB)) + except BlockingIOError: + yield False + return + try: + yield True + finally: + fcntl.flock(f.fileno(), fcntl.LOCK_UN) + + +def fingerprint(item: dict, threads: dict[int, dict]) -> str: + """Key a checked-off item by its link plus its thread's latest message. A + new reply or an edit changes the key, so the item comes back on its own.""" + t = threads.get(item.get("thread_id")) + last = (t.get("latest") or [t["first"]])[-1] if t else {} + signature = (last.get("text", "") + "|" + last.get("at", "")) if t else "" + return hashlib.sha256((item["link"] + "||" + signature).encode()).hexdigest() + + +def set_done(home: Path | None, fp: str, done: bool) -> None: + root = base_dir(home) + with locked(root / "done.lock"): + marks = read_json(root / "done.json", {}) + marks = marks if isinstance(marks, dict) else {} + if done: + marks[fp] = now_iso() + else: + marks.pop(fp, None) + write_json(root / "done.json", marks) + + +def pid_alive(pid) -> bool: + if not isinstance(pid, int) or pid <= 0: + return False + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + except PermissionError: + return True + return True + + +def read_state(home: Path | None = None) -> dict: + """The refresh state. A run whose process died mid-way reads as failed.""" + state = read_json(base_dir(home) / "state.json", {}) + state = state if isinstance(state, dict) else {} + if state.get("running") and not pid_alive(state.get("pid")): + state.update(running=False, ok=False, phase=None, + error=state.get("error") or "The refresh stopped before it finished.") + return state + + +def write_state(home: Path | None, **fields) -> dict: + root = base_dir(home) + with locked(root / "state.lock"): + state = read_json(root / "state.json", {}) + state = state if isinstance(state, dict) else {} + state.update(fields) + write_json(root / "state.json", state) + return state diff --git a/src/assistant/slack_focus/triage.py b/src/assistant/slack_focus/triage.py new file mode 100644 index 0000000..d4d7130 --- /dev/null +++ b/src/assistant/slack_focus/triage.py @@ -0,0 +1,189 @@ +"""Triage: turn bucketed threads and your messages into board lanes. No model. + + - ball-in-your-court buckets map to lanes (Do first, This week, Optional) + - a thread resolves to "Auto-resolved" when every linked PR is merged or + closed, or every linked Jira ticket is done + - your own messages that read like a commitment become Promises + - nothing is dropped here; low-signal threads only carry a hint for the judge +""" +from __future__ import annotations + +import base64 +import json +import os +import re +import subprocess +import urllib.request +from datetime import datetime, timezone +from typing import Callable + +from .settings import Settings + +LANE = {"ball_mine_dm": "Do first", "ball_mine_mention": "Do first", + "thread_moved_on": "This week", "group_mention": "This week", "unread_watched": "Optional"} +LANE_ORDER = ["Do first", "Promise", "This week", "Optional"] +DIGEST_RE = re.compile( + r"standup|TL;DR|KEY (DECISIONS|TOPICS|OUTCOMES)|:clipboard:|weekly sync|e-?scrum|:pushpin:", re.I) +PROMISE_RE = re.compile( + r"\b(i'?ll|i will|let me|i can (?:get|take|look|help|make|add|fix|do|share|send)|" + r"i'?m (?:on|looking|working)|will (?:do|send|share|fix|add|look|get|create|remove|update|report|sync|merge)|" + r"on it|going to|plan to|by (?:mon|tue|wed|thu|fri|tomorrow|eod|eow|next week)|tomorrow|next week|" + r"report back)\b", re.I) +DELIVERABLE_RE = re.compile( + r"\b(PR|ticket|review|fix|doc|docs|test|tests|channel|meeting|canvas|data|slide|plan|demo|epic|branch|" + r"upgrader|pipeline|flag|patent|rule|skill|dashboard|wiki|list|invite|agent|eval)s?\b", re.I) +Runner = Callable[..., subprocess.CompletedProcess] + + +def pr_regex(hosts) -> re.Pattern: + host = "|".join(re.escape(h) for h in hosts) + return re.compile(rf"(?:https?://)?({host})/([\w.-]+/[\w.-]+)/pull/(\d+)") + + +def jira_regex(keys) -> re.Pattern | None: + if not keys: + return None + return re.compile(r"\b(?:" + "|".join(re.escape(k) for k in keys) + r")-\d+\b") + + +class Resolver: + """Answers "is this PR or ticket closed?" with gh and the Jira REST API, + caching each answer for the run. Any failure counts as still open.""" + + def __init__(self, settings: Settings, run: Runner = subprocess.run, env: dict | None = None, + urlopen=urllib.request.urlopen): + self.settings = settings + self._run = run + self._env = os.environ if env is None else env + self._urlopen = urlopen + self._pr: dict[tuple, bool] = {} + self._jira: dict[str, bool] = {} + + def gh(self, args: list[str], timeout: int = 40) -> str | None: + try: + out = self._run(["gh", *args], capture_output=True, text=True, timeout=timeout) + except (OSError, subprocess.SubprocessError): + return None + return out.stdout if out.returncode == 0 else None + + def pr_closed(self, host: str, repo: str, number: str) -> bool: + key = (host, repo, number) + if key not in self._pr: + target = repo if host == "github.com" else f"{host}/{repo}" + out = self.gh(["pr", "view", number, "-R", target, "--json", "state"], timeout=30) + try: + self._pr[key] = out is not None and json.loads(out)["state"] in ("MERGED", "CLOSED") + except (ValueError, KeyError, TypeError): + self._pr[key] = False + return self._pr[key] + + def jira_done(self, key: str) -> bool: + jira = self.settings.jira + if key not in self._jira: + base = jira.base_url or self._env.get("JIRA_BASE_URL", "").rstrip("/") + token = self._env.get(jira.token_env, "") + email = self._env.get(jira.email_env, "") + done = False + if base.startswith("https://") and token: + auth = ("Basic " + base64.b64encode(f"{email}:{token}".encode()).decode()) if email \ + else "Bearer " + token + req = urllib.request.Request(f"{base}/rest/api/2/issue/{key}?fields=status", + headers={"Authorization": auth}) + try: + fields = json.loads(self._urlopen(req, timeout=20).read())["fields"] + done = fields["status"]["statusCategory"]["key"] == "done" + except Exception: # noqa: BLE001 β€” an unreachable ticket stays open + done = False + self._jira[key] = done + return self._jira[key] + + +def _recent_text(t: dict) -> str: + return " ".join(m["text"] for m in (t.get("latest") or [])) + " " + t["first"]["text"] + + +def resolution(t: dict, resolver: Resolver, prs_re: re.Pattern, keys_re: re.Pattern | None) -> str | None: + blob = _recent_text(t) + prs = set(prs_re.findall(blob)) + if prs and all(resolver.pr_closed(*p) for p in prs): + return "linked PR merged or closed" + keys = set(keys_re.findall(blob)) if keys_re and resolver.settings.jira.enabled else set() + if keys and all(resolver.jira_done(k) for k in keys): + return "linked ticket done" + return None + + +def noise(t: dict, me_name: str) -> str | None: + """A hint for the judge that a thread looks low-signal. It never drops.""" + if DIGEST_RE.search(t["first"]["text"]): + return "digest or standup post" + recent = t.get("latest") or [] + if t["bucket"] == "thread_moved_on" and "?" not in " ".join(m["text"] for m in recent): + return "discussion, no open question" + if recent and len(recent[-1]["text"]) < 40 and recent[-1]["author"] != me_name: + return "trailed off (short ack)" + return None + + +def rank(t: dict) -> float: + score = 0.0 + if t["mentioned_after_my_last_reply"] == "direct" or t.get("kind") == "dm": + score += 100 + if "?" in " ".join(m["text"] for m in (t.get("latest") or [t["first"]])): + score += 20 + return score + datetime.fromisoformat(t["last_activity"]).timestamp() / 1e9 + + +def triage(threads_doc: dict, mine: list[dict], settings: Settings, resolver: Resolver) -> dict: + me_name = threads_doc.get("me_name") or "" + prs_re = pr_regex(settings.github_hosts) + keys_re = jira_regex(settings.jira.project_keys) + active, handled = [], [] + for t in threads_doc["threads"]: + lane = LANE.get(t["bucket"]) + if not lane: + continue + why = resolution(t, resolver, prs_re, keys_re) + if why: + handled.append({"text": t["first"]["text"][:140], "channel": t["channel"], "link": t["permalink"], + "why": why}) + continue + ask = next((m for m in reversed(t.get("latest") or []) if m["author"] != me_name), None) or t["first"] + # A 1:1 DM is your ask. A group DM that trails off with no question and + # no @-mention of you waits for this week instead. + if t["bucket"] == "ball_mine_dm" and t.get("kind") == "mpim" and "?" not in (ask["text"] or "") \ + and not t["mentioned_after_my_last_reply"]: + lane = "This week" + active.append({ + "low_signal": noise(t, me_name), "lane": lane, "channel": t["channel"], "who": t["last_author"], + "text": (ask["text"] or t["first"]["text"])[:160], "link": t["permalink"], + "mention": t["mentioned_after_my_last_reply"] == "direct", + "rank": rank(t), "last": t["last_activity"][:16], "thread_id": t["id"], + }) + + seen_prs = set() + for m in mine: + text = m["text"] + if not (PROMISE_RE.search(text) and DELIVERABLE_RE.search(text) and len(text) > 30 + and not text.rstrip().endswith("?")): + continue + prs = set(prs_re.findall(text)) + key = tuple(sorted(prs)) + if key and key in seen_prs: + continue + seen_prs.add(key) + cname = m.get("cname") or "" + channel = ("group DM" if cname.startswith("mpdm-") else "DM") if m.get("dm") else "#" + cname + if prs and all(resolver.pr_closed(*p) for p in prs): + handled.append({"text": text[:140], "channel": channel, "link": m["permalink"], + "why": "linked PR merged or closed"}) + continue + active.append({"lane": "Promise", "channel": channel, "who": "you", "text": text[:200], + "link": m["permalink"], "mention": False, "rank": float(m["ts"]), "last": ""}) + + active.sort(key=lambda i: (LANE_ORDER.index(i["lane"]), -i["rank"])) + return { + "generated": datetime.now(timezone.utc).isoformat(), "window_days": threads_doc["days"], + "me_name": me_name, "active": active, "handled": handled, + "counts": {"active": len(active), "handled": len(handled), "threads": len(threads_doc["threads"])}, + } diff --git a/tests/drive_dashboard_overview.py b/tests/drive_dashboard_overview.py index c9dd014..a688025 100644 --- a/tests/drive_dashboard_overview.py +++ b/tests/drive_dashboard_overview.py @@ -6,6 +6,7 @@ import importlib.util import json import os +import sys import threading from datetime import datetime, timedelta, timezone from functools import partial @@ -114,6 +115,31 @@ def drive(output_dir, browser_executable, coverage_dir=None): "repo": "example/project", "pr": index, }, }) + "\n" for index in range(1, 50))) + slack_root = home / ".assistant/slack-focus" + slack_root.mkdir(parents=True) + (home / ".assistant/comms").mkdir(parents=True) + (home / ".assistant/comms/config.json").write_text(json.dumps({"slack_focus": { + "watched_channels": {"C1": "team"}}})) + slack_threads = [{"id": n, "first": {"text": f"thread {n}", "at": "1"}, "latest": []} for n in (1, 2, 3)] + slack_items = [ + {"lane": "Do first", "text": "Answer Aalia about the take-trim design, which she asked about twice " + "in the thread and again in a DM", "link": "https://acme.slack.com/archives/D1/p1", + "thread_id": 1, "channel": "DM Aalia Habib", "who": "Aalia Habib", "last": "2026-10-18T09:30", + "mention": True, "why": "she is waiting", "effort": "Quick"}, + {"lane": "This week", "text": "Share the roadmap", "link": "https://acme.slack.com/archives/C1/p2", + "thread_id": 2, "channel": "#team", "who": "Tim Beauchamp", "last": "2026-10-17T10:00", + "effort": "Medium"}, + {"lane": "Promise", "text": "Send the freeze plan", "link": "https://acme.slack.com/archives/C1/p3", + "thread_id": 3, "channel": "#team", "who": "you", "last": ""}, + ] + (slack_root / "threads.json").write_text(json.dumps({"threads": slack_threads})) + (slack_root / "focus.json").write_text(json.dumps({ + "generated": now.isoformat(), "window_days": 7, "judge": {"ran": True}, "active": slack_items, + "read": [], "judge_dropped": [], "handled": []})) + sys.path.insert(0, str(REPO / "src")) + from assistant.slack_focus import store as slack_store + (slack_root / "done.json").write_text(json.dumps({ + slack_store.fingerprint(slack_items[2], {t["id"]: t for t in slack_threads}): now.isoformat()})) with mock.patch.dict(os.environ, {"HOME": str(home)}): spec = importlib.util.spec_from_file_location( "browser_renderer", REPO / "bin/render-assistant-page.py") @@ -305,13 +331,16 @@ def reject_action(route): render_version = 100 - def refresh_fixture(): + def render_only(): nonlocal render_version render_version += 1 (output / "cache/world.json").write_text(json.dumps(world)) with mock.patch.dict(os.environ, {"HOME": str(home)}), mock.patch.object( renderer, "utc_now", return_value=now + timedelta(seconds=render_version)): renderer.render() + + def refresh_fixture(): + render_only() page.evaluate("refreshDashboard(true)") refresh_fixture() @@ -505,6 +534,160 @@ def refresh_fixture(): refresh_fixture() assert page.locator('[data-panel="overview"]').is_visible() + slack_calls = [] + slack_script = {"status": [], "refresh": 200, "mark": 200} + + def slack_route(route): + action = route.request.url.rsplit("/slack-focus/", 1)[1] + slack_calls.append((action, route.request.post_data)) + if action == "status": + answer = slack_script["status"].pop(0) if slack_script["status"] else {"running": False} + if answer == 500: + route.fulfill(status=500, body="down") + else: + route.fulfill(status=200, content_type="application/json", body=json.dumps(answer)) + elif action == "refresh": + route.fulfill(status=slack_script["refresh"], content_type="application/json", + body='{"started": true}') + elif slack_script["mark"] == 200: + slack_store.set_done(home, json.loads(route.request.post_data)["fp"], action == "done") + render_only() + route.fulfill(status=200, body="saved") + else: + route.fulfill(status=slack_script["mark"], body="fp must be a 64-character hex fingerprint") + + page.route("**/slack-focus/*", slack_route) + page.locator('[data-tab="slack"]').click() + slack_panel = page.locator('[data-panel="slack"]') + assert page.locator('[data-tab="slack"] .tab-count').inner_text() == "1" + assert slack_panel.locator(".sf-do .sf-row").count() == 1 + assert "@you" in slack_panel.locator(".sf-do .sf-row").inner_text() + assert slack_panel.locator('[data-context-key="sf-done"] .sf-row').count() == 1 + for width in (390, 820, 1440): + page.set_viewport_size({"width": width, "height": 1000}) + measurement = page.evaluate("""() => ({ + offsetWidth: document.documentElement.offsetWidth, + scrollWidth: document.documentElement.scrollWidth, + overflowingRows: [...document.querySelectorAll('[data-panel="slack"] .sf-row')] + .filter(row => row.offsetWidth && row.scrollWidth > row.offsetWidth).length + })""") + assert measurement["scrollWidth"] == measurement["offsetWidth"], measurement + assert measurement["overflowingRows"] == 0, measurement + page.screenshot(path=str(output_dir / f"slack-{width}.png"), full_page=True) + slack_script["status"] = [{"running": True, "phase": "Pulling Slack (1 of 2 threads)"}, + {"running": False, "ok": True}] + page.locator("#sf-refresh").click() + page.wait_for_function("""() => document.getElementById('sf-status') + .textContent === 'Refreshing: Pulling Slack (1 of 2 threads).'""") + assert page.locator("#sf-refresh").is_disabled() + assert page.locator("#sf-refresh").inner_text() == "Refreshing…" + page.evaluate("() => { slackFocusPoll(); }") + page.clock.fast_forward(2000) + page.wait_for_function("text => document.getElementById('sf-status').textContent === text", + arg="Refresh finished. The list updates within a minute.") + assert [a for a, _ in slack_calls] == ["refresh", "status", "status"], slack_calls + assert page.locator("#sf-refresh").inner_text() == "Refresh from Slack" + for answer, message in (({"running": False, "ok": False, "error": "Slack search.messages failed"}, + "The refresh didn't finish: Slack search.messages failed"), + ({"running": False, "ok": False}, + "The refresh didn't finish: unknown error")): + slack_script["status"] = [answer] + page.evaluate("slackFocusPoll()") + assert page.locator("#sf-status").inner_text() == message + slack_script["status"] = [{"running": True}, 500] + page.evaluate("() => { slackFocusPoll(); }") + try: + page.wait_for_function("() => document.getElementById('sf-status').textContent === 'Refreshing: Starting.'", + timeout=5000) + except Exception: + print("DEBUG", page.locator('#sf-status').inner_text(), slack_calls, slack_script) + raise + page.clock.fast_forward(2000) + page.wait_for_function("text => document.getElementById('sf-status').textContent === text", + arg="Couldn't check the refresh: the server returned 500") + slack_script["refresh"] = 500 + page.locator("#sf-refresh").click() + page.wait_for_function("text => document.getElementById('sf-status').textContent === text", + arg="Couldn't start the refresh: the server returned 500") + assert not page.locator("#sf-refresh").is_disabled() + slack_script["refresh"] = 200 + fingerprint = page.locator('.sf-do .sf-mark').get_attribute("data-fp") + page.locator('.sf-do .sf-mark').click() + page.wait_for_function("() => document.querySelectorAll('[data-context-key=\"sf-done\"] .sf-row').length === 2") + assert slack_calls[-1] == ("done", json.dumps({"fp": fingerprint}, separators=(",", ":"))) + assert page.locator(".sf-do .sf-row").count() == 0 + assert page.locator('[data-tab="slack"] .tab-count').inner_text() == "0" + page.locator('[data-context-key="sf-done"] > summary').click() + page.locator(f'[data-context-key="sf-done"] .sf-mark[data-fp="{fingerprint}"]').click() + page.wait_for_function("() => document.querySelectorAll('.sf-do .sf-row').length === 1") + assert slack_calls[-1][0] == "undone" + slack_script["mark"] = 400 + page.locator('.sf-do .sf-mark').click() + page.wait_for_function("text => document.querySelector('.sf-do .sf-mark').textContent === text", + arg="Couldn't save. Try again") + assert not page.locator('.sf-do .sf-mark').is_disabled() + slack_script["mark"] = 200 + page.evaluate("""() => { + document.getElementById('sf-status').remove(); + slackFocusShow({running: true}); + }""") + assert not page.locator("#sf-refresh").is_disabled() + def advance_until(predicate, steps=20): + for _ in range(steps): + if page.evaluate(predicate): + return + page.clock.fast_forward(1000) + page.wait_for_timeout(50) + raise AssertionError(predicate) + + def wait_for_calls(count): + for _ in range(100): + if len(slack_calls) >= count: + return + page.wait_for_timeout(20) + raise AssertionError((count, slack_calls)) + + running_state = json.dumps({"running": True, "pid": os.getpid(), "phase": "Sorting threads"}) + shows_running = "() => document.getElementById('slack-focus').dataset.running === 'true'" + + def finished_at(seconds): + return {"running": False, "ok": True, + "finished": (now + timedelta(seconds=seconds)).isoformat()} + + for resume in ("refresh", "load"): + (slack_root / "state.json").write_text(running_state) + before_resume = len(slack_calls) + slack_script["status"] = [finished_at(render_version + 2)] + if resume == "refresh": + refresh_fixture() + else: + render_only() + page.reload() + wait_for_calls(before_resume + 1) + assert page.evaluate(shows_running), resume + assert page.locator("#sf-status").inner_text() == "Loading the new list…" + (slack_root / "state.json").unlink() + render_only() + advance_until("() => document.getElementById('slack-focus').dataset.running === 'false'") + assert [a for a, _ in slack_calls[before_resume:]] == ["status"], slack_calls[before_resume:] + assert not page.locator("#sf-refresh").is_disabled() + assert page.locator("#sf-status").inner_text().startswith("Last pulled") + (slack_root / "state.json").write_text(running_state) + slack_script["status"] = [finished_at(10 ** 6)] + before_stuck = len(slack_calls) + refresh_fixture() + wait_for_calls(before_stuck + 1) + for _ in range(17): + page.clock.fast_forward(1000) + page.wait_for_timeout(30) + assert page.evaluate(shows_running) + assert page.locator("#sf-status").inner_text() == "Refresh finished. The list updates within a minute." + (slack_root / "state.json").unlink() + refresh_fixture() + page.unroute("**/slack-focus/*") + page.locator('[data-tab="overview"]').click() + assert not errors, errors + file_page = page.context.new_page() file_errors = [] file_page.on("pageerror", lambda error: file_errors.append(str(error))) @@ -540,7 +723,7 @@ def refresh_fixture(): "explicit_focus_requests": len(mutations), "intercepted_brief_seen_requests": len(seen), "coverage_script_instances": len(scripts) if profiler is not None else 0, - "checks": "grouping, density, search restoration, expansion, clipboard denial and expiry, reading stability, legacy focus, missing task guards, refresh errors and concurrency, brief date changes, file protocol, observation expiry, stale request gating"})) + "checks": "grouping, density, search restoration, expansion, clipboard denial and expiry, reading stability, legacy focus, missing task guards, refresh errors and concurrency, brief date changes, file protocol, observation expiry, stale request gating, Slack refresh, check-off, and errors"})) finally: server.shutdown() server.server_close() diff --git a/tests/slack_focus_fixtures.py b/tests/slack_focus_fixtures.py new file mode 100644 index 0000000..c291381 --- /dev/null +++ b/tests/slack_focus_fixtures.py @@ -0,0 +1,155 @@ +"""A small fake Slack workspace for the Slack focus tests. No network.""" +from __future__ import annotations + +import json + +NOW = 1_790_000_000.0 +DAY = 86400 + + +def ts(days_ago: float, n: int = 0) -> str: + return f"{NOW - days_ago * DAY:.0f}.{n:06d}" + + +USERS = {"U1": "Mukul Sharma", "U2": "Aalia Habib", "U3": "Tim Beauchamp", "U4": "Deploy Bot"} +CHANNELS = { + "C1": {"name": "team", "last_read": ts(3)}, + "C2": {"name": "prod-deployments", "last_read": ts(0)}, + "D1": {"is_im": True, "user": "U2", "last_read": ts(5)}, + "G1": {"is_mpim": True, "last_read": ts(5)}, +} + + +def msg(user, when, text, **extra): + return {"user": user, "ts": when, "text": text, **extra} + + +class FakeSlack: + """transport(method, params, token) over a fixed set of threads.""" + + def __init__(self, threads=None, history=None, search=None, auth=None): + self.calls: list[tuple[str, dict]] = [] + self.threads = threads if threads is not None else default_threads() + self.history = history if history is not None else default_history() + self.search_results = search if search is not None else default_search() + self.auth = auth or {"ok": True, "user_id": "U1", "url": "https://acme.slack.com/"} + + def __call__(self, method, params, token): + self.calls.append((method, dict(params))) + if method == "auth.test": + return self.auth + if method == "users.info": + return {"user": {"real_name": USERS.get(params["user"], "")}} + if method == "conversations.info": + return {"channel": dict(CHANNELS.get(params["channel"], {"name": params["channel"]}))} + if method == "usergroups.users.list": + return {"users": ["U1"] if params["usergroup"] == "S1" else ["U9"]} + if method == "search.messages": + query = params["query"] + key = next(k for k in ("from:me", "is:dm", "to:me", "<@U1>") if k in query) + matches = self.search_results.get(key, []) + page = params["page"] + chunk = matches[(page - 1) * 2:page * 2] + pages = max(1, (len(matches) + 1) // 2) + return {"messages": {"matches": chunk, "paging": {"pages": pages}}} + if method == "conversations.history": + msgs = self.history.get(params["channel"], []) + if params.get("cursor") == "" and params["channel"] == "C1" and len(msgs) > 1: + return {"messages": msgs[:1], "response_metadata": {"next_cursor": "next"}} + if params.get("cursor") == "next": + return {"messages": msgs[1:], "response_metadata": {"next_cursor": ""}} + return {"messages": msgs} + if method == "conversations.replies": + return {"messages": self.threads.get((params["channel"], params["ts"]), [])} + raise AssertionError(f"unexpected method {method}") + + +def default_threads(): + return { + # Tim asked Mukul directly in a watched channel; no reply yet. + ("C1", ts(1)): [msg("U3", ts(1), "Who owns the path bug? <@U1> can you check?"), + msg("U2", ts(0.9), "I think it's walnut")], + # Mukul answered last: waiting on others. + ("C1", ts(2)): [msg("U2", ts(2), "Can you review ?"), + msg("U1", ts(1.5), "Done, approved")], + # A group mention of a group Mukul is in. + ("C1", ts(2.5)): [msg("U3", ts(2.5), " please look at the freeze plan"), + msg("U3", ts(2.4), "anyone?")], + # Mukul took part and others kept going, with a question. + ("C1", ts(4)): [msg("U1", ts(4), "Let's split the work"), + msg("U2", ts(3.8), "Should I take the importer? What about tests?")], + # Unread watched-channel post with nobody addressing Mukul. + ("C1", ts(0.5)): [msg("U3", ts(0.5), "FYI the build is green again after the fix landed")], + # A deploy bot post in a skipped channel. + ("C2", ts(1)): [msg("U4", ts(1), "Deployed <@U1> build 42")], + # Mukul reacted to the last message: handled elsewhere. + ("C1", ts(3)): [msg("U3", ts(3), "<@U1> ship it?", + reactions=[{"name": "+1", "users": ["U1"]}])], + # A thread that ended before the window. + ("C1", ts(9)): [msg("U3", ts(9), "old <@U1>")], + # A DM thread with replies. + ("D1", ts(1)): [msg("U2", ts(1), "thread root"), msg("U2", ts(0.8), "reply in thread")], + } + + +def default_history(): + return { + "C1": [{"ts": ts(0.5), "user": "U3", "text": "FYI"}, + {"ts": ts(4), "latest_reply": ts(3.8), "user": "U1", "text": "Let's split the work"}, + {"ts": ts(2), "latest_reply": ts(1.5), "thread_ts": ts(2), "user": "U2", "text": "review"}, + {"ts": ts(2.5), "latest_reply": ts(2.4), "user": "U3", "text": "freeze plan"}, + {"ts": ts(20), "user": "U3", "text": "too old"}, + {"ts": ts(1), "subtype": "channel_join"}], + "D1": [msg("U2", ts(1), "thread root", reply_count=1), + msg("U2", ts(0.7), "Any small features I can pick up?"), + {"ts": ts(0.6), "subtype": "bot_message", "text": "ignored"}], + "G1": [msg("U3", ts(2), "fyi all, slides are in the folder")], + } + + +def _hit(cid, when, text, thread_ts=None, user="U3", **channel): + link = f"https://acme.slack.com/archives/{cid}/p{when.replace('.', '')}" + if thread_ts: + link += f"?thread_ts={thread_ts}&cid={cid}" + return {"ts": when, "text": text, "user": user, "permalink": link, + "channel": {"id": cid, "name": channel.get("name", cid), **channel}} + + +def default_search(): + return { + "<@U1>": [_hit("C1", ts(1), "<@U1> can you check?", thread_ts=ts(1)), + _hit("C1", ts(3), "<@U1> ship it?"), + _hit("C2", ts(1), "Deployed"), + _hit("C1", ts(9), "old <@U1>"), + _hit("G1", ts(2), "hi", is_mpim=True)], + "is:dm": [_hit("D1", ts(0.7), "Any small features I can pick up?", is_im=True)], + "to:me": [_hit("D1", ts(0.8), "reply in thread", thread_ts=ts(1), is_im=True)], + "from:me": [ + _hit("C1", ts(1.5), "Done, approved", thread_ts=ts(2), user="U1", name="team"), + _hit("C1", ts(2.2), "I'll send the freeze plan doc by friday", user="U1", name="team"), + _hit("C1", ts(2.1), "I'll fix the importer in https://github.com/acme/app/pull/9 today", + user="U1", name="team"), + _hit("C1", ts(2.0), "I will review https://github.com/acme/app/pull/9 again soon", + user="U1", name="team"), + _hit("C1", ts(1.9), "Will merge https://github.com/acme/app/pull/8 after the tests pass", + user="U1", name="team"), + _hit("D1", ts(1.8), "Should I send the plan doc tomorrow?", user="U1", is_im=True), + ], + } + + +def write_config(home, block): + path = home / ".assistant/comms/config.json" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps({"llm": {}, "slack_focus": block})) + return path + + +CONFIG = { + "days": 7, + "watched_channels": {"C1": "team"}, + "bot_authors": ["Deploy Bot"], + "skip_channels": ["deployments"], + "github_repos": ["acme/app"], + "jira": {"enabled": True, "base_url": "https://jira.acme.test", "project_keys": ["ABC"]}, +} diff --git a/tests/test_renderer_slack_tab.py b/tests/test_renderer_slack_tab.py new file mode 100644 index 0000000..399c6a7 --- /dev/null +++ b/tests/test_renderer_slack_tab.py @@ -0,0 +1,156 @@ +"""Tests for the dashboard's Slack tab (render_slack_focus_tab in +bin/render-assistant-page.py). HOME is a tmp dir set before the renderer +loads, so the real ~/.assistant is never read.""" +from __future__ import annotations + +import importlib.util +import json +import os +from pathlib import Path + +import pytest + +from assistant.slack_focus import store +from slack_focus_fixtures import CONFIG, write_config + +SCRIPT = Path(__file__).resolve().parent.parent / "bin/render-assistant-page.py" + + +@pytest.fixture +def home(tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + return tmp_path + + +@pytest.fixture +def mod(home): + spec = importlib.util.spec_from_file_location("renderer_slack_tab", str(SCRIPT)) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def item(text, lane="Do first", link="https://acme.slack.com/archives/C1/p1", thread_id=1, **extra): + return {"lane": lane, "text": text, "link": link, "thread_id": thread_id, "channel": "#team", + "who": "Tim Beauchamp", "last": "2026-10-18T09:30", **extra} + + +def seed(home, focus=None, threads=None, state=None, done=None): + root = store.base_dir(home) + if focus is not None: + store.write_json(root / "focus.json", focus) + store.write_json(root / "threads.json", {"threads": threads or [ + {"id": 1, "first": {"text": "a", "at": "1"}, "latest": []}, + {"id": 2, "first": {"text": "b", "at": "1"}, "latest": [{"text": "c", "at": "2"}]}]}) + if state is not None: + store.write_json(root / "state.json", state) + if done is not None: + store.write_json(root / "done.json", done) + + +def board_focus(**extra): + return { + "generated": "2026-10-20T14:03:00+00:00", "window_days": 7, "judge": {"ran": True}, + "active": [item("Answer ", mention=True, why="he asked", effort="Quick"), + item("Ship the plan", lane="Promise", thread_id=2, effort="Medium"), + item("Odd effort", lane="This week", effort="Huge", link="javascript:alert(1)")], + "read": [item("FYI build green", lane="Read")], + "judge_dropped": [{"text": "thanks", "channel": "#team", "who": "Aalia", "link": "https://x/2", + "why": "not something you need to act on"}], + "handled": [{"text": "PR 7 merged", "channel": "#team", "link": "https://x/3", + "why": "linked PR merged or closed"}], + **extra, + } + + +def test_setup_steps_before_configuration(mod): + html, count = mod.render_slack_focus_tab() + assert count is None + assert "Set up the Slack list" in html and "slack_focus" in html and "SLACK_USER_TOKEN" in html + assert 'id="sf-refresh"' in html and 'data-running="false"' in html + + +def test_not_checked_yet_once_configured(mod, home): + write_config(home, CONFIG | {"title": "Mukul's Slack"}) + html, count = mod.render_slack_focus_tab() + assert count is None and "Mukul's Slack" in html + assert "Not checked yet" in html and "Nothing pulled from Slack yet" in html + + +def test_board_renders_lanes_escaped_and_safe(mod, home): + write_config(home, CONFIG) + seed(home, board_focus()) + html, count = mod.render_slack_focus_tab() + assert count == 1 + assert "1 to do first" in html + assert "Answer <Tim>" in html and "" not in html + assert '@you' in html + assert "he asked Β· #team Β· Tim Beauchamp Β· 10-18 09:30" in html + assert 'class="sf-effort sf-quick">Quick' in html and 'class="sf-effort sf-medium">Medium' in html + assert "Huge" not in html + assert 'href="#"' in html and "javascript:" not in html + assert "Last pulled Oct 20, 14:03 UTC from the last 7 days. A model wrote the titles" in html + for label in ("Do first", "Promise", "This week", "Optional", "Done by you", "Just read", "Set aside", + "Auto-resolved"): + assert label in html, label + assert html.count('onclick="slackFocusMark(this)"') == 3 + assert html.count("Nothing here.") == 2 # Optional and Done by you + + +def test_checked_off_rows_move_to_done_by_you(mod, home): + write_config(home, CONFIG) + focus = board_focus() + fp = store.fingerprint(focus["active"][0], {1: {"first": {"text": "a", "at": "1"}, "latest": []}}) + seed(home, focus, done={fp: "2026-10-20T15:00:00+00:00"}) + html, count = mod.render_slack_focus_tab() + assert count == 0 + done = html[html.index('data-context-key="sf-done"'):] + assert "Answer <Tim>" in done[:done.index("")] + assert "checked off 10-20 15:00" in html and 'data-done="false"' in html and ">Undo<" in html + + +def test_running_and_failed_states(mod, home): + write_config(home, CONFIG) + seed(home, board_focus(judge={"ran": False}), + state={"running": True, "pid": os.getpid(), "phase": "Pulling Slack (11 of 40 threads)"}) + html, _ = mod.render_slack_focus_tab() + assert 'data-running="true"' in html and " disabled>Refreshing…" in html + assert "Refreshing: Pulling Slack (11 of 40 threads)." in html + seed(home, board_focus(judge={"ran": False}), state={"running": False, "ok": False, "error": "invalid "}) + html, _ = mod.render_slack_focus_tab() + assert "The last refresh didn't finish: invalid <auth>" in html + assert "These are the raw rows" in html + seed(home, board_focus(generated="garbage"), state={"running": False, "ok": False}) + html, _ = mod.render_slack_focus_tab() + assert "at an unknown time" in html and "unknown error" in html + seed(home, board_focus(), state={"running": True, "pid": os.getpid()}) + assert "Refreshing: Starting." in mod.render_slack_focus_tab()[0] + + +def test_tab_failure_degrades_to_one_message(mod, monkeypatch): + def broken(home): + raise ValueError("bad ") + + from assistant.slack_focus import board + + monkeypatch.setattr(board, "load", broken) + html, count = mod.render_slack_focus_tab() + assert count is None and "The Slack list is unavailable: bad <data>." in html + + +def test_slack_href_only_allows_plain_https(mod): + assert mod._slack_href("https://acme.slack.com/a?b=1&c=2") == "https://acme.slack.com/a?b=1&c=2" + for bad in ("http://acme.slack.com", "javascript:x", "https://", "https://a b", None, "https://[::1"): + assert mod._slack_href(bad) == "#", bad + + +def test_full_render_adds_the_slack_tab(mod, home): + (home / ".claude/cache").mkdir(parents=True) + (home / ".claude/cache/world.json").write_text(json.dumps({"_meta": {}, "counts": {}})) + write_config(home, CONFIG) + seed(home, board_focus()) + mod.render() + html = (home / ".claude/assistant-dashboard.html").read_text() + assert 'data-tab="slack"' in html and 'data-panel="slack"' in html + assert 'Slack 1' in html + assert "'slack', 'todos'" in html and "function slackFocusRefresh()" in html diff --git a/tests/test_slack_focus.py b/tests/test_slack_focus.py new file mode 100644 index 0000000..49edb7c --- /dev/null +++ b/tests/test_slack_focus.py @@ -0,0 +1,559 @@ +"""Tests for src/assistant/slack_focus β€” the Slack tab's pipeline, end to end +over a fake Slack workspace (tests/slack_focus_fixtures.py). No network, no +model, no gh: every outside call goes through an injected fake.""" +from __future__ import annotations + +import base64 +import io +import json +import os +import subprocess +import urllib.error +from datetime import datetime + +import pytest + +from assistant.slack_focus import board, client, collect, judge, settings, store, triage +from slack_focus_fixtures import CONFIG, NOW, FakeSlack, ts, write_config + + +@pytest.fixture +def cfg(): + return settings.parse(CONFIG) + + +@pytest.fixture +def slack(): + return client.Client("xoxp-test", FakeSlack()) + + +def gh_runner(states=None, merged=None, fail=False): + """A fake subprocess.run for gh: `pr view` answers from `states`, + `pr list` answers with `merged`.""" + calls = [] + + def run(cmd, **kwargs): + calls.append(cmd) + if fail: + raise OSError("no gh") + if cmd[1:3] == ["pr", "view"]: + state = (states or {}).get(cmd[3]) + if state is None: + return subprocess.CompletedProcess(cmd, 1, "", "not found") + return subprocess.CompletedProcess(cmd, 0, json.dumps({"state": state}) if state != "junk" else "{", "") + return subprocess.CompletedProcess(cmd, 0, json.dumps(merged or []), "") + + run.calls = calls + return run + + +def by_text(threads, text): + return next(t for t in threads if text in t["first"]["text"]) + + +# ─── settings ──────────────────────────────────────────────────────────────── + +def test_parse_reads_every_user_setting(): + s = settings.parse(CONFIG | {"title": "My board", "github_hosts": ["git.corp.example"]}) + assert s.configured and s.days == 7 and s.title == "My board" + assert s.watched_channels == {"C1": "team"} + assert s.bot_authors == ("Deploy Bot",) and s.skip_channels == ("deployments",) + assert s.github_repos == ("acme/app",) and s.github_hosts == ("git.corp.example",) + assert s.jira.enabled and s.jira.base_url == "https://jira.acme.test" and s.jira.project_keys == ("ABC",) + + +def test_parse_falls_back_per_field_on_bad_types(): + s = settings.parse({"days": "7", "title": " ", "watched_channels": ["C1"], "bot_authors": "bot", + "jira": "yes", "github_hosts": []}) + assert s.configured + assert (s.days, s.title, s.watched_channels, s.bot_authors) == (7, "Slack focus", {}, ()) + assert s.github_hosts == ("github.com",) and not s.jira.enabled + assert settings.parse({"days": True}).days == 7 and settings.parse({"days": 99}).days == 7 + assert settings.parse({"days": 3}).days == 3 + assert not settings.parse(None).configured + + +def test_load_reads_comms_config(tmp_path): + assert not settings.load(tmp_path).configured + write_config(tmp_path, CONFIG) + assert settings.load(tmp_path).watched_channels == {"C1": "team"} + (tmp_path / ".assistant/comms/config.json").write_text("[1]") + assert not settings.load(tmp_path).configured + (tmp_path / ".assistant/comms/config.json").write_text("{bad") + assert not settings.load(tmp_path).configured + + +# ─── client ────────────────────────────────────────────────────────────────── + +def test_client_refuses_any_non_read_method(slack): + with pytest.raises(client.SlackError, match="only reads Slack"): + slack.call("chat.postMessage", {"channel": "C1", "text": "hi"}) + assert "chat.postMessage" not in [m for m, _ in slack._transport.calls] + + +def test_client_needs_a_token_slack_accepts(): + with pytest.raises(client.SlackError, match="SLACK_USER_TOKEN"): + client.Client("", FakeSlack()) + with pytest.raises(client.SlackError, match="invalid_auth"): + client.Client("xoxp-bad", FakeSlack(auth={"ok": False, "error": "invalid_auth"})) + with pytest.raises(client.SlackError, match="unknown error"): + client.Client("xoxp-bad", FakeSlack(auth={"ok": False})) + + +def test_client_identity_and_helpers(slack): + assert (slack.me, slack.me_name, slack.workspace_url) == ("U1", "Mukul Sharma", "https://acme.slack.com") + assert slack.user_name("") == "" + assert slack.channel("D1") == {"name": "DM Aalia Habib", "kind": "dm", "last_read": float(ts(5))} + assert slack.channel("G1")["kind"] == "mpim" and slack.channel("C1")["name"] == "#team" + assert slack.mentions_me("hey <@U1>") == "direct" + assert slack.mentions_me(" look") == "group" + assert slack.mentions_me(" look") is None + assert slack.clean("<@U2> see :tada: now") == "@Aalia Habib see the doc now" + assert slack.clean(" hi") == "@group hi" + assert slack.permalink("C1", "1.000002", "1.000002") == "https://acme.slack.com/archives/C1/p1000002" + assert slack.permalink("C1", "2.0", "1.0").endswith("?thread_ts=1.0&cid=C1") + + +def test_client_raises_on_slack_errors_but_skips_gone_threads(): + answers = {"conversations.replies": [{"ok": False, "error": "thread_not_found"}], + "search.messages": [{"ok": False, "error": "ratelimited"}, {"ok": True, "messages": {}}], + "conversations.history": [{"ok": False, "error": "missing_scope"}]} + fake = FakeSlack() + + def transport(method, params, token): + queue = answers.get(method) + return queue.pop(0) if queue else fake(method, params, token) + + sleeps = [] + sl = client.Client("t", transport, sleep=sleeps.append) + answers["users.info"] = [{"ok": False}] + assert sl.call("conversations.replies", {})["error"] == "thread_not_found" + assert sl.call("search.messages", {}) == {"ok": True, "messages": {}} and sleeps == [30] + with pytest.raises(client.SlackError, match="conversations.history failed: missing_scope"): + sl.call("conversations.history", {}) + with pytest.raises(client.SlackError, match="users.info failed: unknown error"): + sl.call("users.info", {"user": "U7"}) + answers["search.messages"] = [{"ok": False, "error": "ratelimited"}] * 4 + with pytest.raises(client.SlackError, match="stayed rate limited"): + sl.call("search.messages", {}) + assert sleeps == [30] * 5 + + +class _Resp(io.BytesIO): + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + +def test_http_transport_retries_rate_limits_and_flaky_reads(monkeypatch): + sleeps = [] + monkeypatch.setattr(client.time, "sleep", sleeps.append) + answers = [urllib.error.HTTPError("u", 429, "slow", {"Retry-After": "2"}, None), + urllib.error.URLError("reset"), + _Resp(b'{"ok": true}')] + seen = [] + + def urlopen(req, timeout): + seen.append((req.full_url, req.get_header("Authorization"), req.data)) + answer = answers.pop(0) + if isinstance(answer, Exception): + raise answer + return answer + + monkeypatch.setattr(client.urllib.request, "urlopen", urlopen) + assert client.http_transport("auth.test", {"a": 1}, "xoxp-t") == {"ok": True} + assert sleeps == [3, 3] and seen[0] == ("https://slack.com/api/auth.test", "Bearer xoxp-t", b"a=1") + + +def test_http_transport_raises_other_http_errors_and_gives_up(monkeypatch): + monkeypatch.setattr(client.time, "sleep", lambda s: None) + + def forbidden(req, timeout): + raise urllib.error.HTTPError("u", 403, "no", {}, None) + + monkeypatch.setattr(client.urllib.request, "urlopen", forbidden) + with pytest.raises(urllib.error.HTTPError): + client.http_transport("auth.test", {}, "t") + + def down(req, timeout): + raise TimeoutError() + + monkeypatch.setattr(client.urllib.request, "urlopen", down) + with pytest.raises(client.SlackError, match="20 tries"): + client.http_transport("auth.test", {}, "t") + + +# ─── collect ───────────────────────────────────────────────────────────────── + +def test_collect_buckets_every_thread(slack, cfg): + progress = [] + doc, mine = collect.collect(slack, cfg, now=NOW, progress=lambda i, n: progress.append((i, n))) + threads = doc["threads"] + assert doc["me"] == "U1" and doc["me_name"] == "Mukul Sharma" and doc["days"] == 7 + assert [t["id"] for t in threads] == list(range(1, len(threads) + 1)) + assert [t["last_activity"] for t in threads] == sorted((t["last_activity"] for t in threads), reverse=True) + buckets = {t["first"]["text"][:20]: t["bucket"] for t in threads} + assert buckets == { + "FYI the build is gre": "unread_watched", + "thread root": "ball_mine_dm", + "Who owns the path bu": "ball_mine_mention", + "Deployed @Mukul Shar": "bot", + "Can you review PR 7?": "waiting_on_others", + "fyi all, slides are ": "ball_mine_dm", + "@group please look a": "group_mention", + "@Mukul Sharma ship i": "replied_elsewhere", + "Let's split the work": "thread_moved_on", + } + dm = next(t for t in threads if t["kind"] == "dm" and t["messages"] == 2 and t["first"]["text"] == "thread root" + and "Any small" in t["latest"][-1]["text"]) + assert dm["permalink"].startswith("https://acme.slack.com/archives/D1/") + assert not any(t["first"]["text"] == "old @Mukul Sharma" for t in threads) + assert progress and progress[0][0] == 1 + assert len(mine) == 6 and mine[0]["cid"] == "C1" and mine[-1]["dm"] + + +def test_bucket_rule_order(cfg): + base = {"last_author": "Aalia Habib", "channel": "#team", "last_is_me": False, "i_posted_after_in_channel": False, + "i_reacted_to_last": False, "kind": "channel", "mentioned_after_my_last_reply": None, + "i_participated": False, "from_others_after_my_last": 0, "unread": False, "watched": False} + assert collect.bucket(base, cfg) == "read_fyi" + assert collect.bucket(base | {"unread": True}, cfg) == "unread_other" + assert collect.bucket(base | {"channel": "#x-deployments"}, cfg) == "bot" + assert collect.bucket(base | {"i_posted_after_in_channel": True}, cfg) == "replied_elsewhere" + + +def test_my_later_post_in_the_thread_counts_as_replied(cfg): + threads = {("C1", ts(1)): [{"user": "U3", "ts": ts(1), "text": "<@U1> can you check?"}]} + search = {"<@U1>": [{"ts": ts(1), "text": "x", "permalink": "https://acme.slack.com/archives/C1/p1", + "channel": {"id": "C1"}}], + "from:me": [{"ts": ts(0.9), "text": "yes", "channel": {"id": "C1", "name": "team"}, + "permalink": f"https://acme.slack.com/archives/C1/p2?thread_ts={ts(1)}&cid=C1"}]} + sl = client.Client("t", FakeSlack(threads=threads, history={}, search=search)) + doc, _ = collect.collect(sl, settings.parse({}), now=NOW) + assert [t["bucket"] for t in doc["threads"]] == ["replied_elsewhere"] + + +def test_subscribed_thread_uses_its_own_read_marker(): + threads = {("C1", ts(1)): [{"user": "U3", "ts": ts(1), "text": "news", "subscribed": True, + "last_read": ts(0.5)}]} + search = {"<@U1>": [{"ts": ts(1), "text": "x", "permalink": "https://acme.slack.com/archives/C1/p1", + "channel": {"id": "C1"}}]} + sl = client.Client("t", FakeSlack(threads=threads, history={}, search=search)) + doc, _ = collect.collect(sl, settings.parse({"watched_channels": {"C1": "team"}}), now=NOW) + assert doc["threads"][0]["unread"] is False and doc["threads"][0]["bucket"] == "read_fyi" + + +def test_collect_defaults_to_the_current_time(cfg): + sl = client.Client("t", FakeSlack(threads={}, history={}, search={})) + doc, mine = collect.collect(sl, cfg) + assert doc["threads"] == [] and mine == [] + assert abs(datetime.fromisoformat(doc["generated"]).timestamp() - __import__("time").time()) < 60 + + +# ─── triage ────────────────────────────────────────────────────────────────── + +def _collected(slack, cfg): + return collect.collect(slack, cfg, now=NOW) + + +def test_triage_lanes_promises_and_resolution(slack, cfg): + doc, mine = _collected(slack, cfg) + resolver = triage.Resolver(cfg, run=gh_runner({"8": "MERGED", "9": "OPEN"}), env={}) + focus = triage.triage(doc, mine, cfg, resolver) + lanes = [(i["lane"], i["text"][:25]) for i in focus["active"]] + assert lanes[0][0] == "Do first" + assert ("Do first", "Any small features I can ") in lanes + assert ("Do first", "I think it's walnut") in lanes + assert ("This week", "fyi all, slides are in th") in lanes # a quiet group DM waits + assert ("This week", "anyone?") in lanes + assert ("Optional", "FYI the build is green ag") in lanes + promises = [i["text"] for i in focus["active"] if i["lane"] == "Promise"] + # Newest first; the second message about PR 9 is a repeat, and a question isn't a promise. + assert promises == ["I'll fix the importer in https://github.com/acme/app/pull/9 today", + "I'll send the freeze plan doc by friday"] + assert [h["why"] for h in focus["handled"]] == ["linked PR merged or closed"] + assert focus["handled"][0]["text"].startswith("Will merge") + assert focus["me_name"] == "Mukul Sharma" and focus["counts"]["threads"] == len(doc["threads"]) + order = [i["lane"] for i in focus["active"]] + assert order == sorted(order, key=triage.LANE_ORDER.index) + + +def test_triage_resolves_threads_by_pr_or_ticket(cfg): + def thread(text, bucket="ball_mine_mention"): + return {"id": 1, "bucket": bucket, "channel": "#team", "kind": "channel", "permalink": "https://a/1", + "last_author": "Tim", "mentioned_after_my_last_reply": "direct", "last_activity": "2026-10-01T00:00:00", + "first": {"author": "Tim", "text": text, "at": "x"}, "latest": []} + + resolver = triage.Resolver(cfg, run=gh_runner({"7": "CLOSED"}), env={"JIRA_API_TOKEN": "tok"}, + urlopen=lambda req, timeout: _Resp(json.dumps( + {"fields": {"status": {"statusCategory": {"key": "done"}}}}).encode())) + doc = {"days": 7, "me_name": "Mukul Sharma", "threads": [ + thread("see github.com/acme/app/pull/7 please"), + thread("ABC-12 is the ticket"), + thread("nothing linked here"), + thread("ignored lane", bucket="read_fyi")]} + focus = triage.triage(doc, [], cfg, resolver) + assert [h["why"] for h in focus["handled"]] == ["linked PR merged or closed", "linked ticket done"] + assert [i["text"] for i in focus["active"]] == ["nothing linked here"] + no_jira = settings.parse(CONFIG | {"jira": {"enabled": False, "project_keys": ["ABC"]}}) + focus = triage.triage(doc, [], no_jira, triage.Resolver(no_jira, run=gh_runner({"7": "OPEN"}), env={})) + assert focus["handled"] == [] and len(focus["active"]) == 3 + + +def test_promises_in_dms_name_the_conversation_kind(cfg): + mine = [{"cid": "D1", "ts": ts(1), "cname": "U0ABC", "dm": True, "permalink": "https://a/1", + "text": "I'll send the plan doc to you tomorrow"}, + {"cid": "G1", "ts": ts(2), "cname": "mpdm-a--b-1", "dm": True, "permalink": "https://a/2", + "text": "I'll share the review notes next week"}, + {"cid": "C1", "ts": ts(3), "cname": None, "dm": False, "permalink": "https://a/3", + "text": "I'll fix the flaky test in the pipeline"}] + focus = triage.triage({"days": 7, "me_name": "Mukul Sharma", "threads": []}, mine, cfg, + triage.Resolver(cfg, run=gh_runner(), env={})) + assert [i["channel"] for i in focus["active"]] == ["DM", "group DM", "#"] + + +def test_noise_hints_never_drop(): + me = "Mukul Sharma" + t = {"bucket": "ball_mine_mention", "first": {"text": "TL;DR of standup"}, "latest": []} + assert triage.noise(t, me) == "digest or standup post" + t = {"bucket": "thread_moved_on", "first": {"text": "x"}, "latest": [{"text": "ok then", "author": "Tim"}]} + assert triage.noise(t, me) == "discussion, no open question" + t = {"bucket": "ball_mine_dm", "first": {"text": "x"}, "latest": [{"text": "thanks", "author": "Tim"}]} + assert triage.noise(t, me) == "trailed off (short ack)" + t["latest"] = [{"text": "thanks", "author": me}] + assert triage.noise(t, me) is None + + +def test_regexes_use_configured_hosts_and_keys(): + assert triage.pr_regex(["git.corp.example"]).findall("https://git.corp.example/o/r/pull/3") == \ + [("git.corp.example", "o/r", "3")] + assert triage.jira_regex([]) is None + assert triage.jira_regex(["ABC", "XY"]).findall("ABC-1 and XY-22 not UTF-8") == ["ABC-1", "XY-22"] + + +def test_resolver_pr_state_cache_and_failures(cfg): + run = gh_runner({"1": "MERGED", "2": "OPEN", "3": "junk"}) + r = triage.Resolver(cfg, run=run, env={}) + assert r.pr_closed("github.com", "o/r", "1") and r.pr_closed("github.com", "o/r", "1") + assert len(run.calls) == 1 and run.calls[0][-3:] == ["o/r", "--json", "state"] + assert not r.pr_closed("github.com", "o/r", "2") + assert not r.pr_closed("github.com", "o/r", "3") + assert not r.pr_closed("git.corp.example", "o/r", "4") + assert run.calls[-1][5] == "git.corp.example/o/r" + assert triage.Resolver(cfg, run=gh_runner(fail=True), env={}).gh(["x"]) is None + + +def test_resolver_jira_auth_and_failures(cfg, monkeypatch): + seen = [] + + def urlopen(req, timeout): + seen.append((req.full_url, req.get_header("Authorization"))) + if "BAD" in req.full_url: + raise urllib.error.URLError("down") + return _Resp(json.dumps({"fields": {"status": {"statusCategory": {"key": "indeterminate"}}}}).encode()) + + r = triage.Resolver(cfg, env={"JIRA_API_TOKEN": "tok", "JIRA_EMAIL": "me@acme.test"}, urlopen=urlopen) + assert not r.jira_done("ABC-1") and not r.jira_done("ABC-1") and len(seen) == 1 + assert seen[0] == ("https://jira.acme.test/rest/api/2/issue/ABC-1?fields=status", + "Basic " + base64.b64encode(b"me@acme.test:tok").decode()) + assert not r.jira_done("BAD-1") + env_base = settings.parse(CONFIG | {"jira": {"enabled": True, "project_keys": ["ABC"]}}) + r = triage.Resolver(env_base, env={"JIRA_BASE_URL": "https://env.acme.test/", "JIRA_API_TOKEN": "t"}, + urlopen=urlopen) + r.jira_done("ABC-2") + assert seen[-1] == ("https://env.acme.test/rest/api/2/issue/ABC-2?fields=status", "Bearer t") + assert not triage.Resolver(cfg, env={}, urlopen=urlopen).jira_done("ABC-3") # no token: no request + assert len(seen) == 3 + + +# ─── judge ─────────────────────────────────────────────────────────────────── + +def _focus_and_threads(slack, cfg): + doc, mine = _collected(slack, cfg) + focus = triage.triage(doc, mine, cfg, triage.Resolver(cfg, run=gh_runner({"8": "OPEN", "9": "OPEN"}), env={})) + return focus, {t["id"]: t for t in doc["threads"]} + + +def _answer(focus, lanes): + """A model answer: keep candidate i with lanes[i]; drop the rest.""" + return json.dumps([{"id": i, "lane": lane, "title": f"title {i}", "why": "because", "effort": "Quick"} + for i, lane in lanes.items()]) + + +def test_judge_applies_lanes_and_caches(slack, cfg): + focus, threads = _focus_and_threads(slack, cfg) + n = len(focus["active"]) + prompts = [] + + def model(prompt): + prompts.append(prompt) + return "Here you go:\n" + _answer(focus, {0: "Do first", 1: "Read", 2: "Done", 3: "This week", 4: "weird"}) + + judged, cache = judge.judge(focus, threads, ["app#5 Fix importer"], None, model) + assert "Mukul Sharma" in prompts[0] and "app#5 Fix importer" in prompts[0] + assert sorted(i["text"] for i in judged["active"]) == ["title 0", "title 3", "title 4"] + kept_weird = next(i for i in judged["active"] if i["text"] == "title 4") + assert kept_weird["lane"] == focus["active"][4]["lane"] # an unknown lane keeps triage's + assert [i["text"] for i in judged["read"]] == ["title 1"] + assert judged["handled"][-1] == {"text": "title 2", "channel": focus["active"][2]["channel"], + "link": focus["active"][2]["link"], "why": "because"} + assert len(judged["judge_dropped"]) == n - 5 + assert judged["judge"] == {"ran": True, "respond": 3, "read": 1, "dropped": n - 5, "cached": 0, "rejudged": n} + assert focus["active"][0]["text"] != "title 0" # the input isn't mutated + + again, cache2 = judge.judge(focus, threads, ["app#5 Fix importer"], cache, lambda p: pytest.fail("re-judged")) + assert again["judge"]["cached"] == n and again["active"] == judged["active"] + assert cache2 == cache + + calls = [] + judge.judge(focus, threads, ["app#6 other"], cache, lambda p: calls.append(p) or _answer(focus, {0: "Do first"})) + assert calls # a new merged PR invalidates every cached verdict + + +def test_judge_keeps_raw_rows_when_the_model_fails(slack, cfg): + focus, threads = _focus_and_threads(slack, cfg) + for answer in ("", "no json here", "[{bad json]", "[]"): + judged, cache = judge.judge(focus, threads, [], {"keep": 1}, lambda p: answer) + assert judged["judge"]["ran"] is False and judged["active"] == focus["active"] + assert cache == {"keep": 1} + judged, cache = judge.judge(focus, threads, [], None, lambda p: "") + assert cache == {} + + +def test_judge_effort_and_missing_fields(slack, cfg): + focus, threads = _focus_and_threads(slack, cfg) + answer = json.dumps([{"id": 0, "lane": "Do first", "title": "t", "effort": "Huge"}, {"id": 1}, "junk"]) + judged, _ = judge.judge(focus, threads, [], None, lambda p: answer) + assert judged["active"][0]["effort"] == "" and judged["active"][0]["why"] == "" + + +def test_usable_cache_rejects_mismatches(): + sig = "s" + good = {"done_evidence": ["a"], "rubric": sig, "verdicts": {"x": "DROP", "y": {"lane": "Read"}, "z": 3}} + assert judge.usable_cache(good, ["a"], sig) == {"x": "DROP", "y": {"lane": "Read"}} + assert judge.usable_cache(good, ["b"], sig) == {} + assert judge.usable_cache(good, ["a"], "other") == {} + assert judge.usable_cache(good | {"verdicts": []}, ["a"], sig) == {} + assert judge.usable_cache([], ["a"], sig) == {} + + +def test_done_evidence_lists_merged_prs(cfg): + run = gh_runner(merged=[{"number": 5, "title": "Fix importer"}]) + r = triage.Resolver(cfg, run=run, env={}) + assert judge.done_evidence(cfg, r, 7, datetime(2026, 10, 20)) == ["app#5 Fix importer"] + assert "merged:>=2026-10-11" in run.calls[0] + bad = triage.Resolver(cfg, run=lambda cmd, **k: subprocess.CompletedProcess(cmd, 0, "{oops", ""), env={}) + assert judge.done_evidence(cfg, bad, 7, datetime(2026, 10, 20)) == [] + failed = triage.Resolver(cfg, run=lambda cmd, **k: subprocess.CompletedProcess(cmd, 1, "", "err"), env={}) + assert judge.done_evidence(cfg, failed, 7, datetime(2026, 10, 20)) == [] + + +# ─── store ─────────────────────────────────────────────────────────────────── + +def test_fingerprint_changes_with_a_new_reply(): + threads = {1: {"first": {"text": "a", "at": "1"}, "latest": [{"text": "b", "at": "2"}]}, + 2: {"first": {"text": "a", "at": "1"}, "latest": []}} + item = {"link": "https://x/1", "thread_id": 1} + fp = store.fingerprint(item, threads) + assert len(fp) == 64 and fp == store.fingerprint(item, threads) + threads[1]["latest"].append({"text": "c", "at": "3"}) + assert store.fingerprint(item, threads) != fp + assert store.fingerprint({"link": "https://x/1", "thread_id": 2}, threads) != \ + store.fingerprint({"link": "https://x/1"}, threads) + + +def test_done_marks_and_json_io(tmp_path): + store.set_done(tmp_path, "a" * 64, True) + store.set_done(tmp_path, "b" * 64, True) + store.set_done(tmp_path, "a" * 64, False) + marks = store.read_json(store.base_dir(tmp_path) / "done.json") + assert list(marks) == ["b" * 64] + (store.base_dir(tmp_path) / "done.json").write_text("[]") + store.set_done(tmp_path, "c" * 64, True) + assert list(store.read_json(store.base_dir(tmp_path) / "done.json")) == ["c" * 64] + assert store.read_json(tmp_path / "missing.json", "d") == "d" + assert [p.name for p in store.base_dir(tmp_path).iterdir() if p.name.startswith(".")] == [] + + +def test_write_json_leaves_no_temp_file_on_failure(tmp_path): + with pytest.raises(TypeError): + store.write_json(tmp_path / "x.json", {"bad": object()}) + assert list(tmp_path.iterdir()) == [] + + +def test_lock_is_exclusive(tmp_path): + path = tmp_path / "x.lock" + with store.locked(path) as first: + assert first + with store.locked(path, blocking=False) as second: + assert second is False + with store.locked(path, blocking=False) as again: + assert again + + +def test_state_reports_a_dead_run_as_failed(tmp_path): + assert store.read_state(tmp_path) == {} + store.write_state(tmp_path, running=True, pid=os.getpid(), phase="Pulling Slack") + assert store.read_state(tmp_path)["running"] is True + store.write_state(tmp_path, pid=2 ** 22 + 12345) + state = store.read_state(tmp_path) + assert (state["running"], state["ok"], state["phase"]) == (False, False, None) + assert "stopped before it finished" in state["error"] + (store.base_dir(tmp_path) / "state.json").write_text("[]") + assert store.write_state(tmp_path, ok=True) == {"ok": True} + (store.base_dir(tmp_path) / "state.json").write_text("[]") + assert store.read_state(tmp_path) == {} + + +def test_pid_alive(monkeypatch): + assert store.pid_alive(os.getpid()) + assert not store.pid_alive(None) and not store.pid_alive(0) and not store.pid_alive("12") + + def denied(pid, sig): + raise PermissionError() + + monkeypatch.setattr(store.os, "kill", denied) + assert store.pid_alive(1) + + +# ─── board ─────────────────────────────────────────────────────────────────── + +def test_board_splits_open_and_checked_off(tmp_path, slack, cfg): + write_config(tmp_path, CONFIG | {"title": "Mine"}) + view = board.load(tmp_path) + assert view["configured"] and view["focus"] is None and view["title"] == "Mine" + focus, threads = _focus_and_threads(slack, cfg) + root = store.base_dir(tmp_path) + store.write_json(root / "focus.json", focus) + store.write_json(root / "threads.json", {"threads": list(threads.values())}) + first = focus["active"][0] + store.set_done(tmp_path, store.fingerprint(first, threads), True) + view = board.load(tmp_path) + assert [d["text"] for d in view["done_by_you"]] == [first["text"]] and view["done_by_you"][0]["done_at"] + names = [name for name, *_ in view["lanes"]] + assert names == ["Do first", "Promise", "This week", "Optional"] + assert view["do_first"] == sum(i["lane"] == "Do first" for i in focus["active"]) - 1 + assert view["judged"] is False and view["handled"] == focus["handled"] + + +def test_board_ignores_malformed_files(tmp_path): + root = store.base_dir(tmp_path) + store.write_json(root / "focus.json", {"active": "nope"}) + assert board.load(tmp_path)["focus"] is None + store.write_json(root / "focus.json", {"active": [{"lane": "Do first", "link": "https://x", "thread_id": 1}]}) + store.write_json(root / "threads.json", []) + store.write_json(root / "done.json", []) + view = board.load(tmp_path) + assert view["do_first"] == 1 and view["done_by_you"] == [] and not view["configured"] + + +def test_package_never_names_a_write_method(): + """The only Slack calls are reads; no module even names a write method.""" + import pathlib + + src = pathlib.Path(client.__file__).parent + text = "".join(p.read_text() for p in src.glob("*.py")) + for method in ('"chat.', '"reactions.', '"conversations.mark', '"files.', '"pins.'): + assert method not in text diff --git a/tests/test_slack_focus_cli.py b/tests/test_slack_focus_cli.py new file mode 100644 index 0000000..3b177ca --- /dev/null +++ b/tests/test_slack_focus_cli.py @@ -0,0 +1,172 @@ +"""Tests for bin/slack-focus.py β€” the Slack tab's refresh runner. The fake +Slack workspace, a fake gh, and a fake model stand in for every outside call; +HOME is a tmp dir, so ~/.assistant is never touched.""" +from __future__ import annotations + +import importlib.util +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from assistant.slack_focus import store, triage +from slack_focus_fixtures import CONFIG, FakeSlack, write_config + +BIN = Path(__file__).resolve().parent.parent / "bin" + + +def _load(): + spec = importlib.util.spec_from_file_location("slack_focus_cli", str(BIN / "slack-focus.py")) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +cli = _load() + + +@pytest.fixture +def home(tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setattr(cli, "rerender", lambda: None) + write_config(tmp_path, CONFIG) + return tmp_path + + +def offline_resolver(home): + def run(cmd, **kwargs): + if cmd[1:3] == ["pr", "list"]: + return subprocess.CompletedProcess(cmd, 0, "[]", "") + return subprocess.CompletedProcess(cmd, 0, json.dumps({"state": "OPEN"}), "") + + from assistant.slack_focus import settings + + return triage.Resolver(settings.load(home), run=run, env={}) + + +def model_keep_first(prompt): + return json.dumps([{"id": 0, "lane": "Do first", "title": "Answer Aalia", "why": "she asked", "effort": "Quick"}]) + + +def test_full_refresh_writes_every_file(home): + focus = cli.refresh(home, pull=True, use_judge=True, token="xoxp-t", transport=FakeSlack(), + model=model_keep_first, resolver=offline_resolver(home)) + root = store.base_dir(home) + for name in ("threads.json", "mine.json", "focus.json", "judge-cache.json"): + assert (root / name).exists(), name + assert [i["text"] for i in focus["active"]] == ["Answer Aalia"] + assert store.read_json(root / "focus.json") == focus + assert store.read_state(home)["phase"] == "Asking the model which threads need you" + + +def test_triage_only_refresh_reuses_the_last_pull(home): + cli.refresh(home, pull=True, use_judge=False, token="t", transport=FakeSlack(), resolver=offline_resolver(home)) + calls = FakeSlack() + focus = cli.refresh(home, pull=False, use_judge=False, token="", transport=calls, + resolver=offline_resolver(home)) + assert calls.calls == [] and "judge" not in focus and focus["active"] + + +def test_refresh_explains_missing_setup(home): + (home / ".assistant/comms/config.json").write_text("{}") + with pytest.raises(RuntimeError, match="slack_focus block"): + cli.refresh(home, pull=True, use_judge=False, token="t") + write_config(home, CONFIG) + with pytest.raises(RuntimeError, match="no Slack pull"): + cli.refresh(home, pull=False, use_judge=False, token="t") + + +def test_main_records_success_and_failure(home, monkeypatch): + renders = [] + monkeypatch.setattr(cli, "rerender", lambda: renders.append(store.read_state(home)["running"])) + monkeypatch.setattr(cli, "refresh", lambda h, **kw: {"counts": {"active": 1}, "kw": kw}) + assert cli.main([]) == 0 + state = store.read_state(home) + assert (state["running"], state["ok"], state["error"], state["phase"]) == (False, True, None, None) + assert state["pid"] == os.getpid() and state["finished"] and renders == [False] + + def boom(h, **kw): + raise cli.slack_client.SlackError("Slack refused the token: invalid_auth") + + monkeypatch.setattr(cli, "refresh", boom) + assert cli.main(["--triage", "--no-judge"]) == 1 + state = store.read_state(home) + assert (state["ok"], state["error"]) == (False, "Slack refused the token: invalid_auth") + + monkeypatch.setattr(cli, "refresh", lambda h, **kw: (_ for _ in ()).throw(KeyError())) + assert cli.main([]) == 1 and store.read_state(home)["error"] == "KeyError" + + +def test_main_passes_flags_and_token(home, monkeypatch): + seen = {} + monkeypatch.setenv("SLACK_USER_TOKEN", "xoxp-env") + monkeypatch.setattr(cli, "refresh", lambda h, **kw: seen.update(kw) or {"counts": {}}) + cli.main(["--triage", "--no-judge"]) + assert seen == {"pull": False, "use_judge": False, "token": "xoxp-env"} + + +def test_main_skips_while_another_refresh_runs(home, capsys): + with store.locked(store.base_dir(home) / "refresh.lock"): + assert cli.main([]) == 0 + assert "already running" in capsys.readouterr().err + assert store.read_state(home) == {} + + +def test_call_model_runs_tools_off_and_meters(home, monkeypatch): + seen = {} + + def fake_run(cmd, *, input_text=None, timeout=30, merge_bedrock=False): + seen.update(cmd=cmd, prompt=input_text, timeout=timeout) + return 0, json.dumps({"type": "result", "subtype": "success", "result": "[1]", + "usage": {"input_tokens": 10, "output_tokens": 2}, "total_cost_usd": 0.01}), "" + + monkeypatch.setattr(cli, "run", fake_run) + assert cli.call_model("judge these") == "[1]" + assert "--tools" in seen["cmd"] and "--strict-mcp-config" in seen["cmd"] + assert seen["prompt"] == "judge these" and seen["timeout"] == cli.JUDGE_TIMEOUT_SEC + row = json.loads((home / ".assistant/cost-ledger.jsonl").read_text().splitlines()[-1]) + assert (row["caller"], row["status"], row["est_usd"]) == ("slack_focus", "ok", 0.01) + + +def test_call_model_failure_returns_nothing(home, monkeypatch, capsys): + monkeypatch.setattr(cli, "run", lambda cmd, **kw: (1, "", "auth failed")) + assert cli.call_model("x") == "" + assert "rc=1" in capsys.readouterr().err + row = json.loads((home / ".assistant/cost-ledger.jsonl").read_text().splitlines()[-1]) + assert row["status"] == "failed" + + def broken_meter(**kw): + raise OSError("disk full") + + monkeypatch.setattr(cli.metering, "append_cost_row", broken_meter) + assert cli.call_model("x") == "" + assert "metering failed" in capsys.readouterr().err + + +def test_run_returns_output_and_kills_on_timeout(home, tmp_path): + (home / ".zprofile").write_text('export AWS_REGION="us-west-2"\n') + rc, out, err = cli.run([sys.executable, "-c", "import os,sys; print(os.environ.get('AWS_REGION'), sys.stdin.read())"], + input_text="hi", merge_bedrock=True) + assert (rc, out.strip()) == (0, "us-west-2 hi") + rc, out, err = cli.run([sys.executable, "-c", "import time; time.sleep(30)"], timeout=1) + assert rc == 124 and "timeout after 1s" in err + assert cli.run([str(tmp_path / "missing-binary")])[0] == 127 + + +def test_run_falls_back_to_kill_when_the_group_is_gone(home, monkeypatch): + def no_group(pid): + raise ProcessLookupError() + + monkeypatch.setattr(cli.os, "getpgid", no_group) + assert cli.run([sys.executable, "-c", "import time; time.sleep(30)"], timeout=1)[0] == 124 + + +def test_rerender_runs_the_dashboard_renderer(monkeypatch): + calls = [] + monkeypatch.setattr(cli.subprocess, "run", lambda cmd, **kw: calls.append((cmd, kw))) + cli.rerender() + assert calls[0][0] == [sys.executable, str(BIN / "render-assistant-page.py")] + assert calls[0][1]["check"] is False diff --git a/tests/test_todo_server.py b/tests/test_todo_server.py index 200e290..8354474 100644 --- a/tests/test_todo_server.py +++ b/tests/test_todo_server.py @@ -11,6 +11,7 @@ import importlib.util import json +import os import sys import threading import urllib.error @@ -763,3 +764,87 @@ def test_read_body_reads_n_bytes(): def test_read_body_zero_length(): h = _make_handler("0", b"hello") assert h._read_body() == "" + + +# ─── Slack tab routes ───────────────────────────────────────────────────────── + + +class _FakeProc: + def __init__(self, cmd, **kwargs): + self.cmd, self.kwargs, self.pid, self.exit = cmd, kwargs, 424242, None + _FakeProc.made.append(self) + + def poll(self): + return self.exit + + +@pytest.fixture +def slack_home(env, monkeypatch): + _FakeProc.made = [] + monkeypatch.setattr(mod, "HOME", env.tmp) + monkeypatch.setattr(mod, "_slack_focus_proc", None) + monkeypatch.setattr(mod.subprocess, "Popen", _FakeProc) + from assistant.slack_focus import store + return store + + +def test_slack_focus_refresh_starts_once_and_reports_state(server, slack_home): + store = slack_home + assert _request(server.base, "POST", "/slack-focus/status") == (200, "{}") + status, body = _request(server.base, "POST", "/slack-focus/refresh") + assert (status, json.loads(body)) == (200, {"started": True}) + proc = _FakeProc.made[0] + assert proc.cmd[:2] == ["/bin/zsh", "-c"] and ". \"$HOME/.zprofile\"" in proc.cmd[2] + assert proc.cmd[3:] == [sys.executable, str(mod.SLACK_FOCUS_SCRIPT)] + assert proc.kwargs["start_new_session"] is True + assert (store.base_dir(server.env.tmp) / "refresh.log").exists() + state = json.loads(_request(server.base, "POST", "/slack-focus/status")[1]) + assert (state["running"], state["phase"], state["pid"]) == (True, "Starting", os.getpid()) + # A second click while it runs starts nothing. + assert json.loads(_request(server.base, "POST", "/slack-focus/refresh")[1]) == {"started": False} + assert len(_FakeProc.made) == 1 + + +def test_slack_focus_refresh_that_never_starts_is_reported(server, slack_home): + _request(server.base, "POST", "/slack-focus/refresh") + _FakeProc.made[0].exit = 1 + state = json.loads(_request(server.base, "POST", "/slack-focus/status")[1]) + assert (state["running"], state["ok"]) == (False, False) + assert "didn't start" in state["error"] + + +def test_slack_focus_finished_refresh_keeps_its_own_state(server, slack_home): + store = slack_home + _request(server.base, "POST", "/slack-focus/refresh") + store.write_state(server.env.tmp, running=False, ok=True, pid=_FakeProc.made[0].pid) + _FakeProc.made[0].exit = 0 + state = json.loads(_request(server.base, "POST", "/slack-focus/status")[1]) + assert (state["running"], state["ok"]) == (False, True) + assert json.loads(_request(server.base, "POST", "/slack-focus/refresh")[1]) == {"started": True} + + +def test_slack_focus_refresh_skips_when_a_run_holds_the_lock(server, slack_home): + store = slack_home + with store.locked(store.base_dir(server.env.tmp) / "refresh.lock"): + assert json.loads(_request(server.base, "POST", "/slack-focus/refresh")[1]) == {"started": False} + assert _FakeProc.made == [] + + +def test_slack_focus_mutations_refuse_foreign_origins(server, slack_home): + for path in ("/slack-focus/refresh", "/slack-focus/done", "/slack-focus/undone"): + status, _ = _request(server.base, "POST", path, headers={"Origin": "https://evil.example"}) + assert status == 403, path + assert _FakeProc.made == [] + + +def test_slack_focus_done_and_undone(server, slack_home): + store = slack_home + fp = "a" * 64 + status, body = _request(server.base, "POST", "/slack-focus/done", json.dumps({"fp": fp})) + assert (status, body) == (200, "marked done") + assert list(store.read_json(store.base_dir(server.env.tmp) / "done.json")) == [fp] + status, body = _request(server.base, "POST", "/slack-focus/undone", json.dumps({"fp": fp})) + assert (status, body) == (200, "marked not done") + assert store.read_json(store.base_dir(server.env.tmp) / "done.json") == {} + assert _request(server.base, "POST", "/slack-focus/done", json.dumps({"fp": "../x"}))[0] == 400 + assert _request(server.base, "POST", "/slack-focus/done", "{bad")[0] == 400 From d58524b0d6a426e2a01521802b4f5379503cf772 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Tue, 20 Oct 2026 21:41:52 -0700 Subject: [PATCH 2/7] Slack tab: key check-offs by each row's own signature, harden refresh state Co-Authored-By: Claude Opus 5.5 --- bin/slack-focus.py | 13 +++- bin/todo-server.py | 18 +++-- src/assistant/slack_focus/board.py | 5 +- src/assistant/slack_focus/judge.py | 8 ++- src/assistant/slack_focus/settings.py | 4 +- src/assistant/slack_focus/store.py | 46 +++++++----- src/assistant/slack_focus/triage.py | 5 +- tests/drive_dashboard_overview.py | 51 +++++++------ tests/test_renderer_slack_tab.py | 10 +-- tests/test_slack_focus.py | 100 +++++++++++++++++--------- tests/test_slack_focus_cli.py | 33 +++++++++ tests/test_todo_server.py | 26 +++++++ 12 files changed, 223 insertions(+), 96 deletions(-) diff --git a/bin/slack-focus.py b/bin/slack-focus.py index 99cc302..65a40aa 100755 --- a/bin/slack-focus.py +++ b/bin/slack-focus.py @@ -27,9 +27,7 @@ REPO = Path(__file__).resolve().parent.parent BIN = REPO / "bin" SRC = REPO / "src" -for path in (SRC, BIN): - if str(path) not in sys.path: - sys.path.insert(0, str(path)) +sys.path[:0] = [str(SRC), str(BIN)] import comms_lib # noqa: E402 import llm_runner # noqa: E402 @@ -40,6 +38,7 @@ RENDER_SCRIPT = BIN / "render-assistant-page.py" JUDGE_TIMEOUT_SEC = int(os.environ.get("SLACK_FOCUS_JUDGE_TIMEOUT_SEC", "420")) +REFRESH_TIMEOUT_SEC = 1200 CLAUDE_BIN = os.environ.get("CLAUDE_BIN", str(Path.home() / ".local/bin/claude")) @@ -138,6 +137,10 @@ def refresh(home: Path, *, pull: bool, use_judge: bool, token: str, return focus +def _too_slow(signum, frame): + raise TimeoutError(f"The refresh took longer than {REFRESH_TIMEOUT_SEC // 60} minutes.") + + def main(argv: list[str] | None = None) -> int: ap = argparse.ArgumentParser(description="Refresh the Slack focus board.") ap.add_argument("--triage", action="store_true", help="re-triage the last pull without calling Slack") @@ -153,6 +156,8 @@ def main(argv: list[str] | None = None) -> int: ok=None, error=None, phase="Starting") started = time.time() error = None + signal.signal(signal.SIGALRM, _too_slow) + signal.alarm(REFRESH_TIMEOUT_SEC) try: focus = refresh(home, pull=not args.triage, use_judge=not args.no_judge, token=os.environ.get("SLACK_USER_TOKEN", "")) @@ -160,6 +165,8 @@ def main(argv: list[str] | None = None) -> int: except Exception as e: # noqa: BLE001 β€” every run must record how it ended error = str(e)[:300] or type(e).__name__ print(f"refresh failed: {error}", file=sys.stderr) + finally: + signal.alarm(0) store.write_state(home, running=False, finished=store.now_iso(), ok=error is None, error=error, phase=None, seconds=round(time.time() - started)) rerender() diff --git a/bin/todo-server.py b/bin/todo-server.py index fe3e049..cf6e25c 100755 --- a/bin/todo-server.py +++ b/bin/todo-server.py @@ -569,10 +569,15 @@ def slack_focus_refresh(): return json.dumps({"started": False}) store.write_state(HOME, running=True, pid=os.getpid(), started=store.now_iso(), finished=None, ok=None, error=None, phase="Starting") - with open(root / "refresh.log", "w") as log: - _slack_focus_proc = subprocess.Popen( - ["/bin/zsh", "-c", SLACK_FOCUS_LAUNCH, sys.executable, str(SLACK_FOCUS_SCRIPT)], - stdin=subprocess.DEVNULL, stdout=log, stderr=subprocess.STDOUT, start_new_session=True) + try: + with open(root / "refresh.log", "w") as log: + _slack_focus_proc = subprocess.Popen( + ["/bin/zsh", "-c", SLACK_FOCUS_LAUNCH, sys.executable, str(SLACK_FOCUS_SCRIPT)], + stdin=subprocess.DEVNULL, stdout=log, stderr=subprocess.STDOUT, start_new_session=True) + except OSError as e: + store.write_state(HOME, running=False, ok=False, phase=None, finished=store.now_iso(), + error=f"The refresh couldn't start: {e}") + return json.dumps({"started": False}) return json.dumps({"started": True}) @@ -759,6 +764,11 @@ def do_POST(self): if _mutation and not self._origin_allowed_for_mutation(): self._reply(403, "cross-origin mutation refused") return + # The Slack tab's routes reach Slack, gh, and the model, so they also + # refuse Origin: null (a sandboxed frame); the tab never sends it. + if parts[:1] == ["slack-focus"] and self.headers.get("Origin", "") == "null": + self._reply(403, "cross-origin request refused") + return # POST /proposal// if len(parts) == 3 and parts[0] == "proposal": diff --git a/src/assistant/slack_focus/board.py b/src/assistant/slack_focus/board.py index 8ab8630..83fec88 100644 --- a/src/assistant/slack_focus/board.py +++ b/src/assistant/slack_focus/board.py @@ -27,14 +27,11 @@ def load(home: Path | None = None) -> dict: } if focus is None: return view - threads_doc = store.read_json(root / "threads.json", {}) - threads = {t["id"]: t for t in (threads_doc.get("threads") or []) if isinstance(t, dict) and "id" in t} \ - if isinstance(threads_doc, dict) else {} marks = store.read_json(root / "done.json", {}) marks = marks if isinstance(marks, dict) else {} open_items, done_items = [], [] for item in focus["active"]: - item = dict(item, fp=store.fingerprint(item, threads)) + item = dict(item, fp=store.fingerprint(item)) if item["fp"] in marks: done_items.append(dict(item, done_at=marks[item["fp"]])) else: diff --git a/src/assistant/slack_focus/judge.py b/src/assistant/slack_focus/judge.py index 21b8a55..b9d587b 100644 --- a/src/assistant/slack_focus/judge.py +++ b/src/assistant/slack_focus/judge.py @@ -95,7 +95,8 @@ def parse(raw: str) -> dict[int, dict] | None: return None try: data = json.loads(m.group(0)) - return {int(d["id"]): d for d in data if isinstance(d, dict) and "id" in d and d.get("title")} + return {int(d["id"]): d for d in data + if isinstance(d, dict) and "id" in d and isinstance(d.get("title"), str) and d["title"].strip()} except (ValueError, TypeError): return None @@ -142,11 +143,12 @@ def judge(focus: dict, threads: dict[int, dict], evidence: list[str], saved_cach continue item = dict(item) lane = v.get("lane") if v.get("lane") in LANES else item["lane"] - item.update(lane=lane, text=v["title"], why=v.get("why", ""), + why = v.get("why") + item.update(lane=lane, text=v["title"], why=why if isinstance(why, str) else "", effort=v["effort"] if v.get("effort") in EFFORTS else "") if lane == "Done": focus["handled"].append({"text": item["text"], "channel": item["channel"], "link": item["link"], - "why": v.get("why", "already done")}) + "why": why if isinstance(why, str) else "already done"}) else: (read if lane == "Read" else kept).append(item) kept.sort(key=lambda i: (LANE_ORDER.index(i["lane"]) if i["lane"] in LANE_ORDER else 9, -i.get("rank", 0))) diff --git a/src/assistant/slack_focus/settings.py b/src/assistant/slack_focus/settings.py index e809f5a..a74b3d6 100644 --- a/src/assistant/slack_focus/settings.py +++ b/src/assistant/slack_focus/settings.py @@ -20,7 +20,7 @@ class JiraSettings: enabled: bool = False base_url: str = "" token_env: str = "JIRA_API_TOKEN" - email_env: str = "JIRA_EMAIL" + email_env: str = "" project_keys: tuple[str, ...] = () @@ -69,7 +69,7 @@ def parse(block) -> Settings: enabled=jira.get("enabled") is True, base_url=str(jira.get("base_url") or "").rstrip("/"), token_env=str(jira.get("token_env") or "JIRA_API_TOKEN"), - email_env=str(jira.get("email_env") or "JIRA_EMAIL"), + email_env=str(jira.get("email_env") or ""), project_keys=tuple(k.upper() for k in _strings(jira.get("project_keys"))), ), ) diff --git a/src/assistant/slack_focus/store.py b/src/assistant/slack_focus/store.py index 59f6957..abb9eaf 100644 --- a/src/assistant/slack_focus/store.py +++ b/src/assistant/slack_focus/store.py @@ -18,6 +18,7 @@ import json import os import tempfile +import time from contextlib import contextmanager from datetime import datetime, timezone from pathlib import Path @@ -66,13 +67,19 @@ def locked(path: Path, blocking: bool = True): fcntl.flock(f.fileno(), fcntl.LOCK_UN) -def fingerprint(item: dict, threads: dict[int, dict]) -> str: +def signature(thread: dict | None) -> str: + """The thread's latest message as "text|time". Triage stores it on each + row, so a row's key never depends on a later pull's thread numbering.""" + if not thread: + return "" + last = (thread.get("latest") or [thread["first"]])[-1] + return last.get("text", "") + "|" + last.get("at", "") + + +def fingerprint(item: dict) -> str: """Key a checked-off item by its link plus its thread's latest message. A new reply or an edit changes the key, so the item comes back on its own.""" - t = threads.get(item.get("thread_id")) - last = (t.get("latest") or [t["first"]])[-1] if t else {} - signature = (last.get("text", "") + "|" + last.get("at", "")) if t else "" - return hashlib.sha256((item["link"] + "||" + signature).encode()).hexdigest() + return hashlib.sha256((item["link"] + "||" + item.get("sig", "")).encode()).hexdigest() def set_done(home: Path | None, fp: str, done: bool) -> None: @@ -87,23 +94,24 @@ def set_done(home: Path | None, fp: str, done: bool) -> None: write_json(root / "done.json", marks) -def pid_alive(pid) -> bool: - if not isinstance(pid, int) or pid <= 0: - return False - try: - os.kill(pid, 0) - except ProcessLookupError: - return False - except PermissionError: - return True - return True +STARTUP_GRACE_SEC = 60 -def read_state(home: Path | None = None) -> dict: - """The refresh state. A run whose process died mid-way reads as failed.""" - state = read_json(base_dir(home) / "state.json", {}) +def read_state(home: Path | None = None, now: float | None = None) -> dict: + """The refresh state. A run holds refresh.lock while it works, so a run + that isn't holding it a minute after it started has stopped.""" + root = base_dir(home) + state = read_json(root / "state.json", {}) state = state if isinstance(state, dict) else {} - if state.get("running") and not pid_alive(state.get("pid")): + if not state.get("running"): + return state + with locked(root / "refresh.lock", blocking=False) as free: + held = not free + try: + age = (now if now is not None else time.time()) - datetime.fromisoformat(state["started"]).timestamp() + except (KeyError, TypeError, ValueError): + age = float("inf") + if not held and age > STARTUP_GRACE_SEC: state.update(running=False, ok=False, phase=None, error=state.get("error") or "The refresh stopped before it finished.") return state diff --git a/src/assistant/slack_focus/triage.py b/src/assistant/slack_focus/triage.py index d4d7130..151f25c 100644 --- a/src/assistant/slack_focus/triage.py +++ b/src/assistant/slack_focus/triage.py @@ -18,6 +18,7 @@ from typing import Callable from .settings import Settings +from .store import signature LANE = {"ball_mine_dm": "Do first", "ball_mine_mention": "Do first", "thread_moved_on": "This week", "group_mention": "This week", "unread_watched": "Optional"} @@ -82,7 +83,7 @@ def jira_done(self, key: str) -> bool: if key not in self._jira: base = jira.base_url or self._env.get("JIRA_BASE_URL", "").rstrip("/") token = self._env.get(jira.token_env, "") - email = self._env.get(jira.email_env, "") + email = self._env.get(jira.email_env, "") if jira.email_env else "" done = False if base.startswith("https://") and token: auth = ("Basic " + base64.b64encode(f"{email}:{token}".encode()).decode()) if email \ @@ -158,7 +159,7 @@ def triage(threads_doc: dict, mine: list[dict], settings: Settings, resolver: Re "low_signal": noise(t, me_name), "lane": lane, "channel": t["channel"], "who": t["last_author"], "text": (ask["text"] or t["first"]["text"])[:160], "link": t["permalink"], "mention": t["mentioned_after_my_last_reply"] == "direct", - "rank": rank(t), "last": t["last_activity"][:16], "thread_id": t["id"], + "rank": rank(t), "last": t["last_activity"][:16], "thread_id": t["id"], "sig": signature(t), }) seen_prs = set() diff --git a/tests/drive_dashboard_overview.py b/tests/drive_dashboard_overview.py index a688025..0875f91 100644 --- a/tests/drive_dashboard_overview.py +++ b/tests/drive_dashboard_overview.py @@ -132,14 +132,15 @@ def drive(output_dir, browser_executable, coverage_dir=None): {"lane": "Promise", "text": "Send the freeze plan", "link": "https://acme.slack.com/archives/C1/p3", "thread_id": 3, "channel": "#team", "who": "you", "last": ""}, ] + sys.path.insert(0, str(REPO / "src")) + from assistant.slack_focus import store as slack_store + for slack_item in slack_items: + slack_item["sig"] = slack_store.signature(slack_threads[slack_item["thread_id"] - 1]) (slack_root / "threads.json").write_text(json.dumps({"threads": slack_threads})) (slack_root / "focus.json").write_text(json.dumps({ "generated": now.isoformat(), "window_days": 7, "judge": {"ran": True}, "active": slack_items, "read": [], "judge_dropped": [], "handled": []})) - sys.path.insert(0, str(REPO / "src")) - from assistant.slack_focus import store as slack_store - (slack_root / "done.json").write_text(json.dumps({ - slack_store.fingerprint(slack_items[2], {t["id"]: t for t in slack_threads}): now.isoformat()})) + (slack_root / "done.json").write_text(json.dumps({slack_store.fingerprint(slack_items[2]): now.isoformat()})) with mock.patch.dict(os.environ, {"HOME": str(home)}): spec = importlib.util.spec_from_file_location( "browser_renderer", REPO / "bin/render-assistant-page.py") @@ -534,6 +535,7 @@ def refresh_fixture(): refresh_fixture() assert page.locator('[data-panel="overview"]').is_visible() + load_profiler = None slack_calls = [] slack_script = {"status": [], "refresh": 200, "mark": 200} @@ -632,29 +634,38 @@ def slack_route(route): slackFocusShow({running: true}); }""") assert not page.locator("#sf-refresh").is_disabled() - def advance_until(predicate, steps=20): + def advance_until(predicate, on, steps=20): for _ in range(steps): - if page.evaluate(predicate): + if on.evaluate(predicate): return - page.clock.fast_forward(1000) - page.wait_for_timeout(50) + on.clock.fast_forward(1000) + on.wait_for_timeout(50) raise AssertionError(predicate) - def wait_for_calls(count): + def wait_for_calls(count, on=page): for _ in range(100): if len(slack_calls) >= count: return - page.wait_for_timeout(20) + on.wait_for_timeout(20) raise AssertionError((count, slack_calls)) - running_state = json.dumps({"running": True, "pid": os.getpid(), "phase": "Sorting threads"}) + running_state = json.dumps({"running": True, "started": slack_store.now_iso(), "phase": "Sorting threads"}) shows_running = "() => document.getElementById('slack-focus').dataset.running === 'true'" def finished_at(seconds): return {"running": False, "ok": True, "finished": (now + timedelta(seconds=seconds)).isoformat()} - for resume in ("refresh", "load"): + load_page = page.context.new_page() + load_page.on("pageerror", lambda error: errors.append(str(error))) + load_page.clock.install() + load_page.route("**/slack-focus/*", slack_route) + if profiler is not None: + load_profiler = page.context.new_cdp_session(load_page) + load_profiler.send("Profiler.enable") + load_profiler.send("Debugger.enable") + load_profiler.send("Profiler.startPreciseCoverage", {"callCount": True, "detailed": True}) + for resume, on in (("refresh", page), ("load", load_page)): (slack_root / "state.json").write_text(running_state) before_resume = len(slack_calls) slack_script["status"] = [finished_at(render_version + 2)] @@ -662,16 +673,16 @@ def finished_at(seconds): refresh_fixture() else: render_only() - page.reload() - wait_for_calls(before_resume + 1) - assert page.evaluate(shows_running), resume - assert page.locator("#sf-status").inner_text() == "Loading the new list…" + load_page.goto(url + "#slack") + wait_for_calls(before_resume + 1, on) + assert on.evaluate(shows_running), resume + assert on.locator("#sf-status").inner_text() == "Loading the new list…" (slack_root / "state.json").unlink() render_only() - advance_until("() => document.getElementById('slack-focus').dataset.running === 'false'") + advance_until("() => document.getElementById('slack-focus').dataset.running === 'false'", on) assert [a for a, _ in slack_calls[before_resume:]] == ["status"], slack_calls[before_resume:] - assert not page.locator("#sf-refresh").is_disabled() - assert page.locator("#sf-status").inner_text().startswith("Last pulled") + assert not on.locator("#sf-refresh").is_disabled() + assert on.locator("#sf-status").inner_text().startswith("Last pulled") (slack_root / "state.json").write_text(running_state) slack_script["status"] = [finished_at(10 ** 6)] before_stuck = len(slack_calls) @@ -708,7 +719,7 @@ def finished_at(seconds): assert not errors, errors if profiler is not None: scripts = [] - for collector in (profiler, file_profiler): + for collector in (profiler, file_profiler, load_profiler): entries = collector.send("Profiler.takePreciseCoverage")["result"] for entry in entries: source = collector.send("Debugger.getScriptSource", { diff --git a/tests/test_renderer_slack_tab.py b/tests/test_renderer_slack_tab.py index 399c6a7..2c3f18f 100644 --- a/tests/test_renderer_slack_tab.py +++ b/tests/test_renderer_slack_tab.py @@ -5,7 +5,6 @@ import importlib.util import json -import os from pathlib import Path import pytest @@ -51,7 +50,7 @@ def seed(home, focus=None, threads=None, state=None, done=None): def board_focus(**extra): return { "generated": "2026-10-20T14:03:00+00:00", "window_days": 7, "judge": {"ran": True}, - "active": [item("Answer ", mention=True, why="he asked", effort="Quick"), + "active": [item("Answer ", mention=True, why="he asked", effort="Quick", sig="a|1"), item("Ship the plan", lane="Promise", thread_id=2, effort="Medium"), item("Odd effort", lane="This week", effort="Huge", link="javascript:alert(1)")], "read": [item("FYI build green", lane="Read")], @@ -100,7 +99,7 @@ def test_board_renders_lanes_escaped_and_safe(mod, home): def test_checked_off_rows_move_to_done_by_you(mod, home): write_config(home, CONFIG) focus = board_focus() - fp = store.fingerprint(focus["active"][0], {1: {"first": {"text": "a", "at": "1"}, "latest": []}}) + fp = store.fingerprint(focus["active"][0]) seed(home, focus, done={fp: "2026-10-20T15:00:00+00:00"}) html, count = mod.render_slack_focus_tab() assert count == 0 @@ -111,8 +110,9 @@ def test_checked_off_rows_move_to_done_by_you(mod, home): def test_running_and_failed_states(mod, home): write_config(home, CONFIG) + started = store.now_iso() seed(home, board_focus(judge={"ran": False}), - state={"running": True, "pid": os.getpid(), "phase": "Pulling Slack (11 of 40 threads)"}) + state={"running": True, "started": started, "phase": "Pulling Slack (11 of 40 threads)"}) html, _ = mod.render_slack_focus_tab() assert 'data-running="true"' in html and " disabled>Refreshing…" in html assert "Refreshing: Pulling Slack (11 of 40 threads)." in html @@ -123,7 +123,7 @@ def test_running_and_failed_states(mod, home): seed(home, board_focus(generated="garbage"), state={"running": False, "ok": False}) html, _ = mod.render_slack_focus_tab() assert "at an unknown time" in html and "unknown error" in html - seed(home, board_focus(), state={"running": True, "pid": os.getpid()}) + seed(home, board_focus(), state={"running": True, "started": started}) assert "Refreshing: Starting." in mod.render_slack_focus_tab()[0] diff --git a/tests/test_slack_focus.py b/tests/test_slack_focus.py index 49edb7c..cb046f6 100644 --- a/tests/test_slack_focus.py +++ b/tests/test_slack_focus.py @@ -6,7 +6,6 @@ import base64 import io import json -import os import subprocess import urllib.error from datetime import datetime @@ -60,6 +59,7 @@ def test_parse_reads_every_user_setting(): assert s.bot_authors == ("Deploy Bot",) and s.skip_channels == ("deployments",) assert s.github_repos == ("acme/app",) and s.github_hosts == ("git.corp.example",) assert s.jira.enabled and s.jira.base_url == "https://jira.acme.test" and s.jira.project_keys == ("ABC",) + assert (s.jira.token_env, s.jira.email_env) == ("JIRA_API_TOKEN", "") def test_parse_falls_back_per_field_on_bad_types(): @@ -214,6 +214,24 @@ def test_collect_buckets_every_thread(slack, cfg): assert len(mine) == 6 and mine[0]["cid"] == "C1" and mine[-1]["dm"] +def test_collect_skips_empty_dms_and_deleted_threads(cfg): + search = {"is:dm": [{"ts": ts(1), "text": "x", "permalink": "https://acme.slack.com/archives/D1/p1", + "channel": {"id": "D1"}}], + "<@U1>": [{"ts": ts(1), "text": "x", "channel": {"id": "C1"}, + "permalink": f"https://acme.slack.com/archives/C1/p1?thread_ts={ts(2)}&cid=C1"}]} + sl = client.Client("t", FakeSlack(threads={}, history={}, search=search)) + doc, _ = collect.collect(sl, cfg, now=NOW) + assert doc["threads"] == [] + assert ("conversations.replies", {"channel": "C1", "ts": ts(2), "limit": 200}) in sl._transport.calls + + +def test_search_stops_at_its_page_limit(): + fake = FakeSlack(search={"<@U1>": [{"ts": str(n), "text": "x", "channel": {"id": "C1"}} for n in range(6)]}) + sl = client.Client("t", fake) + assert len(list(sl.search("<@U1>", max_pages=1))) == 2 + assert [p["page"] for m, p in fake.calls if m == "search.messages"] == [1] + + def test_bucket_rule_order(cfg): base = {"last_author": "Aalia Habib", "channel": "#team", "last_is_me": False, "i_posted_after_in_channel": False, "i_reacted_to_last": False, "kind": "channel", "mentioned_after_my_last_reply": None, @@ -354,14 +372,15 @@ def urlopen(req, timeout): raise urllib.error.URLError("down") return _Resp(json.dumps({"fields": {"status": {"statusCategory": {"key": "indeterminate"}}}}).encode()) - r = triage.Resolver(cfg, env={"JIRA_API_TOKEN": "tok", "JIRA_EMAIL": "me@acme.test"}, urlopen=urlopen) + cloud = settings.parse(CONFIG | {"jira": CONFIG["jira"] | {"email_env": "JIRA_EMAIL"}}) + r = triage.Resolver(cloud, env={"JIRA_API_TOKEN": "tok", "JIRA_EMAIL": "me@acme.test"}, urlopen=urlopen) assert not r.jira_done("ABC-1") and not r.jira_done("ABC-1") and len(seen) == 1 assert seen[0] == ("https://jira.acme.test/rest/api/2/issue/ABC-1?fields=status", "Basic " + base64.b64encode(b"me@acme.test:tok").decode()) assert not r.jira_done("BAD-1") env_base = settings.parse(CONFIG | {"jira": {"enabled": True, "project_keys": ["ABC"]}}) - r = triage.Resolver(env_base, env={"JIRA_BASE_URL": "https://env.acme.test/", "JIRA_API_TOKEN": "t"}, - urlopen=urlopen) + r = triage.Resolver(env_base, env={"JIRA_BASE_URL": "https://env.acme.test/", "JIRA_API_TOKEN": "t", + "JIRA_EMAIL": "someone-else@acme.test"}, urlopen=urlopen) r.jira_done("ABC-2") assert seen[-1] == ("https://env.acme.test/rest/api/2/issue/ABC-2?fields=status", "Bearer t") assert not triage.Resolver(cfg, env={}, urlopen=urlopen).jira_done("ABC-3") # no token: no request @@ -424,9 +443,13 @@ def test_judge_keeps_raw_rows_when_the_model_fails(slack, cfg): def test_judge_effort_and_missing_fields(slack, cfg): focus, threads = _focus_and_threads(slack, cfg) - answer = json.dumps([{"id": 0, "lane": "Do first", "title": "t", "effort": "Huge"}, {"id": 1}, "junk"]) + answer = json.dumps([{"id": 0, "lane": "Do first", "title": "t", "effort": "Huge", "why": 7}, + {"id": 1}, {"id": 2, "title": 42}, {"id": 3, "title": " "}, "junk", + {"id": 4, "lane": "Done", "title": "done", "why": ["x"]}]) judged, _ = judge.judge(focus, threads, [], None, lambda p: answer) + assert [i["text"] for i in judged["active"]] == ["t"] assert judged["active"][0]["effort"] == "" and judged["active"][0]["why"] == "" + assert judged["handled"][-1]["why"] == "already done" def test_usable_cache_rejects_mismatches(): @@ -453,15 +476,25 @@ def test_done_evidence_lists_merged_prs(cfg): # ─── store ─────────────────────────────────────────────────────────────────── def test_fingerprint_changes_with_a_new_reply(): - threads = {1: {"first": {"text": "a", "at": "1"}, "latest": [{"text": "b", "at": "2"}]}, - 2: {"first": {"text": "a", "at": "1"}, "latest": []}} - item = {"link": "https://x/1", "thread_id": 1} - fp = store.fingerprint(item, threads) - assert len(fp) == 64 and fp == store.fingerprint(item, threads) - threads[1]["latest"].append({"text": "c", "at": "3"}) - assert store.fingerprint(item, threads) != fp - assert store.fingerprint({"link": "https://x/1", "thread_id": 2}, threads) != \ - store.fingerprint({"link": "https://x/1"}, threads) + thread = {"first": {"text": "a", "at": "1"}, "latest": [{"text": "b", "at": "2"}]} + assert store.signature(thread) == "b|2" + assert store.signature({"first": {"text": "a", "at": "1"}, "latest": []}) == "a|1" + assert store.signature(None) == "" + item = {"link": "https://x/1", "sig": store.signature(thread)} + fp = store.fingerprint(item) + assert len(fp) == 64 and fp == store.fingerprint(dict(item)) + thread["latest"].append({"text": "c", "at": "3"}) + assert store.fingerprint(dict(item, sig=store.signature(thread))) != fp + assert store.fingerprint({"link": "https://x/1"}) == store.fingerprint({"link": "https://x/1", "sig": ""}) + + +def test_fingerprint_matches_the_focus_board_formula(): + """A focus-board done.json keeps working: same link, same last message, same key.""" + import hashlib + + link = "https://acme.slack.com/archives/C1/p1" + old = hashlib.sha256((link + "||" + "thanks!|2026-10-20T10:00").encode()).hexdigest() + assert store.fingerprint({"link": link, "sig": "thanks!|2026-10-20T10:00"}) == old def test_done_marks_and_json_io(tmp_path): @@ -493,31 +526,28 @@ def test_lock_is_exclusive(tmp_path): assert again -def test_state_reports_a_dead_run_as_failed(tmp_path): +def test_state_is_live_only_while_a_run_holds_the_lock(tmp_path): + root = store.base_dir(tmp_path) assert store.read_state(tmp_path) == {} - store.write_state(tmp_path, running=True, pid=os.getpid(), phase="Pulling Slack") - assert store.read_state(tmp_path)["running"] is True - store.write_state(tmp_path, pid=2 ** 22 + 12345) - state = store.read_state(tmp_path) + started = "2026-10-20T10:00:00+00:00" + at = datetime.fromisoformat(started).timestamp() + store.write_state(tmp_path, running=True, started=started, phase="Pulling Slack") + assert store.read_state(tmp_path, now=at + 30)["running"] is True # still starting up + with store.locked(root / "refresh.lock"): + assert store.read_state(tmp_path, now=at + 3600)["running"] is True + state = store.read_state(tmp_path, now=at + 61) assert (state["running"], state["ok"], state["phase"]) == (False, False, None) assert "stopped before it finished" in state["error"] - (store.base_dir(tmp_path) / "state.json").write_text("[]") + store.write_state(tmp_path, started=None) + assert store.read_state(tmp_path)["running"] is False + store.write_state(tmp_path, started="garbage", error="Slack refused the token") + assert store.read_state(tmp_path)["error"] == "Slack refused the token" + (root / "state.json").write_text("[]") assert store.write_state(tmp_path, ok=True) == {"ok": True} - (store.base_dir(tmp_path) / "state.json").write_text("[]") + (root / "state.json").write_text("[]") assert store.read_state(tmp_path) == {} -def test_pid_alive(monkeypatch): - assert store.pid_alive(os.getpid()) - assert not store.pid_alive(None) and not store.pid_alive(0) and not store.pid_alive("12") - - def denied(pid, sig): - raise PermissionError() - - monkeypatch.setattr(store.os, "kill", denied) - assert store.pid_alive(1) - - # ─── board ─────────────────────────────────────────────────────────────────── def test_board_splits_open_and_checked_off(tmp_path, slack, cfg): @@ -527,9 +557,11 @@ def test_board_splits_open_and_checked_off(tmp_path, slack, cfg): focus, threads = _focus_and_threads(slack, cfg) root = store.base_dir(tmp_path) store.write_json(root / "focus.json", focus) - store.write_json(root / "threads.json", {"threads": list(threads.values())}) first = focus["active"][0] - store.set_done(tmp_path, store.fingerprint(first, threads), True) + assert first["sig"] == store.signature(threads[first["thread_id"]]) + store.set_done(tmp_path, store.fingerprint(first), True) + # A newer pull renumbers threads.json; the check-off still holds. + store.write_json(root / "threads.json", {"threads": [dict(t, id=t["id"] + 100) for t in threads.values()]}) view = board.load(tmp_path) assert [d["text"] for d in view["done_by_you"]] == [first["text"]] and view["done_by_you"][0]["done_at"] names = [name for name, *_ in view["lanes"]] diff --git a/tests/test_slack_focus_cli.py b/tests/test_slack_focus_cli.py index 3b177ca..d4e2af0 100644 --- a/tests/test_slack_focus_cli.py +++ b/tests/test_slack_focus_cli.py @@ -170,3 +170,36 @@ def test_rerender_runs_the_dashboard_renderer(monkeypatch): cli.rerender() assert calls[0][0] == [sys.executable, str(BIN / "render-assistant-page.py")] assert calls[0][1]["check"] is False + + +def test_run_gives_up_on_output_after_the_kill(home, monkeypatch): + class Stuck: + pid = 1 + + def __init__(self, *args, **kwargs): + self.calls = 0 + + def communicate(self, input=None, timeout=None): + raise subprocess.TimeoutExpired("cmd", timeout) + + def kill(self): + pass + + monkeypatch.setattr(cli.subprocess, "Popen", Stuck) + monkeypatch.setattr(cli.os, "getpgid", lambda pid: (_ for _ in ()).throw(ProcessLookupError())) + assert cli.run(["x"], timeout=1) == (124, "", "\ntimeout after 1s") + + +def test_script_entry_point_reports_missing_setup(tmp_path): + env = {**os.environ, "HOME": str(tmp_path)} + done = subprocess.run([sys.executable, str(BIN / "slack-focus.py"), "--triage", "--no-judge"], + capture_output=True, text=True, env=env, timeout=60) + assert done.returncode == 1 and "slack_focus block" in done.stderr + assert store.read_state(tmp_path)["error"].startswith("Add a slack_focus block") + + +def test_main_stops_a_refresh_that_runs_too_long(home, monkeypatch): + monkeypatch.setattr(cli, "REFRESH_TIMEOUT_SEC", 1) + monkeypatch.setattr(cli, "refresh", lambda h, **kw: __import__("time").sleep(5)) + assert cli.main([]) == 1 + assert store.read_state(home)["error"] == "The refresh took longer than 0 minutes." diff --git a/tests/test_todo_server.py b/tests/test_todo_server.py index 8354474..e4cbcaa 100644 --- a/tests/test_todo_server.py +++ b/tests/test_todo_server.py @@ -848,3 +848,29 @@ def test_slack_focus_done_and_undone(server, slack_home): assert store.read_json(store.base_dir(server.env.tmp) / "done.json") == {} assert _request(server.base, "POST", "/slack-focus/done", json.dumps({"fp": "../x"}))[0] == 400 assert _request(server.base, "POST", "/slack-focus/done", "{bad")[0] == 400 + + +def test_slack_focus_store_import_adds_src_to_the_path(slack_home, monkeypatch): + monkeypatch.setattr(sys, "path", [p for p in sys.path if p != str(mod.SRC_DIR)]) + assert mod._slack_focus_store() is slack_home + assert sys.path[0] == str(mod.SRC_DIR) + + +def test_slack_focus_routes_refuse_a_null_origin(server, slack_home): + for path in ("/slack-focus/status", "/slack-focus/refresh", "/slack-focus/done"): + status, _ = _request(server.base, "POST", path, json.dumps({"fp": "a" * 64}), headers={"Origin": "null"}) + assert status == 403, path + status, _ = _request(server.base, "POST", "/slack-focus/status", headers={"Origin": f"http://127.0.0.1:{mod.PORT}"}) + assert status == 200 + assert _FakeProc.made == [] + + +def test_slack_focus_spawn_failure_is_recorded(server, slack_home, monkeypatch): + def no_zsh(*args, **kwargs): + raise FileNotFoundError("/bin/zsh") + + monkeypatch.setattr(mod.subprocess, "Popen", no_zsh) + assert json.loads(_request(server.base, "POST", "/slack-focus/refresh")[1]) == {"started": False} + state = json.loads(_request(server.base, "POST", "/slack-focus/status")[1]) + assert (state["running"], state["ok"]) == (False, False) + assert state["error"].startswith("The refresh couldn't start: ") From d7766b8c399d0795bcfdf98587f1ff31acbc73cc Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Tue, 20 Oct 2026 22:34:43 -0700 Subject: [PATCH 3/7] Slack tab: address review findings (coverage, local times, wording, judge feature name) Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 21 +++++++++++++++++++-- README.md | 12 ++++++------ bin/assistant-llm.py | 2 +- bin/render-assistant-page.py | 25 +++++++++++++++++-------- bin/slack-focus.py | 6 +++--- tests/drive_dashboard_overview.py | 10 +++------- tests/test_renderer_slack_tab.py | 27 ++++++++++++++++++++++----- tests/test_slack_focus.py | 17 +++++++++++++++-- tests/test_slack_focus_cli.py | 9 ++++++--- tests/test_todo_server.py | 19 +++++++++++++++++++ 10 files changed, 111 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 635d889..933ab1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,10 @@ The version is carried in `pyproject.toml` and `src/assistant/__init__.py` - The pipeline lives in `src/assistant/slack_focus/`. Collect and triage are rules only; the judge is one model call over the surfaced candidates, tools off, metered as `caller="slack_focus"`, with verdicts cached by - content. If the model fails, the raw rows show. + content. If the model fails, the raw rows show. It uses the balanced + model tier with one attempt; the focus-board ran your `claude` alias and + retried once. `bin/assistant-llm.py` routes it as the `slack_judge` + feature. - The Slack client refuses any method outside a read allowlist before a request leaves the machine, and a test checks no module names a write method. @@ -34,7 +37,21 @@ The version is carried in `pyproject.toml` and `src/assistant/__init__.py` name, and workspace come from the token. Jira project keys and GitHub hosts used to be hard-coded. - Files live in `~/.assistant/slack-focus/`. - - `bin/assistant-llm.py` accepts `--feature slack_focus`. + - Each row stores its thread's latest message, so a check-off keeps its + key while a new pull renumbers threads. The key formula matches the + focus-board's, so its `done.json` carries over. + - A refresh counts as running only while it holds `refresh.lock`, so a + crash or a reused process ID can't leave the button stuck. + - Any Slack error other than a deleted or private thread or channel stops + the refresh with that error; `ratelimited` answers are retried. The + focus-board carried on with a partial pull. A refresh stops after 20 + minutes. + - The Slack routes refuse `Origin: null`, which the other routes allow + for file:// pages. + - Times on the tab show in local time. + - Promises made in DMs read "DM" or "group DM" instead of a channel ID. + - Jira uses a bearer token unless `email_env` names the env var with your + account email; the focus-board always looked for `$JIRA_EMAIL`. - A row's text now quotes the other person's latest message, not yours, and the short-ack hint ignores your own replies. The focus-board compared author names with the label "you", which never matched a real name. diff --git a/README.md b/README.md index 2b5008b..ebca22a 100644 --- a/README.md +++ b/README.md @@ -189,7 +189,7 @@ The Slack tab needs a Slack **user** token, because only user tokens can search "github_repos": ["my-org/my-repo"], "github_hosts": ["github.com"], "jira": {"enabled": false, "base_url": "https://jira.example.com", "project_keys": ["ABC"], - "token_env": "JIRA_API_TOKEN", "email_env": "JIRA_EMAIL"} + "token_env": "JIRA_API_TOKEN"} } } ``` @@ -197,15 +197,15 @@ The Slack tab needs a Slack **user** token, because only user tokens can search | Setting | What it does | |---|---| | `days` | How far back to look, 1–30. Default 7. | -| `title` | The tab's heading. | -| `watched_channels` | Channel ID β†’ your label. Every thread there counts, not only the ones that mention you. Mentions and DMs are always checked. | -| `bot_authors` | Display names whose last word means the thread is a bot post. | +| `title` | The heading inside the Slack tab. The tab itself is always labeled **Slack**. | +| `watched_channels` | Channel ID β†’ a label for your own reference. Every thread there counts, not only the ones that mention you. Mentions and DMs are always checked. | +| `bot_authors` | Display names of bots. A thread whose latest message is from one of them is treated as a bot post. | | `skip_channels` | Channel-name fragments to treat as bot noise, such as `deployments`. | | `github_repos` | Repos where your merged PRs can retire an ask ("already done"). Uses `gh`. | | `github_hosts` | Hosts whose pull request links count, for GitHub Enterprise. Default `github.com`. | -| `jira` | Optional. With `enabled` and `project_keys`, a thread whose linked tickets are all done is auto-resolved. The token comes from the env var named in `token_env`; with `email_env` set, it uses Jira Cloud basic auth, otherwise a bearer token. `base_url` falls back to `$JIRA_BASE_URL`. | +| `jira` | Optional. With `enabled` and `project_keys`, a thread whose linked tickets are all done is auto-resolved. The token comes from the env var named in `token_env` (default `JIRA_API_TOKEN`), sent as a bearer token. For Jira Cloud, set `email_env` to the env var that holds your account email, such as `JIRA_EMAIL`, to use basic auth. `base_url` must start with `https://`; it falls back to `$JIRA_BASE_URL`. | -Open the **Slack** tab and select **Refresh from Slack**. The first pull of 7 days takes a minute or two. To refresh from a terminal, run `python3 bin/slack-focus.py` (`--triage` reuses the last pull, `--no-judge` skips the model). Files live in `~/.assistant/slack-focus/`. To route the model step to Droid, run `bin/assistant-llm.py set droid --feature slack_focus`. +Open the **Slack** tab and select **Refresh from Slack**. A 7-day pull takes about 4 minutes, plus up to a minute for the model step; a refresh stops after 20 minutes. A Slack error, such as a missing scope or a rate limit that doesn't clear, stops the refresh and shows the error on the tab, so the list is never built from a partial pull. To refresh from a terminal, run `python3 bin/slack-focus.py` (`--triage` reuses the last pull, `--no-judge` skips the model). Files live in `~/.assistant/slack-focus/`. To route the model step to Droid, run `bin/assistant-llm.py set droid --feature slack_judge`. Moving from the standalone focus board? Copy its `out/done.json` to `~/.assistant/slack-focus/done.json` to keep your check-offs. ## Config knobs you own diff --git a/bin/assistant-llm.py b/bin/assistant-llm.py index 1ef0d17..24ea3f7 100755 --- a/bin/assistant-llm.py +++ b/bin/assistant-llm.py @@ -23,7 +23,7 @@ "strategist", "narrator", "lessons", - "slack_focus", + "slack_judge", ) diff --git a/bin/render-assistant-page.py b/bin/render-assistant-page.py index ec8ad9c..9adee6a 100755 --- a/bin/render-assistant-page.py +++ b/bin/render-assistant-page.py @@ -2467,12 +2467,22 @@ def _slack_href(url): return "#" +def _slack_local_time(value, fmt="%b %d, %H:%M"): + """A stored UTC time in this machine's local time, or "" if unreadable.""" + when = parse_iso(value) if isinstance(value, str) else None + if when is None: + return "" + if when.tzinfo is None: + when = when.replace(tzinfo=timezone.utc) + return when.astimezone().strftime(fmt) + + def _slack_focus_row(item, action=None): tag = '@you' if item.get("mention") else "" meta = [x for x in (item.get("why"), item.get("channel"), item.get("who"), - (item.get("last") or "")[5:].replace("T", " ")) if x] - if action == "undo" and item.get("done_at"): - meta.append("checked off " + str(item["done_at"])[5:16].replace("T", " ")) + _slack_local_time(item.get("last"))) if x] + if action == "undo" and _slack_local_time(item.get("done_at")): + meta.append("checked off " + _slack_local_time(item["done_at"])) effort = item.get("effort") pill = f'{e(effort)}' if effort in ("Quick", "Small", "Medium") else "" button = "" @@ -2506,10 +2516,9 @@ def _render_slack_focus_tab_inner(): f'{" disabled" if running else ""}>{"Refreshing…" if running else "Refresh from Slack"}') focus = view["focus"] if focus: - when = parse_iso(focus.get("generated")) - pulled = when.strftime("%b %d, %H:%M UTC") if when else "at an unknown time" + pulled = _slack_local_time(focus.get("generated")) or "at an unknown time" how = ("A model wrote the titles and set aside noise." if view["judged"] - else "These are the raw rows; the model step didn't run.") + else "The model didn't run, so these rows aren't summarized.") status = f'Last pulled {e(pulled)} from the last {e(str(focus.get("window_days", "?")))} days. {how}' else: status = "Not checked yet. Select Refresh from Slack to build your list." @@ -2541,7 +2550,7 @@ def _render_slack_focus_tab_inner(): [_slack_focus_row(i, "undo") for i in view["done_by_you"]]), ("sf-read", "Just read", "Worth knowing. No reply needed.", [_slack_focus_row(i) for i in view["read"]]), - ("sf-aside", "Set aside", "The model judged these not actionable. Scan them to be sure.", + ("sf-aside", "Set aside", "The model found nothing for you to do here. Scan them to be sure.", [_slack_focus_row(i) for i in view["set_aside"]]), ("sf-resolved", "Auto-resolved", "A linked pull request or ticket closed.", [_slack_focus_row(i) for i in view["handled"]]), @@ -4085,7 +4094,7 @@ def render(): await refreshDashboard(true); } catch (error) { button.disabled = false; - button.textContent = "Couldn't save. Try again"; + button.textContent = "Couldn't save. Try again."; } } diff --git a/bin/slack-focus.py b/bin/slack-focus.py index 65a40aa..11298b8 100755 --- a/bin/slack-focus.py +++ b/bin/slack-focus.py @@ -75,7 +75,7 @@ def run(cmd: list[str], *, input_text: str | None = None, timeout: int = 30, def call_model(prompt: str) -> str: """The judge's one model call. Tools are off, so Slack text can't steer it into acting. Returns "" on any failure, which keeps the raw rows.""" - route = llm_runner.load_route_config(llm_runner.config_path(home=_home()), "slack_focus") + route = llm_runner.load_route_config(llm_runner.config_path(home=_home()), "slack_judge") provider = llm_runner.select_provider(route, []) model = route.droid_model if provider == "droid" else model_tiers.model_for("balanced") run_dir = store.base_dir(_home()) / "judge-run" @@ -107,7 +107,7 @@ def phase(home: Path, text: str) -> None: def refresh(home: Path, *, pull: bool, use_judge: bool, token: str, transport=slack_client.http_transport, model=call_model, - resolver: triage.Resolver | None = None) -> dict: + resolver: triage.Resolver | None = None, now: float | None = None) -> dict: """Run the pipeline and return the board it wrote.""" root = store.base_dir(home) cfg = settings.load(home) @@ -117,7 +117,7 @@ def refresh(home: Path, *, pull: bool, use_judge: bool, token: str, phase(home, "Pulling Slack") client = slack_client.Client(token, transport) threads_doc, mine = collect.collect( - client, cfg, progress=lambda i, n: store.write_state(home, phase=f"Pulling Slack ({i} of {n} threads)")) + client, cfg, now=now, progress=lambda i, n: store.write_state(home, phase=f"Pulling Slack ({i} of {n} threads)")) store.write_json(root / "threads.json", threads_doc) store.write_json(root / "mine.json", mine) threads_doc = store.read_json(root / "threads.json") diff --git a/tests/drive_dashboard_overview.py b/tests/drive_dashboard_overview.py index 0875f91..741bc43 100644 --- a/tests/drive_dashboard_overview.py +++ b/tests/drive_dashboard_overview.py @@ -598,12 +598,8 @@ def slack_route(route): assert page.locator("#sf-status").inner_text() == message slack_script["status"] = [{"running": True}, 500] page.evaluate("() => { slackFocusPoll(); }") - try: - page.wait_for_function("() => document.getElementById('sf-status').textContent === 'Refreshing: Starting.'", - timeout=5000) - except Exception: - print("DEBUG", page.locator('#sf-status').inner_text(), slack_calls, slack_script) - raise + page.wait_for_function( + "() => document.getElementById('sf-status').textContent === 'Refreshing: Starting.'") page.clock.fast_forward(2000) page.wait_for_function("text => document.getElementById('sf-status').textContent === text", arg="Couldn't check the refresh: the server returned 500") @@ -626,7 +622,7 @@ def slack_route(route): slack_script["mark"] = 400 page.locator('.sf-do .sf-mark').click() page.wait_for_function("text => document.querySelector('.sf-do .sf-mark').textContent === text", - arg="Couldn't save. Try again") + arg="Couldn't save. Try again.") assert not page.locator('.sf-do .sf-mark').is_disabled() slack_script["mark"] = 200 page.evaluate("""() => { diff --git a/tests/test_renderer_slack_tab.py b/tests/test_renderer_slack_tab.py index 2c3f18f..28b606f 100644 --- a/tests/test_renderer_slack_tab.py +++ b/tests/test_renderer_slack_tab.py @@ -5,6 +5,7 @@ import importlib.util import json +import time from pathlib import Path import pytest @@ -18,7 +19,11 @@ @pytest.fixture def home(tmp_path, monkeypatch): monkeypatch.setenv("HOME", str(tmp_path)) - return tmp_path + monkeypatch.setenv("TZ", "UTC") + time.tzset() + yield tmp_path + monkeypatch.undo() + time.tzset() @pytest.fixture @@ -84,11 +89,11 @@ def test_board_renders_lanes_escaped_and_safe(mod, home): assert "1 to do first" in html assert "Answer <Tim>" in html and "" not in html assert '@you' in html - assert "he asked Β· #team Β· Tim Beauchamp Β· 10-18 09:30" in html + assert "he asked Β· #team Β· Tim Beauchamp Β· Oct 18, 09:30" in html assert 'class="sf-effort sf-quick">Quick' in html and 'class="sf-effort sf-medium">Medium' in html assert "Huge" not in html assert 'href="#"' in html and "javascript:" not in html - assert "Last pulled Oct 20, 14:03 UTC from the last 7 days. A model wrote the titles" in html + assert "Last pulled Oct 20, 14:03 from the last 7 days. A model wrote the titles" in html for label in ("Do first", "Promise", "This week", "Optional", "Done by you", "Just read", "Set aside", "Auto-resolved"): assert label in html, label @@ -105,7 +110,7 @@ def test_checked_off_rows_move_to_done_by_you(mod, home): assert count == 0 done = html[html.index('data-context-key="sf-done"'):] assert "Answer <Tim>" in done[:done.index("")] - assert "checked off 10-20 15:00" in html and 'data-done="false"' in html and ">Undo<" in html + assert "checked off Oct 20, 15:00" in html and 'data-done="false"' in html and ">Undo<" in html def test_running_and_failed_states(mod, home): @@ -119,7 +124,7 @@ def test_running_and_failed_states(mod, home): seed(home, board_focus(judge={"ran": False}), state={"running": False, "ok": False, "error": "invalid "}) html, _ = mod.render_slack_focus_tab() assert "The last refresh didn't finish: invalid <auth>" in html - assert "These are the raw rows" in html + assert "The model didn't run, so these rows aren't summarized." in html seed(home, board_focus(generated="garbage"), state={"running": False, "ok": False}) html, _ = mod.render_slack_focus_tab() assert "at an unknown time" in html and "unknown error" in html @@ -154,3 +159,15 @@ def test_full_render_adds_the_slack_tab(mod, home): assert 'data-tab="slack"' in html and 'data-panel="slack"' in html assert 'Slack 1' in html assert "'slack', 'todos'" in html and "function slackFocusRefresh()" in html + + +def test_times_show_in_local_time(mod, home, monkeypatch): + monkeypatch.setenv("TZ", "America/Los_Angeles") + time.tzset() + write_config(home, CONFIG) + seed(home, board_focus()) + html, _ = mod.render_slack_focus_tab() + assert "Last pulled Oct 20, 07:03 from" in html + assert "Tim Beauchamp Β· Oct 18, 02:30" in html + assert mod._slack_local_time("2026-10-20T14:03:00+00:00", "%H:%M") == "07:03" + assert mod._slack_local_time("not a time") == "" and mod._slack_local_time(None) == "" diff --git a/tests/test_slack_focus.py b/tests/test_slack_focus.py index cb046f6..9f8f79a 100644 --- a/tests/test_slack_focus.py +++ b/tests/test_slack_focus.py @@ -431,6 +431,17 @@ def model(prompt): assert calls # a new merged PR invalidates every cached verdict +def test_judge_rejudges_a_row_whose_text_changed(slack, cfg): + focus, threads = _focus_and_threads(slack, cfg) + _, cache = judge.judge(focus, threads, [], None, lambda p: _answer(focus, {0: "Do first"})) + changed = json.loads(json.dumps(focus)) + changed["active"][0]["text"] = "a brand new reply" + prompts = [] + judged, _ = judge.judge(changed, threads, [], cache, lambda p: prompts.append(p) or _answer(changed, {0: "Read"})) + assert len(prompts) == 1 and "a brand new reply" in prompts[0] + assert judged["judge"]["rejudged"] == 1 and judged["read"][0]["text"] == "title 0" + + def test_judge_keeps_raw_rows_when_the_model_fails(slack, cfg): focus, threads = _focus_and_threads(slack, cfg) for answer in ("", "no json here", "[{bad json]", "[]"): @@ -587,5 +598,7 @@ def test_package_never_names_a_write_method(): src = pathlib.Path(client.__file__).parent text = "".join(p.read_text() for p in src.glob("*.py")) - for method in ('"chat.', '"reactions.', '"conversations.mark', '"files.', '"pins.'): - assert method not in text + import re + + assert not re.search(r"""["'](chat|reactions|files|pins|reminders|stars)\.\w+|conversations\.(mark|join|leave)""", + text) diff --git a/tests/test_slack_focus_cli.py b/tests/test_slack_focus_cli.py index d4e2af0..8d741b1 100644 --- a/tests/test_slack_focus_cli.py +++ b/tests/test_slack_focus_cli.py @@ -13,7 +13,7 @@ import pytest from assistant.slack_focus import store, triage -from slack_focus_fixtures import CONFIG, FakeSlack, write_config +from slack_focus_fixtures import CONFIG, NOW, FakeSlack, write_config BIN = Path(__file__).resolve().parent.parent / "bin" @@ -53,17 +53,20 @@ def model_keep_first(prompt): def test_full_refresh_writes_every_file(home): focus = cli.refresh(home, pull=True, use_judge=True, token="xoxp-t", transport=FakeSlack(), - model=model_keep_first, resolver=offline_resolver(home)) + model=model_keep_first, resolver=offline_resolver(home), now=NOW) root = store.base_dir(home) for name in ("threads.json", "mine.json", "focus.json", "judge-cache.json"): assert (root / name).exists(), name assert [i["text"] for i in focus["active"]] == ["Answer Aalia"] + assert len(store.read_json(root / "threads.json")["threads"]) == 10 + assert len(focus["judge_dropped"]) > 5 assert store.read_json(root / "focus.json") == focus assert store.read_state(home)["phase"] == "Asking the model which threads need you" def test_triage_only_refresh_reuses_the_last_pull(home): - cli.refresh(home, pull=True, use_judge=False, token="t", transport=FakeSlack(), resolver=offline_resolver(home)) + cli.refresh(home, pull=True, use_judge=False, token="t", transport=FakeSlack(), resolver=offline_resolver(home), + now=NOW) calls = FakeSlack() focus = cli.refresh(home, pull=False, use_judge=False, token="", transport=calls, resolver=offline_resolver(home)) diff --git a/tests/test_todo_server.py b/tests/test_todo_server.py index e4cbcaa..8b63264 100644 --- a/tests/test_todo_server.py +++ b/tests/test_todo_server.py @@ -874,3 +874,22 @@ def no_zsh(*args, **kwargs): state = json.loads(_request(server.base, "POST", "/slack-focus/status")[1]) assert (state["running"], state["ok"]) == (False, False) assert state["error"].startswith("The refresh couldn't start: ") + + +def test_slack_focus_done_rerenders_the_dashboard(server, slack_home, monkeypatch): + renders = [] + monkeypatch.setattr(mod, "rerender_dashboard", lambda: renders.append(1)) + _request(server.base, "POST", "/slack-focus/done", json.dumps({"fp": "b" * 64})) + _request(server.base, "POST", "/slack-focus/undone", json.dumps({"fp": "b" * 64})) + assert renders == [1, 1] + + +def test_slack_focus_reap_leaves_a_started_childs_state_alone(server, slack_home): + store = slack_home + _request(server.base, "POST", "/slack-focus/refresh") + store.write_state(server.env.tmp, running=True, pid=_FakeProc.made[0].pid, + started="2026-10-20T10:00:00+00:00", error=None) + _FakeProc.made[0].exit = -9 + state = json.loads(_request(server.base, "POST", "/slack-focus/status")[1]) + assert (state["running"], state["ok"]) == (False, False) + assert state["error"] == "The refresh stopped before it finished." From a20b3032b70c01605a38656ade5d990a99439ac6 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Tue, 20 Oct 2026 22:41:04 -0700 Subject: [PATCH 4/7] Slack tab: make the 20-minute limit uncatchable by retry loops, kill the model on it, retry the run lock Co-Authored-By: Claude Opus 5.5 --- bin/slack-focus.py | 26 ++++++++++++++++------- src/assistant/slack_focus/judge.py | 3 ++- src/assistant/slack_focus/store.py | 20 +++++++++++------- tests/test_slack_focus.py | 27 ++++++++++++++++++++++-- tests/test_slack_focus_cli.py | 33 +++++++++++++++++++++++++++++- 5 files changed, 91 insertions(+), 18 deletions(-) diff --git a/bin/slack-focus.py b/bin/slack-focus.py index 11298b8..adad731 100755 --- a/bin/slack-focus.py +++ b/bin/slack-focus.py @@ -61,15 +61,22 @@ def run(cmd: list[str], *, input_text: str | None = None, timeout: int = 30, out, err = proc.communicate(input=input_text, timeout=timeout) return proc.returncode, out or "", err or "" except subprocess.TimeoutExpired: - try: - os.killpg(os.getpgid(proc.pid), signal.SIGKILL) - except (OSError, ProcessLookupError): - proc.kill() + _kill_group(proc) try: out, err = proc.communicate(timeout=10) except subprocess.TimeoutExpired: out, err = "", "" return 124, out or "", (err or "") + f"\ntimeout after {timeout}s" + except BaseException: + _kill_group(proc) + raise + + +def _kill_group(proc: subprocess.Popen) -> None: + try: + os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + except (OSError, ProcessLookupError): + proc.kill() def call_model(prompt: str) -> str: @@ -137,8 +144,13 @@ def refresh(home: Path, *, pull: bool, use_judge: bool, token: str, return focus +class RefreshTooSlow(BaseException): + """Raised by the alarm. A BaseException, so the retry and error handlers + that catch OSError (TimeoutError included) along the way can't swallow it.""" + + def _too_slow(signum, frame): - raise TimeoutError(f"The refresh took longer than {REFRESH_TIMEOUT_SEC // 60} minutes.") + raise RefreshTooSlow(f"The refresh took longer than {REFRESH_TIMEOUT_SEC // 60} minutes.") def main(argv: list[str] | None = None) -> int: @@ -148,7 +160,7 @@ def main(argv: list[str] | None = None) -> int: args = ap.parse_args(argv) home = _home() root = store.base_dir(home) - with store.locked(root / "refresh.lock", blocking=False) as got: + with store.locked(root / "refresh.lock", blocking=False, wait=1.0) as got: if not got: print("A refresh is already running.", file=sys.stderr) return 0 @@ -162,7 +174,7 @@ def main(argv: list[str] | None = None) -> int: focus = refresh(home, pull=not args.triage, use_judge=not args.no_judge, token=os.environ.get("SLACK_USER_TOKEN", "")) print(json.dumps(focus["counts"])) - except Exception as e: # noqa: BLE001 β€” every run must record how it ended + except (Exception, RefreshTooSlow) as e: # noqa: BLE001 β€” every run must record how it ended error = str(e)[:300] or type(e).__name__ print(f"refresh failed: {error}", file=sys.stderr) finally: diff --git a/src/assistant/slack_focus/judge.py b/src/assistant/slack_focus/judge.py index b9d587b..bf6e550 100644 --- a/src/assistant/slack_focus/judge.py +++ b/src/assistant/slack_focus/judge.py @@ -86,7 +86,8 @@ def usable_cache(saved, evidence: list[str], rubric_sig: str) -> dict: if not isinstance(saved, dict) or set(saved.get("done_evidence", [])) != set(evidence) \ or saved.get("rubric") != rubric_sig or not isinstance(saved.get("verdicts"), dict): return {} - return {k: v for k, v in saved["verdicts"].items() if v == "DROP" or isinstance(v, dict)} + return {k: v for k, v in saved["verdicts"].items() + if v == "DROP" or (isinstance(v, dict) and isinstance(v.get("title"), str) and v["title"].strip())} def parse(raw: str) -> dict[int, dict] | None: diff --git a/src/assistant/slack_focus/store.py b/src/assistant/slack_focus/store.py index abb9eaf..2dfabb8 100644 --- a/src/assistant/slack_focus/store.py +++ b/src/assistant/slack_focus/store.py @@ -52,15 +52,21 @@ def write_json(path: Path, data) -> None: @contextmanager -def locked(path: Path, blocking: bool = True): - """An exclusive flock on `path`. Yields False when non-blocking and taken.""" +def locked(path: Path, blocking: bool = True, wait: float = 0.0): + """An exclusive flock on `path`. Non-blocking, it retries for `wait` + seconds, then yields False if the lock is still taken.""" path.parent.mkdir(parents=True, exist_ok=True) + deadline = time.monotonic() + wait with open(path, "a+") as f: - try: - fcntl.flock(f.fileno(), fcntl.LOCK_EX | (0 if blocking else fcntl.LOCK_NB)) - except BlockingIOError: - yield False - return + while True: + try: + fcntl.flock(f.fileno(), fcntl.LOCK_EX | (0 if blocking else fcntl.LOCK_NB)) + break + except BlockingIOError: + if time.monotonic() >= deadline: + yield False + return + time.sleep(0.05) try: yield True finally: diff --git a/tests/test_slack_focus.py b/tests/test_slack_focus.py index 9f8f79a..55251db 100644 --- a/tests/test_slack_focus.py +++ b/tests/test_slack_focus.py @@ -465,8 +465,9 @@ def test_judge_effort_and_missing_fields(slack, cfg): def test_usable_cache_rejects_mismatches(): sig = "s" - good = {"done_evidence": ["a"], "rubric": sig, "verdicts": {"x": "DROP", "y": {"lane": "Read"}, "z": 3}} - assert judge.usable_cache(good, ["a"], sig) == {"x": "DROP", "y": {"lane": "Read"}} + good = {"done_evidence": ["a"], "rubric": sig, "verdicts": { + "x": "DROP", "y": {"lane": "Read", "title": "t"}, "z": 3, "n": {"title": 5}, "b": {"title": " "}}} + assert judge.usable_cache(good, ["a"], sig) == {"x": "DROP", "y": {"lane": "Read", "title": "t"}} assert judge.usable_cache(good, ["b"], sig) == {} assert judge.usable_cache(good, ["a"], "other") == {} assert judge.usable_cache(good | {"verdicts": []}, ["a"], sig) == {} @@ -533,10 +534,32 @@ def test_lock_is_exclusive(tmp_path): assert first with store.locked(path, blocking=False) as second: assert second is False + with store.locked(path, blocking=False, wait=0.2) as waited: + assert waited is False with store.locked(path, blocking=False) as again: assert again +def test_lock_wait_takes_a_lock_released_meanwhile(tmp_path): + import threading + + path = tmp_path / "x.lock" + holding, release = threading.Event(), threading.Event() + + def hold(): + with store.locked(path): + holding.set() + release.wait(5) + + t = threading.Thread(target=hold) + t.start() + holding.wait(5) + threading.Timer(0.2, release.set).start() + with store.locked(path, blocking=False, wait=3) as got: + assert got + t.join(5) + + def test_state_is_live_only_while_a_run_holds_the_lock(tmp_path): root = store.base_dir(tmp_path) assert store.read_state(tmp_path) == {} diff --git a/tests/test_slack_focus_cli.py b/tests/test_slack_focus_cli.py index 8d741b1..c24fa66 100644 --- a/tests/test_slack_focus_cli.py +++ b/tests/test_slack_focus_cli.py @@ -8,6 +8,7 @@ import os import subprocess import sys +import time from pathlib import Path import pytest @@ -202,7 +203,37 @@ def test_script_entry_point_reports_missing_setup(tmp_path): def test_main_stops_a_refresh_that_runs_too_long(home, monkeypatch): + def hangs_inside_a_retry_loop(h, **kw): + for _ in range(5): + try: + time.sleep(5) + except OSError: # the Slack transport retries TimeoutError like this + continue + return {"counts": {}} + monkeypatch.setattr(cli, "REFRESH_TIMEOUT_SEC", 1) - monkeypatch.setattr(cli, "refresh", lambda h, **kw: __import__("time").sleep(5)) + monkeypatch.setattr(cli, "refresh", hangs_inside_a_retry_loop) + started = time.time() assert cli.main([]) == 1 + assert time.time() - started < 4 assert store.read_state(home)["error"] == "The refresh took longer than 0 minutes." + + +def test_run_kills_the_model_when_the_refresh_times_out(home, monkeypatch): + killed = [] + + class Hung: + pid = 4242 + + def __init__(self, *args, **kwargs): + pass + + def communicate(self, input=None, timeout=None): + raise cli.RefreshTooSlow("too slow") + + monkeypatch.setattr(cli.subprocess, "Popen", Hung) + monkeypatch.setattr(cli.os, "getpgid", lambda pid: pid) + monkeypatch.setattr(cli.os, "killpg", lambda pgid, sig: killed.append((pgid, sig))) + with pytest.raises(cli.RefreshTooSlow): + cli.run(["claude"], timeout=60) + assert killed == [(4242, cli.signal.SIGKILL)] From 353ad41a3bba2247857fbcdc050713db572a64de Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Wed, 21 Oct 2026 08:14:38 -0700 Subject: [PATCH 5/7] Slack tab: keep old DM threads, quick DM asks, and empty model answers Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 23 +++++++-- bin/slack-focus.py | 8 +-- bin/todo-server.py | 14 ++++- src/assistant/slack_focus/collect.py | 49 ++++++++++-------- src/assistant/slack_focus/judge.py | 7 ++- src/assistant/slack_focus/triage.py | 20 ++------ tests/slack_focus_fixtures.py | 17 +++++-- tests/test_slack_focus.py | 76 +++++++++++++++++++++++----- tests/test_slack_focus_cli.py | 14 +++++ tests/test_todo_server.py | 23 +++++++++ 10 files changed, 188 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 933ab1f..8cb1ed5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,9 +52,26 @@ The version is carried in `pyproject.toml` and `src/assistant/__init__.py` - Promises made in DMs read "DM" or "group DM" instead of a channel ID. - Jira uses a bearer token unless `email_env` names the env var with your account email; the focus-board always looked for `$JIRA_EMAIL`. -- A row's text now quotes the other person's latest message, not yours, and - the short-ack hint ignores your own replies. The focus-board compared - author names with the label "you", which never matched a real name. + - A new reply in a DM thread that started before the window now shows up. + The focus-board dropped it. + - A message counts as answered only when you posted after it. The + focus-board also counted a message of yours from up to 60 seconds + before, which hid quick DM asks. + - A direct @-mention of you outranks an earlier group mention in the same + thread. + - DMs and threads are read to the end. The focus-board stopped at 200 + messages. + - A model answer of `[]` sets every new row aside as noise. The + focus-board treated it as a failure and showed raw rows. + - A promise the model moves to Do first ranks below direct asks instead + of above every row. + - A refresh error never includes the Slack token. + - If a refresh launch hangs before it starts, the next **Refresh from + Slack** stops it and starts a new one. + - The unused low-signal hints are gone. +- A row's text now quotes the other person's latest message, not yours. The + focus-board compared author names with the label "you", which never + matched a real name. ### Changed - Read agent sessions from cmux first, with mukul-memory's registry as a diff --git a/bin/slack-focus.py b/bin/slack-focus.py index adad731..a2cb014 100755 --- a/bin/slack-focus.py +++ b/bin/slack-focus.py @@ -168,14 +168,16 @@ def main(argv: list[str] | None = None) -> int: ok=None, error=None, phase="Starting") started = time.time() error = None + token = os.environ.get("SLACK_USER_TOKEN", "") signal.signal(signal.SIGALRM, _too_slow) signal.alarm(REFRESH_TIMEOUT_SEC) try: - focus = refresh(home, pull=not args.triage, use_judge=not args.no_judge, - token=os.environ.get("SLACK_USER_TOKEN", "")) + focus = refresh(home, pull=not args.triage, use_judge=not args.no_judge, token=token) print(json.dumps(focus["counts"])) except (Exception, RefreshTooSlow) as e: # noqa: BLE001 β€” every run must record how it ended - error = str(e)[:300] or type(e).__name__ + # The error shows on the page, so it never carries the token. + text = str(e).replace(token.strip(), "[token]") if token.strip() else str(e) + error = text[:300] or type(e).__name__ print(f"refresh failed: {error}", file=sys.stderr) finally: signal.alarm(0) diff --git a/bin/todo-server.py b/bin/todo-server.py index cf6e25c..f2281c5 100755 --- a/bin/todo-server.py +++ b/bin/todo-server.py @@ -72,6 +72,7 @@ import os import re import shutil +import signal import subprocess import sys import time @@ -565,8 +566,19 @@ def slack_focus_refresh(): store = _slack_focus_store() root = store.base_dir(HOME) with store.locked(root / "refresh.lock", blocking=False) as free: - if not free or _slack_focus_proc is not None: + if not free: return json.dumps({"started": False}) + if store.read_state(HOME).get("running"): + return json.dumps({"started": False}) + if _slack_focus_proc is not None: + # The last launch never took the lock (a hung ~/.zprofile, say), and + # its startup grace is over. Stop it so this one can run. + try: + os.killpg(_slack_focus_proc.pid, signal.SIGKILL) + except ProcessLookupError: + pass + _slack_focus_proc.wait() + _slack_focus_proc = None store.write_state(HOME, running=True, pid=os.getpid(), started=store.now_iso(), finished=None, ok=None, error=None, phase="Starting") try: diff --git a/src/assistant/slack_focus/collect.py b/src/assistant/slack_focus/collect.py index efab78a..a17a27c 100644 --- a/src/assistant/slack_focus/collect.py +++ b/src/assistant/slack_focus/collect.py @@ -37,6 +37,19 @@ def bucket(t: dict, settings: Settings) -> str: return "read_fyi" +def _messages(client: Client, method: str, params: dict) -> list[dict]: + """Every page of a history or replies call, oldest first, each message once.""" + seen: dict[str, dict] = {} + cursor = "" + while True: + r = client.call(method, params | ({"cursor": cursor} if cursor else {})) + for m in r.get("messages", []): + seen.setdefault(m["ts"], m) + cursor = r.get("response_metadata", {}).get("next_cursor", "") + if not cursor: + return sorted(seen.values(), key=lambda m: float(m["ts"])) + + def _shape(client: Client, m: dict) -> dict: author = client.user_name(m["user"]) if m.get("user") else m.get("username", "bot") return {"author": author, "at": iso(m["ts"])[:16], "text": client.clean(m.get("text", ""))[:400]} @@ -49,8 +62,8 @@ def _summarize(client: Client, settings: Settings, cid: str, msgs: list[dict], t my_last = max(mine) if mine else 0.0 last = msgs[-1] after_me = [m for m in msgs if float(m["ts"]) > my_last and m.get("user") != client.me] - mention_after = next((client.mentions_me(m.get("text", "")) for m in after_me - if client.mentions_me(m.get("text", ""))), None) + kinds = {client.mentions_me(m.get("text", "")) for m in after_me} + mention_after = "direct" if "direct" in kinds else "group" if "group" in kinds else None last_read = parent_last_read if parent_last_read is not None else ch["last_read"] return { "cid": cid, "root": msgs[0]["ts"], "channel": ch["name"], "kind": ch["kind"], @@ -89,29 +102,25 @@ def collect(client: Client, settings: Settings, now: float | None = None, candidates.setdefault((m["channel"]["id"], thread_ts), None) for cid in settings.watched_channels: - cursor = "" - while True: - r = client.call("conversations.history", - {"channel": cid, "oldest": since - 14 * 86400, "limit": 200, "cursor": cursor}) - for m in r.get("messages", []): - if float(m.get("latest_reply") or m["ts"]) >= since \ - and m.get("subtype") not in ("channel_join", "bot_message"): - candidates.setdefault((cid, m.get("thread_ts") or m["ts"]), None) - cursor = r.get("response_metadata", {}).get("next_cursor", "") - if not cursor: - break + for m in _messages(client, "conversations.history", {"channel": cid, "oldest": since - 14 * 86400, "limit": 200}): + if float(m.get("latest_reply") or m["ts"]) >= since \ + and m.get("subtype") not in ("channel_join", "bot_message"): + candidates.setdefault((cid, m.get("thread_ts") or m["ts"]), None) threads = [] # A DM surfaced any way gets its full in-window history, so a DM ask isn't - # cut down to the one message search matched. + # cut down to the one message search matched. That history covers threads + # that start in the window; an older thread with a new reply keeps its own + # fetch. dm_hits = set(dm_channels) for cid, thread_ts in list(candidates): if client.channel(cid)["kind"] in ("dm", "mpim"): dm_hits.add(cid) - del candidates[(cid, thread_ts)] + if float(thread_ts) >= since: + del candidates[(cid, thread_ts)] for cid in sorted(dm_hits): - h = client.call("conversations.history", {"channel": cid, "oldest": since, "limit": 200}) - top = sorted((m for m in h.get("messages", []) if not m.get("subtype")), key=lambda m: float(m["ts"])) + h = _messages(client, "conversations.history", {"channel": cid, "oldest": since, "limit": 200}) + top = [m for m in h if not m.get("subtype")] if not top: continue for m in top: @@ -123,8 +132,8 @@ def collect(client: Client, settings: Settings, now: float | None = None, for i, (cid, thread_ts) in enumerate(candidates): if progress and i % 10 == 0: progress(i + 1, total) - r = client.call("conversations.replies", {"channel": cid, "ts": thread_ts, "limit": 200}) - msgs = [m for m in r.get("messages", []) if m.get("subtype") not in SKIP_SUBTYPES] + msgs = [m for m in _messages(client, "conversations.replies", {"channel": cid, "ts": thread_ts, "limit": 200}) + if m.get("subtype") not in SKIP_SUBTYPES] if not msgs or float(msgs[-1]["ts"]) < since: continue parent = msgs[0] @@ -139,7 +148,7 @@ def collect(client: Client, settings: Settings, now: float | None = None, for t in threads: last = datetime.fromisoformat(t["last_activity"]).timestamp() t["i_posted_after_in_channel"] = any( - c == t["cid"] and ts >= last - 60 and (dm or f"thread_ts={t['root']}" in link) + c == t["cid"] and ts > last and (dm or f"thread_ts={t['root']}" in link) for c, ts, link, dm in mine_ts) threads.sort(key=lambda t: t["last_activity"], reverse=True) diff --git a/src/assistant/slack_focus/judge.py b/src/assistant/slack_focus/judge.py index bf6e550..bced770 100644 --- a/src/assistant/slack_focus/judge.py +++ b/src/assistant/slack_focus/judge.py @@ -91,15 +91,18 @@ def usable_cache(saved, evidence: list[str], rubric_sig: str) -> dict: def parse(raw: str) -> dict[int, dict] | None: + """The model's rows by id. `[]` means every candidate is noise; an answer + with rows but none usable is a failure, like no answer.""" m = re.search(r"\[.*\]", raw or "", re.S) if not m: return None try: data = json.loads(m.group(0)) - return {int(d["id"]): d for d in data + rows = {int(d["id"]): d for d in data if isinstance(d, dict) and "id" in d and isinstance(d.get("title"), str) and d["title"].strip()} except (ValueError, TypeError): return None + return rows if rows or data == [] else None def judge(focus: dict, threads: dict[int, dict], evidence: list[str], saved_cache, llm: LLM) -> tuple[dict, dict]: @@ -120,7 +123,7 @@ def judge(focus: dict, threads: dict[int, dict], evidence: list[str], saved_cach f"{chr(10).join(evidence) or '(none found)'}\n\nCANDIDATES:\n{json.dumps(payload, indent=0)}\n") model = parse(llm(prompt)) if to_judge else {} - if to_judge and not model: + if model is None: focus["judge"] = {"ran": False, "note": "The model didn't return usable rows, so these are the raw ones."} return focus, saved_cache if isinstance(saved_cache, dict) else {} diff --git a/src/assistant/slack_focus/triage.py b/src/assistant/slack_focus/triage.py index 151f25c..5a43c0e 100644 --- a/src/assistant/slack_focus/triage.py +++ b/src/assistant/slack_focus/triage.py @@ -4,7 +4,7 @@ - a thread resolves to "Auto-resolved" when every linked PR is merged or closed, or every linked Jira ticket is done - your own messages that read like a commitment become Promises - - nothing is dropped here; low-signal threads only carry a hint for the judge + - nothing is dropped here; the judge sets noise aside """ from __future__ import annotations @@ -23,8 +23,6 @@ LANE = {"ball_mine_dm": "Do first", "ball_mine_mention": "Do first", "thread_moved_on": "This week", "group_mention": "This week", "unread_watched": "Optional"} LANE_ORDER = ["Do first", "Promise", "This week", "Optional"] -DIGEST_RE = re.compile( - r"standup|TL;DR|KEY (DECISIONS|TOPICS|OUTCOMES)|:clipboard:|weekly sync|e-?scrum|:pushpin:", re.I) PROMISE_RE = re.compile( r"\b(i'?ll|i will|let me|i can (?:get|take|look|help|make|add|fix|do|share|send)|" r"i'?m (?:on|looking|working)|will (?:do|send|share|fix|add|look|get|create|remove|update|report|sync|merge)|" @@ -114,18 +112,6 @@ def resolution(t: dict, resolver: Resolver, prs_re: re.Pattern, keys_re: re.Patt return None -def noise(t: dict, me_name: str) -> str | None: - """A hint for the judge that a thread looks low-signal. It never drops.""" - if DIGEST_RE.search(t["first"]["text"]): - return "digest or standup post" - recent = t.get("latest") or [] - if t["bucket"] == "thread_moved_on" and "?" not in " ".join(m["text"] for m in recent): - return "discussion, no open question" - if recent and len(recent[-1]["text"]) < 40 and recent[-1]["author"] != me_name: - return "trailed off (short ack)" - return None - - def rank(t: dict) -> float: score = 0.0 if t["mentioned_after_my_last_reply"] == "direct" or t.get("kind") == "dm": @@ -156,7 +142,7 @@ def triage(threads_doc: dict, mine: list[dict], settings: Settings, resolver: Re and not t["mentioned_after_my_last_reply"]: lane = "This week" active.append({ - "low_signal": noise(t, me_name), "lane": lane, "channel": t["channel"], "who": t["last_author"], + "lane": lane, "channel": t["channel"], "who": t["last_author"], "text": (ask["text"] or t["first"]["text"])[:160], "link": t["permalink"], "mention": t["mentioned_after_my_last_reply"] == "direct", "rank": rank(t), "last": t["last_activity"][:16], "thread_id": t["id"], "sig": signature(t), @@ -180,7 +166,7 @@ def triage(threads_doc: dict, mine: list[dict], settings: Settings, resolver: Re "why": "linked PR merged or closed"}) continue active.append({"lane": "Promise", "channel": channel, "who": "you", "text": text[:200], - "link": m["permalink"], "mention": False, "rank": float(m["ts"]), "last": ""}) + "link": m["permalink"], "mention": False, "rank": float(m["ts"]) / 1e9, "last": ""}) active.sort(key=lambda i: (LANE_ORDER.index(i["lane"]), -i["rank"])) return { diff --git a/tests/slack_focus_fixtures.py b/tests/slack_focus_fixtures.py index c291381..cb080a7 100644 --- a/tests/slack_focus_fixtures.py +++ b/tests/slack_focus_fixtures.py @@ -33,6 +33,16 @@ def __init__(self, threads=None, history=None, search=None, auth=None): self.history = history if history is not None else default_history() self.search_results = search if search is not None else default_search() self.auth = auth or {"ok": True, "user_id": "U1", "url": "https://acme.slack.com/"} + self.page_size = 0 # when set, DM history and replies come back in pages + + def _paged(self, msgs, params, parent_on_every_page=False): + if not self.page_size: + return {"messages": msgs} + head, rest = (msgs[:1], msgs[1:]) if parent_on_every_page else ([], msgs) + start = int(params.get("cursor") or 0) + more = start + self.page_size < len(rest) + return {"messages": head + rest[start:start + self.page_size], + "response_metadata": {"next_cursor": str(start + self.page_size) if more else ""}} def __call__(self, method, params, token): self.calls.append((method, dict(params))) @@ -54,13 +64,14 @@ def __call__(self, method, params, token): return {"messages": {"matches": chunk, "paging": {"pages": pages}}} if method == "conversations.history": msgs = self.history.get(params["channel"], []) - if params.get("cursor") == "" and params["channel"] == "C1" and len(msgs) > 1: + if "cursor" not in params and params["channel"] == "C1" and len(msgs) > 1: return {"messages": msgs[:1], "response_metadata": {"next_cursor": "next"}} if params.get("cursor") == "next": return {"messages": msgs[1:], "response_metadata": {"next_cursor": ""}} - return {"messages": msgs} + return self._paged(msgs, params) if method == "conversations.replies": - return {"messages": self.threads.get((params["channel"], params["ts"]), [])} + return self._paged(self.threads.get((params["channel"], params["ts"]), []), params, + parent_on_every_page=True) raise AssertionError(f"unexpected method {method}") diff --git a/tests/test_slack_focus.py b/tests/test_slack_focus.py index 55251db..4199b94 100644 --- a/tests/test_slack_focus.py +++ b/tests/test_slack_focus.py @@ -13,7 +13,7 @@ import pytest from assistant.slack_focus import board, client, collect, judge, settings, store, triage -from slack_focus_fixtures import CONFIG, NOW, FakeSlack, ts, write_config +from slack_focus_fixtures import CONFIG, NOW, FakeSlack, _hit, msg, ts, write_config @pytest.fixture @@ -225,6 +225,47 @@ def test_collect_skips_empty_dms_and_deleted_threads(cfg): assert ("conversations.replies", {"channel": "C1", "ts": ts(2), "limit": 200}) in sl._transport.calls +def test_a_new_reply_in_an_old_dm_thread_is_collected(cfg): + threads = {("D1", ts(10)): [msg("U2", ts(10), "old thread"), msg("U2", ts(0.5), "can you approve my promo packet?")]} + search = {"to:me": [_hit("D1", ts(0.5), "can you approve my promo packet?", thread_ts=ts(10), is_im=True)]} + sl = client.Client("t", FakeSlack(threads=threads, history={}, search=search)) + doc, _ = collect.collect(sl, cfg, now=NOW) + assert [(t["first"]["text"], t["bucket"]) for t in doc["threads"]] == [("old thread", "ball_mine_dm")] + + +def test_a_dm_that_arrives_right_after_yours_still_needs_you(cfg): + asked = f"{float(ts(1)) + 20:.6f}" + history = {"D1": [msg("U1", ts(1), "hey Aalia"), msg("U2", asked, "can you review my design doc today?")]} + search = {"is:dm": [_hit("D1", asked, "can you review", is_im=True)], + "from:me": [_hit("D1", ts(1), "hey Aalia", user="U1", is_im=True)]} + sl = client.Client("t", FakeSlack(threads={}, history=history, search=search)) + doc, _ = collect.collect(sl, cfg, now=NOW) + assert [t["bucket"] for t in doc["threads"]] == ["ball_mine_dm"] + + +def test_a_direct_mention_outranks_an_earlier_group_mention(cfg): + threads = {("C1", ts(1)): [msg("U3", ts(1), " heads-up on the freeze"), + msg("U3", ts(0.9), "<@U1> can you sign off today?")]} + search = {"<@U1>": [_hit("C1", ts(0.9), "sign off", thread_ts=ts(1))]} + sl = client.Client("t", FakeSlack(threads=threads, history={}, search=search)) + doc, _ = collect.collect(sl, settings.parse({}), now=NOW) + t = doc["threads"][0] + assert (t["mentioned_after_my_last_reply"], t["bucket"]) == ("direct", "ball_mine_mention") + + +def test_long_threads_and_dms_are_read_to_the_end(): + replies = [msg("U3", ts(1), "<@U1> kickoff")] + [msg("U2", ts(1, n), f"reply {n}") for n in range(1, 6)] + dm = [msg("U2", ts(2, n), f"dm {n}") for n in range(5)] + fake = FakeSlack(threads={("C1", ts(1)): replies}, history={"D1": dm}, + search={"<@U1>": [_hit("C1", ts(1), "kickoff")], "is:dm": [_hit("D1", ts(2), "dm 0", is_im=True)]}) + fake.page_size = 2 + doc, _ = collect.collect(client.Client("t", fake), settings.parse({}), now=NOW) + thread = by_text(doc["threads"], "kickoff") + assert thread["messages"] == 6 and thread["latest"][-1]["text"] == "reply 5" + dm_row = by_text(doc["threads"], "dm 0") + assert dm_row["messages"] == 5 and dm_row["latest"][-1]["text"] == "dm 4" + + def test_search_stops_at_its_page_limit(): fake = FakeSlack(search={"<@U1>": [{"ts": str(n), "text": "x", "channel": {"id": "C1"}} for n in range(6)]}) sl = client.Client("t", fake) @@ -332,18 +373,6 @@ def test_promises_in_dms_name_the_conversation_kind(cfg): assert [i["channel"] for i in focus["active"]] == ["DM", "group DM", "#"] -def test_noise_hints_never_drop(): - me = "Mukul Sharma" - t = {"bucket": "ball_mine_mention", "first": {"text": "TL;DR of standup"}, "latest": []} - assert triage.noise(t, me) == "digest or standup post" - t = {"bucket": "thread_moved_on", "first": {"text": "x"}, "latest": [{"text": "ok then", "author": "Tim"}]} - assert triage.noise(t, me) == "discussion, no open question" - t = {"bucket": "ball_mine_dm", "first": {"text": "x"}, "latest": [{"text": "thanks", "author": "Tim"}]} - assert triage.noise(t, me) == "trailed off (short ack)" - t["latest"] = [{"text": "thanks", "author": me}] - assert triage.noise(t, me) is None - - def test_regexes_use_configured_hosts_and_keys(): assert triage.pr_regex(["git.corp.example"]).findall("https://git.corp.example/o/r/pull/3") == \ [("git.corp.example", "o/r", "3")] @@ -444,7 +473,7 @@ def test_judge_rejudges_a_row_whose_text_changed(slack, cfg): def test_judge_keeps_raw_rows_when_the_model_fails(slack, cfg): focus, threads = _focus_and_threads(slack, cfg) - for answer in ("", "no json here", "[{bad json]", "[]"): + for answer in ("", "no json here", "[{bad json]", '[{"id": 0, "lane": "Read"}]'): judged, cache = judge.judge(focus, threads, [], {"keep": 1}, lambda p: answer) assert judged["judge"]["ran"] is False and judged["active"] == focus["active"] assert cache == {"keep": 1} @@ -452,6 +481,25 @@ def test_judge_keeps_raw_rows_when_the_model_fails(slack, cfg): assert cache == {} +def test_judge_reads_an_empty_answer_as_all_noise(slack, cfg): + focus, threads = _focus_and_threads(slack, cfg) + _, cache = judge.judge(focus, threads, [], None, lambda p: _answer(focus, {0: "Do first"})) + changed = json.loads(json.dumps(focus)) + changed["active"][1]["text"] = "thanks!" + judged, cache = judge.judge(changed, threads, [], cache, lambda p: "[]") + assert judged["judge"]["ran"] is True and judged["judge"]["rejudged"] == 1 + assert [i["text"] for i in judged["active"]] == ["title 0"] + again, _ = judge.judge(changed, threads, [], cache, lambda p: pytest.fail("re-judged")) + assert again["active"] == judged["active"] + + +def test_a_promise_moved_to_do_first_ranks_below_direct_asks(slack, cfg): + focus, threads = _focus_and_threads(slack, cfg) + p = next(i for i, item in enumerate(focus["active"]) if item["lane"] == "Promise") + judged, _ = judge.judge(focus, threads, [], None, lambda _: _answer(focus, {p: "Do first", 0: "Do first"})) + assert [i["text"] for i in judged["active"]] == ["title 0", f"title {p}"] + + def test_judge_effort_and_missing_fields(slack, cfg): focus, threads = _focus_and_threads(slack, cfg) answer = json.dumps([{"id": 0, "lane": "Do first", "title": "t", "effort": "Huge", "why": 7}, diff --git a/tests/test_slack_focus_cli.py b/tests/test_slack_focus_cli.py index c24fa66..656ca65 100644 --- a/tests/test_slack_focus_cli.py +++ b/tests/test_slack_focus_cli.py @@ -112,6 +112,20 @@ def test_main_passes_flags_and_token(home, monkeypatch): assert seen == {"pull": False, "use_judge": False, "token": "xoxp-env"} +def test_main_keeps_the_token_out_of_the_recorded_error(home, monkeypatch): + def bad_header(h, **kw): + raise ValueError("Invalid header value b'Bearer xoxp-secret\\n'") + + monkeypatch.setattr(cli, "refresh", bad_header) + monkeypatch.setenv("SLACK_USER_TOKEN", "xoxp-secret\n") + assert cli.main([]) == 1 + assert store.read_state(home)["error"] == "Invalid header value b'Bearer [token]\\n'" + assert "xoxp-secret" not in (store.base_dir(home) / "state.json").read_text() + monkeypatch.delenv("SLACK_USER_TOKEN") + assert cli.main([]) == 1 + assert store.read_state(home)["error"] == "Invalid header value b'Bearer xoxp-secret\\n'" + + def test_main_skips_while_another_refresh_runs(home, capsys): with store.locked(store.base_dir(home) / "refresh.lock"): assert cli.main([]) == 0 diff --git a/tests/test_todo_server.py b/tests/test_todo_server.py index 8b63264..e340225 100644 --- a/tests/test_todo_server.py +++ b/tests/test_todo_server.py @@ -777,6 +777,10 @@ def __init__(self, cmd, **kwargs): def poll(self): return self.exit + def wait(self): + self.waited = True + return self.exit + @pytest.fixture def slack_home(env, monkeypatch): @@ -805,6 +809,25 @@ def test_slack_focus_refresh_starts_once_and_reports_state(server, slack_home): assert len(_FakeProc.made) == 1 +@pytest.mark.parametrize("already_gone", [False, True]) +def test_slack_focus_refresh_replaces_a_launch_that_never_took_the_lock(server, slack_home, monkeypatch, + already_gone): + store = slack_home + killed = [] + + def killpg(pgid, sig): + killed.append((pgid, sig)) + if already_gone: + raise ProcessLookupError(pgid) + + monkeypatch.setattr(mod.os, "killpg", killpg) + _request(server.base, "POST", "/slack-focus/refresh") + store.write_state(server.env.tmp, started="2026-10-20T10:00:00+00:00") + assert json.loads(_request(server.base, "POST", "/slack-focus/refresh")[1]) == {"started": True} + assert killed == [(424242, mod.signal.SIGKILL)] and _FakeProc.made[0].waited + assert len(_FakeProc.made) == 2 and mod._slack_focus_proc is _FakeProc.made[1] + + def test_slack_focus_refresh_that_never_starts_is_reported(server, slack_home): _request(server.base, "POST", "/slack-focus/refresh") _FakeProc.made[0].exit = 1 From 6f952a849fedf155fe97d207819088daba3eb16a Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Wed, 21 Oct 2026 08:22:22 -0700 Subject: [PATCH 6/7] Slack tab: name a broken config, keep the pull time, pin review-found gaps with tests Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 +++-- bin/render-assistant-page.py | 4 ++- bin/slack-focus.py | 6 ++-- src/assistant/slack_focus/board.py | 3 +- src/assistant/slack_focus/settings.py | 10 ++++-- src/assistant/slack_focus/triage.py | 3 +- tests/drive_dashboard_overview.py | 1 + tests/test_renderer_slack_tab.py | 9 ++++++ tests/test_slack_focus.py | 43 +++++++++++++++++++++++-- tests/test_slack_focus_cli.py | 45 ++++++++++++++++++++++++++- 10 files changed, 117 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cb1ed5..361c8c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,8 +29,8 @@ The version is carried in `pyproject.toml` and `src/assistant/__init__.py` retried once. `bin/assistant-llm.py` routes it as the `slack_judge` feature. - The Slack client refuses any method outside a read allowlist before a - request leaves the machine, and a test checks no module names a write - method. + request leaves the machine, and a test checks that no module names a + common Slack write method. - Everything about the user is in the new `slack_focus` block of `~/.assistant/comms/config.json`: watched channels, bot names, skipped channel names, GitHub repos and hosts, and Jira projects. The Slack ID, @@ -69,6 +69,10 @@ The version is carried in `pyproject.toml` and `src/assistant/__init__.py` - If a refresh launch hangs before it starts, the next **Refresh from Slack** stops it and starts a new one. - The unused low-signal hints are gone. + - A `config.json` that isn't valid JSON shows that on the tab, with the + line, instead of the setup steps next to the last list. + - "Last pulled" shows when Slack was last pulled, also after a + `--triage` run. - A row's text now quotes the other person's latest message, not yours. The focus-board compared author names with the label "you", which never matched a real name. diff --git a/bin/render-assistant-page.py b/bin/render-assistant-page.py index 9adee6a..66922b5 100755 --- a/bin/render-assistant-page.py +++ b/bin/render-assistant-page.py @@ -2532,7 +2532,9 @@ def _render_slack_focus_tab_inner(): head = (f'

{e(view["title"])}{count}

' f'
{button}{status}
' f'
{error}') - if not view["configured"]: + if view["problem"]: + body = f'
{e(view["problem"])} Fix it, then refresh.
' + elif not view["configured"]: body = ('
Set up the Slack list: add a slack_focus block to ' '~/.assistant/comms/config.json and export SLACK_USER_TOKEN in ' '~/.zprofile. The README lists every setting.
') diff --git a/bin/slack-focus.py b/bin/slack-focus.py index a2cb014..038a079 100755 --- a/bin/slack-focus.py +++ b/bin/slack-focus.py @@ -119,7 +119,7 @@ def refresh(home: Path, *, pull: bool, use_judge: bool, token: str, root = store.base_dir(home) cfg = settings.load(home) if not cfg.configured: - raise RuntimeError("Add a slack_focus block to ~/.assistant/comms/config.json first.") + raise RuntimeError(cfg.problem or "Add a slack_focus block to ~/.assistant/comms/config.json first.") if pull: phase(home, "Pulling Slack") client = slack_client.Client(token, transport) @@ -145,8 +145,8 @@ def refresh(home: Path, *, pull: bool, use_judge: bool, token: str, class RefreshTooSlow(BaseException): - """Raised by the alarm. A BaseException, so the retry and error handlers - that catch OSError (TimeoutError included) along the way can't swallow it.""" + """Raised by the alarm. A BaseException, so the `except Exception` and + `except OSError` handlers along the way can't swallow it.""" def _too_slow(signum, frame): diff --git a/src/assistant/slack_focus/board.py b/src/assistant/slack_focus/board.py index 83fec88..8381a49 100644 --- a/src/assistant/slack_focus/board.py +++ b/src/assistant/slack_focus/board.py @@ -18,9 +18,10 @@ def load(home: Path | None = None) -> dict: root = store.base_dir(home) cfg = settings_mod.load(home) focus = store.read_json(root / "focus.json") - focus = focus if isinstance(focus, dict) and isinstance(focus.get("active"), list) else None + focus = focus if cfg.configured and isinstance(focus, dict) and isinstance(focus.get("active"), list) else None view = { "configured": cfg.configured, + "problem": cfg.problem, "title": cfg.title, "state": store.read_state(home), "focus": focus, diff --git a/src/assistant/slack_focus/settings.py b/src/assistant/slack_focus/settings.py index a74b3d6..9fc39a3 100644 --- a/src/assistant/slack_focus/settings.py +++ b/src/assistant/slack_focus/settings.py @@ -35,6 +35,7 @@ class Settings: github_repos: tuple[str, ...] = () github_hosts: tuple[str, ...] = ("github.com",) jira: JiraSettings = field(default_factory=JiraSettings) + problem: str = "" def config_path(home: Path | None = None) -> Path: @@ -76,10 +77,13 @@ def parse(block) -> Settings: def load(home: Path | None = None) -> Settings: - """Read the live settings. A missing or unreadable config means the board - isn't set up yet, which the tab shows as setup steps, never as an error.""" + """Read the live settings. A missing config means the board isn't set up + yet, which the tab shows as setup steps. A config that isn't valid JSON + names the problem instead.""" try: doc = json.loads(config_path(home).read_text()) - except (OSError, json.JSONDecodeError): + except OSError: return Settings() + except json.JSONDecodeError as e: + return Settings(problem=f"~/.assistant/comms/config.json isn't valid JSON: {e.msg} on line {e.lineno}.") return parse(doc.get("slack_focus") if isinstance(doc, dict) else None) diff --git a/src/assistant/slack_focus/triage.py b/src/assistant/slack_focus/triage.py index 5a43c0e..f79aed9 100644 --- a/src/assistant/slack_focus/triage.py +++ b/src/assistant/slack_focus/triage.py @@ -170,7 +170,8 @@ def triage(threads_doc: dict, mine: list[dict], settings: Settings, resolver: Re active.sort(key=lambda i: (LANE_ORDER.index(i["lane"]), -i["rank"])) return { - "generated": datetime.now(timezone.utc).isoformat(), "window_days": threads_doc["days"], + "generated": threads_doc.get("generated") or datetime.now(timezone.utc).isoformat(), + "window_days": threads_doc["days"], "me_name": me_name, "active": active, "handled": handled, "counts": {"active": len(active), "handled": len(handled), "threads": len(threads_doc["threads"])}, } diff --git a/tests/drive_dashboard_overview.py b/tests/drive_dashboard_overview.py index 741bc43..1346dd8 100644 --- a/tests/drive_dashboard_overview.py +++ b/tests/drive_dashboard_overview.py @@ -670,6 +670,7 @@ def finished_at(seconds): else: render_only() load_page.goto(url + "#slack") + assert load_page.locator('[data-panel="slack"]').is_visible() wait_for_calls(before_resume + 1, on) assert on.evaluate(shows_running), resume assert on.locator("#sf-status").inner_text() == "Loading the new list…" diff --git a/tests/test_renderer_slack_tab.py b/tests/test_renderer_slack_tab.py index 28b606f..4e17835 100644 --- a/tests/test_renderer_slack_tab.py +++ b/tests/test_renderer_slack_tab.py @@ -74,6 +74,15 @@ def test_setup_steps_before_configuration(mod): assert 'id="sf-refresh"' in html and 'data-running="false"' in html +def test_a_broken_config_names_the_problem(mod, home): + write_config(home, CONFIG) + seed(home, focus=board_focus()) + (home / ".assistant/comms/config.json").write_text("{bad") + html, count = mod.render_slack_focus_tab() + assert count is None and "isn't valid JSON" in html and "Fix it, then refresh." in html + assert "Set up the Slack list" not in html and "Last pulled" not in html and "to do first" not in html + + def test_not_checked_yet_once_configured(mod, home): write_config(home, CONFIG | {"title": "Mukul's Slack"}) html, count = mod.render_slack_focus_tab() diff --git a/tests/test_slack_focus.py b/tests/test_slack_focus.py index 4199b94..675551e 100644 --- a/tests/test_slack_focus.py +++ b/tests/test_slack_focus.py @@ -79,8 +79,12 @@ def test_load_reads_comms_config(tmp_path): assert settings.load(tmp_path).watched_channels == {"C1": "team"} (tmp_path / ".assistant/comms/config.json").write_text("[1]") assert not settings.load(tmp_path).configured - (tmp_path / ".assistant/comms/config.json").write_text("{bad") - assert not settings.load(tmp_path).configured + assert settings.load(tmp_path).problem == "" + (tmp_path / ".assistant/comms/config.json").write_text('{"slack_focus": {"days": 7,}}') + broken = settings.load(tmp_path) + assert not broken.configured + assert broken.problem.startswith("~/.assistant/comms/config.json isn't valid JSON: ") + assert broken.problem.endswith(" on line 1.") # ─── client ────────────────────────────────────────────────────────────────── @@ -361,6 +365,29 @@ def thread(text, bucket="ball_mine_mention"): assert focus["handled"] == [] and len(focus["active"]) == 3 +def test_a_thread_resolves_only_when_every_link_is_closed(cfg): + def thread(text): + return {"id": 1, "bucket": "ball_mine_mention", "channel": "#team", "kind": "channel", "permalink": "https://a/1", + "last_author": "Tim", "mentioned_after_my_last_reply": "direct", "last_activity": "2026-10-01T00:00:00", + "first": {"author": "Tim", "text": text, "at": "x"}, "latest": []} + + def jira(req, timeout): + key = "done" if "ABC-1" in req.full_url else "indeterminate" + return _Resp(json.dumps({"fields": {"status": {"statusCategory": {"key": key}}}}).encode()) + + resolver = triage.Resolver(cfg, run=gh_runner({"7": "MERGED", "8": "OPEN"}), env={"JIRA_API_TOKEN": "t"}, + urlopen=jira) + doc = {"days": 7, "me_name": "Mukul Sharma", "threads": [ + thread("github.com/acme/app/pull/7 and github.com/acme/app/pull/8"), thread("ABC-1 and ABC-2")]} + focus = triage.triage(doc, [], cfg, resolver) + assert focus["handled"] == [] and len(focus["active"]) == 2 + + +def test_triage_keeps_the_pull_time(cfg): + doc = {"days": 7, "generated": "2026-10-20T04:24:02+00:00", "threads": []} + assert triage.triage(doc, [], cfg, triage.Resolver(cfg, run=gh_runner(), env={}))["generated"] == doc["generated"] + + def test_promises_in_dms_name_the_conversation_kind(cfg): mine = [{"cid": "D1", "ts": ts(1), "cname": "U0ABC", "dm": True, "permalink": "https://a/1", "text": "I'll send the plan doc to you tomorrow"}, @@ -652,7 +679,17 @@ def test_board_splits_open_and_checked_off(tmp_path, slack, cfg): assert view["judged"] is False and view["handled"] == focus["handled"] +def test_board_hides_the_last_list_when_the_config_breaks(tmp_path, slack, cfg): + write_config(tmp_path, CONFIG) + store.write_json(store.base_dir(tmp_path) / "focus.json", {"active": [], "handled": []}) + assert board.load(tmp_path)["focus"] is not None + (tmp_path / ".assistant/comms/config.json").write_text("{bad") + view = board.load(tmp_path) + assert view["focus"] is None and view["problem"].startswith("~/.assistant/comms/config.json isn't valid JSON") + + def test_board_ignores_malformed_files(tmp_path): + write_config(tmp_path, CONFIG) root = store.base_dir(tmp_path) store.write_json(root / "focus.json", {"active": "nope"}) assert board.load(tmp_path)["focus"] is None @@ -660,7 +697,7 @@ def test_board_ignores_malformed_files(tmp_path): store.write_json(root / "threads.json", []) store.write_json(root / "done.json", []) view = board.load(tmp_path) - assert view["do_first"] == 1 and view["done_by_you"] == [] and not view["configured"] + assert view["do_first"] == 1 and view["done_by_you"] == [] and view["configured"] def test_package_never_names_a_write_method(): diff --git a/tests/test_slack_focus_cli.py b/tests/test_slack_focus_cli.py index 656ca65..548bfc3 100644 --- a/tests/test_slack_focus_cli.py +++ b/tests/test_slack_focus_cli.py @@ -126,6 +126,32 @@ def bad_header(h, **kw): assert store.read_state(home)["error"] == "Invalid header value b'Bearer xoxp-secret\\n'" +def test_main_waits_briefly_for_a_lock_someone_is_probing(home, monkeypatch): + import threading + + monkeypatch.setattr(cli, "refresh", lambda h, **kw: {"counts": {}}) + holding, release = threading.Event(), threading.Event() + + def probe(): + with store.locked(store.base_dir(home) / "refresh.lock"): + holding.set() + release.wait(5) + + t = threading.Thread(target=probe) + t.start() + holding.wait(5) + threading.Timer(0.2, release.set).start() + assert cli.main([]) == 0 + t.join(5) + assert store.read_state(home)["ok"] is True + + +def test_refresh_reports_a_broken_config(home): + (home / ".assistant/comms/config.json").write_text("{bad") + with pytest.raises(RuntimeError, match="isn't valid JSON"): + cli.refresh(home, pull=True, use_judge=False, token="t") + + def test_main_skips_while_another_refresh_runs(home, capsys): with store.locked(store.base_dir(home) / "refresh.lock"): assert cli.main([]) == 0 @@ -149,6 +175,23 @@ def fake_run(cmd, *, input_text=None, timeout=30, merge_bedrock=False): assert (row["caller"], row["status"], row["est_usd"]) == ("slack_focus", "ok", 0.01) +def test_the_documented_droid_command_routes_the_judge(home, monkeypatch): + done = subprocess.run([sys.executable, str(BIN / "assistant-llm.py"), "set", "droid", "--feature", "slack_judge"], + capture_output=True, text=True, timeout=60) + assert done.returncode == 0, done.stderr + seen = {} + + def invoke(**kw): + seen.update(kw) + return cli.llm_runner.LLMResult(provider="droid", model=kw["model"], rc=1, stdout="", stderr="no", + wall_ms=1, tokens_in=0, tokens_out=0, cost_usd=None, usage_source="none", + session_id=None, result_text="[1]", usable=False) + + monkeypatch.setattr(cli.llm_runner, "invoke", invoke) + assert cli.call_model("judge these") == "" # text from an unusable run is never used + assert seen["provider"] == "droid" and seen["disable_tools"] is True + + def test_call_model_failure_returns_nothing(home, monkeypatch, capsys): monkeypatch.setattr(cli, "run", lambda cmd, **kw: (1, "", "auth failed")) assert cli.call_model("x") == "" @@ -221,7 +264,7 @@ def hangs_inside_a_retry_loop(h, **kw): for _ in range(5): try: time.sleep(5) - except OSError: # the Slack transport retries TimeoutError like this + except Exception: # noqa: BLE001 β€” the retry and Jira handlers catch this broadly continue return {"counts": {}} From 4caacbf0e29a94d1b36b314fccea1c51396ddfc5 Mon Sep 17 00:00:00 2001 From: Mukul Sharma Date: Wed, 21 Oct 2026 08:34:22 -0700 Subject: [PATCH 7/7] Slack tab: test that group lookups are cached Co-Authored-By: Claude Opus 5.5 --- tests/test_slack_focus.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_slack_focus.py b/tests/test_slack_focus.py index 675551e..7c2a471 100644 --- a/tests/test_slack_focus.py +++ b/tests/test_slack_focus.py @@ -112,6 +112,8 @@ def test_client_identity_and_helpers(slack): assert slack.mentions_me("hey <@U1>") == "direct" assert slack.mentions_me(" look") == "group" assert slack.mentions_me(" look") is None + assert slack.mentions_me(" again") == "group" + assert [p["usergroup"] for m, p in slack._transport.calls if m == "usergroups.users.list"] == ["S1", "S2"] assert slack.clean("<@U2> see :tada: now") == "@Aalia Habib see the doc now" assert slack.clean(" hi") == "@group hi" assert slack.permalink("C1", "1.000002", "1.000002") == "https://acme.slack.com/archives/C1/p1000002"