feat: optional AI salience layer (self-hosted)#175
Open
pavelmaksimov25 wants to merge 43 commits into
Open
Conversation
github-actions
Bot
added
documentation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc75fb3b85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
minimizeBody removes HTML comments by deleting them, which concatenates adjacent text and can reassemble injection phrases the raw-field guard never saw (e.g. "ignore<!-- x --> previous instructions" becomes "ignore previous instructions" after minimization). Build the minimizedOpenEnvelope before the guard runs so both the guard and openUserPrompt see the exact same text that lands in the model envelope. Adds regression test that was RED before this commit.
digestUserPrompt omitted request.Mentions, so the model decided parent_loudness (quiet drops reminder mentions) without knowing whether any mentions were configured for the channel. Render them after the PR list, mirroring openUserPrompt's per-candidate style.
- Fix stale Run() doc comment: four sections always run (config, database, mappings, ai); a fifth appears only when a target and validator are both present. - Remove the non-existent 'file' row from the doctor.md check table; CheckMappings emits only 'entries' and conditional 'path routing'. - Add a note clarifying that a YAML parse error is a startup error, not a doctor section. - Update the output format example to include the [ai] section with its actual check names, and fix 'config' to 'entries' under [mappings].
- Replace the tautological TestEntryHashIgnoresAIFields with a meaningful comment cross-referencing the new provider-level test. - Add TestProviderEntriesHashesUnaffectedByAIConfig to routing/application: builds two providers identical except for per-tier ai: blocks and asserts their entry hashes are equal, confirming that AI config changes never invalidate config.lock. - Extend TestOpenHandlerBuildsAdvisorRequest with TierEnabled, Instructions, and EmojiAllowlist assertions, closing the per-tier opt-out regression seam. Wire standardResolver with explicit AIEnabled/AIInstructions so the assertions have meaningful values.
- sanitize.go: loop the atKeywordPattern replacement to a fixed point so reassembly attacks like "@he@herere" cannot survive a single pass. Add a sanitize test case for the reassembly input. - routing/domain/models.go: move the AIEnabled/AIInstructions comment above the two fields it documents, not below where it reads as docs for GitProvider. - gemini/module.go, openaicompat/module.go: reword doc comments; the composition root constructs the gateway directly via a switch, not by mounting these modules. Each is a convention-consistent fx wiring available for alternative use. - docs/security.md: replace the imprecise "≤200-character sanitized note" with accurate caps: context blocks and digest notes at 120 runes, thread notes at 200 runes (per domain/constants.go). - .env.example: add "# --- AI layer (optional) ---" section header consistent with the file's style.
pavelmaksimov25
force-pushed
the
feat/ai-salience-layer
branch
from
July 18, 2026 09:27
cc75fb3 to
579c4d7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 579c4d7e45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
🐳 Beta image published: docker pull ghcr.io/mptooling/notifycat:pr-175Built from 2a0fe78. |
pavelmaksimov25
commented
Jul 19, 2026
NewModelAdvisor accepted a concrete *DeterministicAdvisor but calls only domain.Advisor methods on it. Depend on the abstraction per the architecture rules; the wiring already passes a *DeterministicAdvisor, which satisfies it.
The provider-facing JSON schemas hardcoded the loudness/format/emphasis/ highlight enum values, duplicating salience/domain's enums. Build the enum arrays from those constants so the wire contract cannot drift; a golden test pins the schemas byte-for-byte identical.
PR #175 review raised a nested UNTRUSTED_DATA marker injection — forged BEGIN/END pairs sandwiching a poisoned prompt. Both existing defenses already neutralize it: the guard tripwire routes the event to the deterministic path, and wrapUntrusted defangs the delimiters so only the code's own pair survives. Lock that exact payload as a regression test.
…prober Address review: the rate-limited / generic-error switch reads more directly as two sequential if statements. Same order (rate-limited first), same behavior.
On Bitbucket, PR.Author is a user-controlled display name, so writing it in the trusted section of the open prompt let a crafted name steer the model. Move the author into the untrusted envelope (minimized), and have the guard inspect the minimized author it now shares with the model.
Address review: ModelAdvisor lived in its own file beside the tiny NewAdvisor factory. Fold both into advisor.go and rename the test to match; no behavior change.
Drop the AI-authored thread reply on opened-PR notifications end to end: the schema/prompt/wire field, the per-channel decision field and its clamp, plus the notification-side PostThreadReply port, Slack adapter, and ThreadNoteRequest. The digest's own threaded reply (PostReply) is unaffected.
… prompt Address review (#4): the system prompt now states the data is wrapped once and only the first BEGIN / last END are the boundaries, so forged inner markers read as data. Documents the invariant the code already enforces (wrapUntrusted defangs delimiters; the guard trips on the marker word) — belt, not replacement.
… one clampOpen previously kept only the channels the model returned, so a model that omitted a candidate silently dropped that channel's notification — contradicting the never-skip invariant. It now pads every omitted candidate back to its deterministic default (loudness stays the noise lever: quiet, not omission) and flags a clamp violation. The open task prompt and the OpenDecision contract are updated to state the invariant explicitly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional, default-off AI "salience" layer. When enabled, the AI decides how loudly each PR notification presents — per-channel loudness, which of the configured mentions to include, the leading emoji, standard vs. compact format, breaking-change emphasis, a short sanitized context note, and stuck-PR digest ordering. It never decides whether a notification is sent and never writes message bodies. With AI off (the default) or on any failure, behavior is byte-identical to today through a deterministic fallback.
Bring-your-own-key: Gemini or any OpenAI-compatible endpoint, including keyless local models such as Ollama. Full operator guide in
docs/ai.md; design spec indocs/superpowers/specs/2026-07-07-ai-salience-design.md.What's inside:
internal/salience/(DDD three-layer). A no-errorAdvisorport with deterministic → model → resilient implementations (LRU cache, circuit breaker, per-decision timeouts), a pure guard pipeline (secret redaction, prompt-injection tripwire, untrusted-data envelope, output sanitizer + per-field clamp), and two independent provider adapters (gemini,openaicompat).ai:block with fail-fast boot validation;AI_API_KEYis env-only andSecret-typed. Per-tierai.enabled/ai.instructionsoverrides in mappings (provider, model, and key stay global).notifycat-doctorgains anaisection with a live one-token probe and best-effort rate-limit headroom.ARCHITECTURE.mdandCLAUDE.mdupdated to eight domains.Security posture: attacker-influenced PR text is minimized and secret-redacted, wrapped in an untrusted-data envelope, and sent to a zero-tools single-turn model call, then strictly parsed and clamped. A successful injection can at most pick a wrong-but-valid enum or a short sanitized note — it can never mint a mention or link, suppress a PR, or reach the API key. The injection tripwire inspects the exact minimized text the model receives.
Notes
just checkis green (Go 1.25.12 — the branch bumps the toolchain to clear the crypto/tls advisory GO-2026-5856).mainunrelated to AI. It is up separately as fix: route config.yaml mappings and digest through the routing wire codec #173 for independent fast-tracking; once fix: route config.yaml mappings and digest through the routing wire codec #173 merges, this branch rebases ontomainand those commits drop out.mainand will have documentation conflicts to resolve there.