Skip to content

feat: shared injection template registry for platform credential proxy support#21970

Closed
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1774644627-slack-credential-injection-platform
Closed

feat: shared injection template registry for platform credential proxy support#21970
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1774644627-slack-credential-injection-platform

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented Mar 27, 2026

Summary

Enables platform-hosted assistants to use the Slack skill's MITM proxy credential injection by creating a shared injection template registry.

Problem: When a platform-hosted assistant stores a slack_channel:bot_token credential via the HTTP /v1/secrets route, handleAddSecret called upsertCredentialMetadata(service, field, {}) with an empty policy — no injection templates were written. The injection templates (which tell the proxy how to inject the token into outbound requests) were only defined inside the daemon's config-slack-channel.ts handler, which never runs on platform assistants. This meant resolveCredentialRef("slack_channel/bot_token") would find metadata but with no injection templates, so the proxy had nothing to inject.

Solution: A new injection-registry.ts module provides a static registry mapping well-known service/field pairs to their default injection templates. Both handleAddSecret (HTTP secret route) and ensureBotTokenInjectionTemplates (daemon handler) now consult this registry, so injection metadata is written regardless of which code path provisions the credential.

Review & Testing Checklist for Human

  • Verify the registry entry matches the previously-hardcoded templates exactly — compare INJECTION_REGISTRY["slack_channel/bot_token"] in injection-registry.ts against the removed SLACK_INJECTION_TEMPLATES constant in config-slack-channel.ts. The hostPattern, injectionType, headerName, and valuePrefix must be identical.
  • Check the if (entry) guard in ensureBotTokenInjectionTemplates — if the registry entry were ever removed, this function silently becomes a no-op. Consider whether that's acceptable or if it should log a warning.
  • End-to-end test: Provision a slack_channel:bot_token credential on a platform-hosted assistant via the /v1/secrets HTTP endpoint (e.g. POST { type: "credential", name: "slack_channel:bot_token", value: "xoxb-..." }), then verify the metadata store contains injection templates and the proxy correctly injects the Bearer header on requests to slack.com.

Notes

  • The 3 pre-existing test failures in secret-routes-managed-proxy.test.ts reproduce on main and are unrelated to this change (test isolation issue with provider registry state).
  • This PR is necessary but not sufficient for full platform Slack support — the platform backend still needs a UI/API path to actually send the slack_channel:bot_token credential to the assistant's /v1/secrets endpoint. This PR ensures the assistant-side handling is correct when that credential arrives.

Link to Devin session: https://app.devin.ai/sessions/c05d7f266b7e45eb8db890aaffcc8b05
Requested by: @emmiekehoe


Open with Devin

…y support

Create a centralized injection template registry that maps well-known
credential service/field pairs to their default injection templates.
This enables platform-hosted assistants to use the Slack skill with
MITM proxy credential injection — previously only personal assistants
could use this because the daemon handler was the sole source of
injection template metadata.

Changes:
- New injection-registry.ts with Slack bot token injection templates
- handleAddSecret now applies registry templates when storing credentials
- config-slack-channel.ts delegates to shared registry instead of hardcoding
- Updated Slack skill compatibility to include platform-hosted assistants

Co-Authored-By: emmie@vellum.ai <emmiekehoe@gmail.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@emmiekehoe emmiekehoe marked this pull request as draft March 27, 2026 21:05
@emmiekehoe emmiekehoe requested a review from m-abboud March 27, 2026 21:08
@emmiekehoe emmiekehoe closed this Apr 20, 2026
ashleeradka pushed a commit that referenced this pull request May 28, 2026
Per the Electron docs and the upstream `#21970` thread on
`setActivationPolicy`, the documented sequencing is to await
`dock.show()` (the Promise resolves once the Dock has reflected the
change) before calling `setActivationPolicy("regular")` — so the two
surfaces transition in lockstep.

The previous `void app.dock.show()` was probably harmless in practice
(macOS reconciles), but matching the documented pattern is cheap and
removes one class of "why did the dock briefly look wrong on a fast
toggle?" question. The `accessory` direction is synchronous on the
Electron side (`hide()` is void), so no await there.

`applyPolicy` is now async; the two callers are non-awaiting (the
debounced refresh and the install-time bootstrap have nothing to
await on), so they `void` the returned Promise — annotated in
comments at each site.
ashleeradka added a commit that referenced this pull request May 28, 2026
…32461)

* feat(macos): Dock unread badge + visibility state machine (LUM-1966)

Mirrors what the Swift app does today:

- Unread conversation count → `app.dock.setBadge` (1–99 pass through,
  truncate to "99+" beyond, "" to clear). Matches Swift Vellum's
  `unseenVisibleConversationCount` formatter in
  `clients/macos/.../AppDelegate+WindowsAndSurfaces.swift`.
- Visibility state machine: any visible window OR signed-in →
  `regular` (Dock icon shown); no visible window AND signed out →
  `accessory` (Dock hidden, menu-bar-only). Debounced ~100ms so a fast
  close-then-open doesn't flash the icon. Cleared on `before-quit`.

`src/main/dock.ts` owns the state and the two IPC handlers
(`vellum:dock:setBadge`, `vellum:dock:setSignedIn`). `installDock()`
is called from `whenReady`, wires `browser-window-created` for
window-count observation, and is idempotent against hot-reload.

The `accessory` transition is gated on `ALLOW_ACCESSORY_MODE = false`
until the tray icon from LUM-1965 lands. Going accessory before that
would hide the Dock icon with no replacement entry point, leaving the
user no way to bring the window back. The whole state machine is in
place, ready to flip with one constant change when tray ships.

Bridge + renderer:

- `apps/macos/src/preload/index.ts` — `dock.setBadge` /
  `dock.setSignedIn` on the typed `VellumBridge`.
- `apps/web/src/runtime/is-electron.ts` — ambient declaration mirrored
  on `window.vellum.dock`.
- `apps/web/src/runtime/dock.ts` — per-capability wrapper following the
  `native-biometric.ts` pattern (no-ops off Electron, so callers don't
  need an `isElectron()` guard at every call site).
- `apps/web/src/hooks/use-electron-dock-sync.ts` — derives the unread
  count from `conversations.filter(c =>
  c.hasUnseenLatestAssistantMessage)` (same predicate Swift uses) and
  reads `isLoggedIn` from `useAuthStore`. Two `useEffect` calls publish
  to the bridge on change.
- `ChatLayout` mounts the sync hook once; the conversation list it
  already subscribes to is the input, so there's no extra query.

When LUM-1924 wires BFF auth into main, `setSignedIn` becomes a no-op
the renderer drops — that side of the bridge is documented as
temporary.

* refactor(web): move use-electron-dock-sync into chat domain

CONVENTIONS.md rule: "Used by exactly one domain → live inside that
domain. Used by two or more domains → lift to the top-level shared
dir." The hook is mounted only in ChatLayout (it derives the unread
count from `conversations`, which only chat holds), so it belongs in
domains/chat/hooks/ — not the top-level cross-domain hooks/.

No behavior change. If a second domain (intelligence, library, etc.)
later needs to publish to the Dock, lift it then.

Imports stay clean: the hook still only reaches into shared
top-level dirs (@/runtime/dock, @/stores/auth-store,
@/types/conversation-types), so no peer-domain coupling.

* docs(macos): scrub internal-tracker refs from dock comments + docs

This repo is open-source; in-tree comments and the README shouldn't
reference internal tracker IDs. Reworded the four "until LUM-XXXX
lands" / "until the tray ships in LUM-XXXX" notes in dock.ts,
preload, runtime/dock.ts, the chat-domain sync hook, and the README
to describe the future condition itself ("once main owns auth state
directly", "until a menu-bar entry point exists") rather than the
tracker ID.

Also fixed a stale module path in the chat-layout comment block left
behind by the previous refactor (the hook moved into the chat
domain).

* fix(macos): exclude background threads + clear Dock badge on signout

Two Codex review findings:

1. **Background / scheduled / archived conversations were counting
   toward the Dock badge.** The hook was using the raw
   `hasUnseenLatestAssistantMessage` flag, but the existing
   `isBackgroundConversation` predicate already classifies the
   automated thread types (background, scheduled, system:background,
   system:scheduled) the Swift app excludes via
   `shouldSuppressGlobalUnreadAggregations`. Promoted the right
   filter into a named `contributesToUnreadCount` predicate in
   `utils/conversation-predicates.ts` (so sidebar-attention and
   dock-badge derivations stay aligned) and the hook reduces over it.

2. **Dock badge was stale on signout.** The hook only published
   `setSignedIn(false)` on logout, and the auth-scoped layout
   unmounted before `unreadCount` could update — so
   `setDockBadge(0)` was never called and the badge persisted on the
   signed-out app until quit or a later count update. Moved the
   clear into main: when `setSignedIn` flips from true → false, the
   handler resets `badgeCount` and applies it synchronously, ahead
   of the debounced policy refresh. That side-steps the
   hard-navigate-destroys-renderer race entirely — main keeps
   running, main owns the dock surface.

* fix(macos): await app.dock.show() before flipping activation policy

Per the Electron docs and the upstream `#21970` thread on
`setActivationPolicy`, the documented sequencing is to await
`dock.show()` (the Promise resolves once the Dock has reflected the
change) before calling `setActivationPolicy("regular")` — so the two
surfaces transition in lockstep.

The previous `void app.dock.show()` was probably harmless in practice
(macOS reconciles), but matching the documented pattern is cheap and
removes one class of "why did the dock briefly look wrong on a fast
toggle?" question. The `accessory` direction is synchronous on the
Electron side (`hide()` is void), so no await there.

`applyPolicy` is now async; the two callers are non-awaiting (the
debounced refresh and the install-time bootstrap have nothing to
await on), so they `void` the returned Promise — annotated in
comments at each site.

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

1 participant