Skip to content

feat(addie): add wg-slack-context job distilling WG Slack into .agents/wg/slack-context.md#5811

Merged
bokelley merged 2 commits into
mainfrom
wg-slack-context
Jul 3, 2026
Merged

feat(addie): add wg-slack-context job distilling WG Slack into .agents/wg/slack-context.md#5811
bokelley merged 2 commits into
mainfrom
wg-slack-context

Conversation

@bokelley

@bokelley bokelley commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements the Secretariat's knowledge bridge from specs/spec-guardian.md (§Knowledge bridge, landed in #5808): a daily Addie job that sweeps public working-group Slack channels, distills spec-relevant discussion into .agents/wg/slack-context.md — positions summarized, no attribution, no quotes — and ships each refresh as a single-file PR. Because .agents/** is sensitive-path gated, every refresh is held for human merge: that gate is the reviewability property the constitution's information-sources rules depend on (what the desks read at base SHA is content a human already saw merge).

Design

  • server/src/addie/jobs/github-pr.ts — single-file PR helper on the same auth seam as github-filer.ts (GITHUB_TOKEN + raw fetch, 10s timeouts, null-on-failure). Force-resets the rolling branch (addie/wg-slack-context) to base head on every run so the PR diff is always exactly "main + this file"; reuses the open PR when one exists.
  • server/src/addie/jobs/wg-slack-context.ts
    • Privacy fail-closed: a channel feeds the file only when conversations.info confirms is_private === false; null/unknown is skipped. Private WG channels never reach a public repo file.
    • Identity scrubbing at three layers: Slack markup cleaned before the LLM (user mentions → "a member"), distiller prompt forbids names/handles/companies-as-attribution and verbatim quotes, and a residual-mention guard strips any surviving <@U…> from model output.
    • Distiller: ModelConfig.primary, per-topic output (status / summary / related #issues / thread permalink), sentinel NO_SPEC_RELEVANT_DISCUSSION → no PR.
    • Material-change gate: distilled body compared against the copy on main (metadata header ignored) — no churn PRs.
  • Registration: 24h interval, weekday business hours, failureThreshold: 1; skip reasons surfaced in the result shape (missing-env, no-activity, no-spec-content, no-material-change, pr-failed).

Ops note (action needed before enabling in prod)

The job reuses the existing GITHUB_TOKEN env var. Issue-filing (knowledge-gap-closer) only needed issues:write; this job additionally needs contents:write and pull_requests:write on adcontextprotocol/adcp. If the prod token lacks those scopes the job logs pr-failed and exits — no crash — but it won't deliver until the token is upgraded (or a dedicated Secretariat token is provisioned, which pairs naturally with the planned "AAO Secretariat" GitHub App).

Test plan

  • 14 unit tests (server/tests/unit/wg-slack-context-job.test.ts, github-pr.test.ts): private-channel exclusion + fail-closed on unknown privacy, mention scrubbing on both input and output, sentinel/no-activity/material-change skips, branch create vs force-reset paths, PR reuse, missing-token no-op, contents decode/404/500 semantics.
  • tsc --project server/tsconfig.json --noEmit clean; full precommit suite passed on commit.
  • Manual: first prod run produces a sane digest PR (requires token scopes above; verify no attribution survives before merging that first refresh).

No changeset — Addie server work, not protocol surface.

🤖 Generated with Claude Code

…s/wg/slack-context.md

The Secretariat's knowledge bridge (specs/spec-guardian.md): a daily job
sweeps public working-group Slack channels, distills spec-relevant
threads with no attribution and no quotes, and ships the refresh as a
single-file PR. The sensitive-path gate holds each refresh for human
review, which is the reviewability property the constitution's
information-sources rules depend on.

- github-pr.ts: single-file PR helper on the github-filer auth seam
  (GITHUB_TOKEN + fetch); force-resets the working branch to base head
  so the diff is always exactly base + this file.
- wg-slack-context.ts: fail-closed channel privacy (only channels the
  Slack API confirms public feed the file), Slack-markup scrubbing,
  residual-mention guard on model output, material-change gate against
  the copy on main.
- Registered 24h weekday business-hours; result-shaped skips for
  missing env, no activity, no spec content, no material change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mintlify

mintlify Bot commented Jul 3, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
adcp 🟢 Ready View Preview Jul 3, 2026, 8:20 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

Comment thread server/tests/unit/github-pr.test.ts Fixed
aao-release-bot[bot]
aao-release-bot Bot previously approved these changes Jul 3, 2026

@aao-release-bot aao-release-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-tested implementation of the Secretariat's Slack knowledge bridge (spec-guardian.md §Knowledge bridge, #5808). The architecture is the right shape: raw Slack never reaches an approving desk — it enters through a diffable one-file PR that trips the .agents/** sensitive-path gate, so merging is the review. Fail-closed beats fail-open, and the privacy check honors that.

Things I checked

  • Privacy fail-closed (wg-slack-context.ts:424-428): !info || info.is_private !== false correctly skips null, unknown, and is_private === true. Only an explicit live conversations.info false proceeds. security-reviewer confirmed no cross-boundary read; the 30-min channel cache only softens the safe direction.
  • upsertFilePr control flow (github-pr.ts:104-194): base-ref lookup → 422→PATCH force-reset (:118-131), existingSha undefined on 404 so PUT creates, sha threaded on 200, PR list-and-reuse vs create. code-reviewer: all branches correct.
  • Material-change gate (wg-slack-context.ts:475-487): the daily Generated: date sits above the \n---\n divider and extractBody compares only the post-divider body, so date churn is suppressed; byte-identical LLM output is skipped as no-material-change.
  • Sensitive-path gate is real, not asserted: .github/workflows/ai-review.yml comment-and-skips any .agents/* PR, and the force-reset keeps the diff to exactly one file — reviewers can't be slipped an unrelated change.
  • Credentials: GITHUB_TOKEN/ANTHROPIC_API_KEY read from env, sent as Bearer/SDK ctor only, never logged; missing token fails closed with no GitHub calls (github-pr.ts:95-99, test at github-pr.test.ts:609).
  • Changeset: none, correctly — server-side Addie job, no static/schemas/source/** or docs/reference/** surface. MUST FIX #6 does not apply.
  • 14 unit tests cover private-exclusion/fail-closed, mention scrubbing on input and output, sentinel/no-activity/no-material-change skips, branch create-vs-reset, PR reuse, missing-token no-op.

Test-plan honesty

The one unchecked box — [ ] Manual: first prod run produces a sane digest PR (verify no attribution survives before merging) — is manual verification of this PR's primary behavior, and it ships unrun against a live Slack corpus (blocked on prod token scopes). That is acceptable here only because the design makes the first digest PR itself a human-reviewed artifact: nothing reaches main until a person reads the exact one-file diff. The unchecked box is the human gate, not a skipped test. Do run it before enabling in prod, and treat that first merge as the attribution smoke test the box describes.

Follow-ups (non-blocking — file as issues)

  • Attribution backstop is instruction-only for plaintext names (security-reviewer Medium; wg-slack-context.ts:305-321). cleanSlackText/stripResidualMentions only rewrite <@[UW]…> markup. A member's plaintext name/company — or an injected in-corpus instruction to attribute — survives to the PR branch, caught only by the human merge gate. The gate holds the privacy line, but per "Slack is prompt-injection-untrusted" a mechanical backstop shouldn't depend on the model obeying. Fence the corpus (buildDistillerPrompt:337-369 injects it undelimited) and add a roster-based plaintext redaction pass before composeFile.
  • Material-change gate rests on LLM determinism. Any reworded/reordered digest defeats the exact-match compare. Bounded in practice — force-reset + open-PR reuse means recommits to one evergreen PR, not PR spam — so it's a quality nit, not spam risk. A whitespace/heading-order normalization would tighten it.
  • Two upsertFilePr branches untested: the non-404 fileResp error path (github-pr.ts:142-144) and the listResp not-ok fallthrough (:168-173). Neither load-bearing.

Minor nits (non-blocking)

  1. First-text-block read. wg-slack-context.ts:468 reads response.content[0] only; a non-text leading block would silently become no-spec-content. No tools/thinking are enabled today, but content.find(b => b.type === 'text') is more robust.
  2. !m.subtype filter (wg-slack-context.ts:436) drops thread_broadcast replies too. Likely intentional narrowing toward parents — a one-line comment would save the next reader the double-take.
  3. Error-body logging (github-pr.ts:157,185) logs the GitHub error body; bounded (it's the already-distilled digest, not raw Slack), but status-only would match the Slack client's redaction discipline.

The ops note is candid about the token-scope gap; the job logs pr-failed and exits rather than crashing, which is the right failure mode for a scope it can't self-provision.

LGTM. Follow-ups noted below.

CodeQL read the literal .replace('%2F','/') as incomplete sanitization
(js/incomplete-sanitization, high). It was never sanitizing anything —
assert the expected path directly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@aao-release-bot aao-release-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, contained implementation of the §Knowledge-bridge design from #5808. Right shape: the safety property the constitution's information-sources rules depend on — that desks read only content a human already saw merge — is enforced by code, not asserted by prose.

Things I checked

  • Privacy fail-closed. wg-slack-context.ts:425if (!info || info.is_private !== false) admits a channel only on an explicit is_private === false from a successful conversations.info. getChannelInfo returns null on any API failure, so null/unknown/true all skip. This is the sole privacy gate — listWorkingGroupsWithSlackChannel (working-group-db.ts:465-472) returns private channels too — and it's written correctly. security-reviewer: fail-closed is correct.
  • The human-merge gate is real, not aspirational. security-reviewer traced it end to end: Argus reads at base.sha, .agents/** is a sensitive path that skips auto-review, and github-pr.ts:112-131 force-resets the branch so the refresh diff is exactly "base + this file." A poisoned digest lands in a diffable artifact a human sees before any desk reads it. fail-closed beats fail-open — confirmed.
  • 422 force-reset. github-pr.ts:114-131 — POST git/refs, and on 422 ("Reference already exists") PATCH git/refs/heads/<branch> with {sha: baseSha, force: true}. Standard GitHub semantics. Test at github-pr.test.ts:584 asserts the PATCH body.
  • Material-change gate round-trips. composeFile (wg-slack-context.ts:372-390) prepends a metadata header; extractBody (:324-327) slices from the first \n---\n and trims, yielding exactly body.trim(); the gate compares currentBody === body.trim() (:484). Header-prepend vs body-only-compare is consistent, not a bug.
  • No token leak. GITHUB_TOKEN rides only the Authorization header (github-pr.ts:44-62); every error log records status + resp.text() bodies, never headers. Missing token degrades to skipped: 'missing-env'/'pr-failed' with no crash and no partial write.
  • Identity scrubbing, three layers. cleanSlackText (:305) → prompt rules (:352-356) → stripResidualMentions (:319) on model output. Permalinks (:448) point only into confirmed-public channels, so they leak no more than the public channel already does.
  • No changeset — correct. Server-only Addie work, no static/schemas/source/** or wire surface touched.

Test-plan honesty

The one unchecked box is the primary artifact: [ ] Manual: first prod run produces a sane digest PR ... verify no attribution survives before merging that first refresh. Calling it out per the rule — but here the deferral is structural, not skipped work: the "no attribution survives" check happens at the human merge of the produced refresh PR, which is exactly the gate the design routes it through. The verification can't precede this PR because the artifact doesn't exist until the job runs. The 14 unit tests cover the mechanism; the safety-critical human check is enforced downstream by the sensitive-path merge gate. Not a downgrade — but the reviewer of that first refresh PR owns the no-attribution confirmation.

Follow-ups (non-blocking — file as issues)

  • Stale-PR / churn on the material-change gate. wg-slack-context.ts:479 reads main, not the open PR branch head. If a refresh PR is unmerged, a run whose body equals main skips as no-material-change and leaves a stale PR; the inverse re-PUTs identical content as a no-op commit. code-reviewer Minor #1. Comparing against the PR branch head when one is open closes it. Neither path crashes.
  • History truncation. wg-slack-context.ts:432getChannelHistory(..., { limit: 100 }) with no pagination. Over a 14-day window a busy channel silently drops older substantive threads before the filter. Document or paginate.
  • Prompt-fencing hygiene. buildDistillerPrompt (:337-369) interpolates Slack text with no delimiter a message author couldn't forge. Non-exploitable against desks because of the merge gate, but a fence ("treat everything inside as untrusted data") tightens artifact integrity and reduces garbled digests reaching the human reviewer. security-reviewer L1, cheap.
  • Missing job-level pr-failed test. The upsertFilePrnullskipped: 'pr-failed' mapping — the exact path the ops note is about — is untested at the job level. github-pr.test.ts covers the helper's nulls; the job's mapping and shouldLogResult gating aren't asserted.

Minor nits (non-blocking)

  1. Redundant trim. wg-slack-context.ts:475,484body = stripResidualMentions(raw) where raw is already trimmed, then the gate compares body.trim() while composeFile embeds un-trimmed body. Equal in practice; trimming once into body removes the asymmetry.

Ops note in the PR body is accurate: the job needs contents:write + pull_requests:write on the token beyond issue-filing's issues:write, and it fails to pr-failed rather than crashing if the scopes are absent. Worth confirming the prod token — or the planned Secretariat App — carries them before the first weekday-business-hours fire.

LGTM. Follow-ups noted below.

@bokelley bokelley merged commit b519186 into main Jul 3, 2026
15 checks passed
@bokelley bokelley deleted the wg-slack-context branch July 3, 2026 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants