Commit Graph
2 Commits
Author SHA1 Message Date
DottaandPaperclip 71af2fbc3b fix(slack): teach agents how people connect their accounts (#15576)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Slack connections let people talk to an agent with their own
Paperclip permissions.
> - Each bot has a saved command that starts account linking.
> - The Access page explains this flow, but Slack agent turn context
omitted it.
> - An agent could guess the command or confuse channel membership with
Paperclip access.
> - This pull request gives agents the saved command and account
confirmation instructions.
> - People can ask the agent how to join without changing access
controls.

## Linked Issues or Issue Description

**What happened?**

Slack agent context explained messages, tools, and questions. It did not
explain how a teammate can connect an account. The saved slash command
can differ from the current agent name.

**Expected behavior**

Give the saved bot command, such as `/research_ops connect`. The
teammate runs it themselves. They sign in through a private confirmation
link. A new company member must receive admin approval before account
confirmation. The connection manager can copy instructions from Access →
Invite people.

**Steps to reproduce**

1. Configure a Slack bot with a custom slash command.
2. Rename its assigned agent.
3. Inspect a fresh or resumed Slack task prompt. Before this change, it
contains no account invitation instructions or saved connect command.

**Paperclip version or commit**

Base: `d6df12cef69fcaf2d2fe66a393168931d5b8b4e7`.

**Deployment mode**

Applies to local and hosted Slack chat connections. Deterministic tests
used a local isolated database. The new model probe has not run against
a live Slack bot.

Related work: Refs #15413 and #13638. This fixes agent guidance for
their existing account-linking flow. It adds no new membership system or
invitation endpoint.

## What Changed

- Read only the saved public slash command from the company-scoped
conversation endpoint. Supply it only to the assigned agent for Slack
turns with an active or verifying connection.
- Validate the command with the shared Slack configuration schema. Refer
to Access → Invite people when it is missing or invalid. Never guess
from the current agent name.
- Explain personal account confirmation, link expiry, and company
membership approval on fresh and resumed turns. Distinguish channel
invitations from Paperclip access.
- Add deterministic guidance regressions, a manual invitation model
probe, and setup documentation.

## Verification

- Passed: 63 tests in `heartbeat-context-summary.test.ts` and
`heartbeat-chat-task-link.test.ts`.
- Passed: four real-heartbeat regressions in
`heartbeat-slack-invitation.test.ts`. They check the saved command after
an agent rename, a persisted resumed session, full and compact prompts,
missing-command fallback, inactive endpoints, and a different assigned
agent. They also reject caller-supplied command fields and exclude other
setup metadata.
- Passed: the isolated `chat-channels.integration.test.ts` case
`discovers a Slack connect identity without starting work or granting
access`. It covers the private link, duplicate connect requests, and
nonmember access requests without a membership grant.
- Passed: `pnpm --filter @paperclipai/server typecheck` and `pnpm
--filter @paperclipai/server build`.
- Passed: `pnpm test:slack-connector --list` and `git diff --check`.
- Passed: repository `pnpm -r typecheck`, `pnpm build`, and `pnpm
check:token-gates`. Typecheck required local IPC access for the
migration check.
- Passed: final-commit CI, with 54 successful checks and two optional
Storybook checks skipped. CI includes all server, chat, workspace,
serialized, Runner, and browser test shards, typecheck, build, and
canary dry run.
- The full local `pnpm test:run` was started. It was stopped after full
CI passed; it had no final local summary. The focused invitation checks
passed locally. Do not count the interrupted local run as a full-suite
pass.
- Greptile gave the exact final commit
`f37e52bde621ef7f5b2bb345074d53345f8e4ee9` a 5/5 score. Both review
findings were fixed and their threads resolved.
- The new `invite-person` model probe is manual. These deterministic
results do not establish a live model or Slack acceptance pass.

## Risks

- Model guidance cannot prove that a person joined. The existing
account-linking and membership checks remain authoritative.
- Legacy rows without a saved command use the Access page fallback.
Invalid command text is excluded from the prompt.
- The query selects only the public command. It does not expose
registration secrets, tokens, or personal confirmation links.
- No schema changes, new provider requests, permission grants, or
telemetry changes.

## Model Used

- OpenAI GPT-6 through Codex. The exact backend variant and context
window are not exposed in this session. Used reasoning, repository
inspection, code editing, and test execution.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-08 10:28:14 -05:00
DottaandPaperclip 51653becc2 refactor(server): extract heartbeat task markdown rendering (#15568)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Heartbeat orchestration supplies each run with task context.
> - The task Markdown renderer is inside a service with more than 31,000
lines.
> - The renderer formats task data and does not write run state.
> - It is a small first step toward a more manageable heartbeat service.
> - This PR moves the renderer without changing its function body or
public export.
> - Direct tests and existing caller tests check the output before and
after the move.

## Linked Issues or Issue Description

**What existing behavior does this improve?**

This improves the structure and test coverage of heartbeat task context
rendering.

**Subsystem affected**

server/ — orchestration services.

**Current behavior**

`buildPaperclipTaskMarkdown` occupies 352 lines inside `heartbeat.ts`.
The service also owns run execution, scheduling, recovery, and
cancellation.

**Proposed behavior**

Move the renderer to `heartbeat/task-markdown.ts`.
Keep its function body and the existing `heartbeat.ts` export unchanged.
Leave orchestration in place for this first PR.

**Reason and benefit**

Prompt formatting has its own small source file and direct regression
tests.
Reviewers can check this one extraction before a later refactor.

**Breaking changes**

None. The input type, rendered output, and existing import path stay the
same.

Related renderer changes: Refs: #4732, Refs: #14030.
These PRs change prompt behavior. This PR only moves the current
renderer.

## What Changed

- Move the 352-line renderer to
`server/src/services/heartbeat/task-markdown.ts`.
- Import and re-export it from `heartbeat.ts`. Remove imports used only
by the renderer.
- Add seven direct regression cases. Cover empty context, ordered
comment-only wakes, input preservation, nested code fences, ancestor
limits, attachment-only wakes, and rejected plans.
- Keep the renderer and its direct tests in
`server/src/services/heartbeat/`. Document this folder as the home for
relevant later extractions in `doc/DEVELOPING.md`.

## Verification

- The focused suite passed before and after extraction, and after moving
to the heartbeat folder: four files, 85 tests.
- Run `pnpm exec vitest run
server/src/services/heartbeat/task-markdown.test.ts
server/src/__tests__/heartbeat-context-summary.test.ts
server/src/__tests__/heartbeat-chat-task-link.test.ts
server/src/__tests__/codex-local-execute.test.ts`.
- The extracted function matches the original function byte for byte.
The folder move only adjusts imports.
- `node scripts/check-module-boundaries.mjs` passed.
- `git diff --check` passed.
- The initial extraction passed local `pnpm -r typecheck` and `pnpm
build`.
- The folder update passed `pnpm --filter @paperclipai/server exec tsc
--noEmit` and `pnpm --filter @paperclipai/server build`.
- The initial extraction passed the full CI suite. Its duplicate local
`pnpm test:run` was stopped after CI passed; it did not complete
locally.
- The folder update passed all CI gates on commit
`e21e589b379d7ca2fae16c1dcd91cf2f604fd03f`: 54 passing checks, two
skipped Storybook checks. Three test shards passed after one retry
following simultaneous runner shutdowns; those failures had no failed
test assertions.
- Fresh Greptile review of commit
`e21e589b379d7ca2fae16c1dcd91cf2f604fd03f`: 5/5, no actionable or inline
findings.

## Risks

- Low risk. A moved module can change import resolution or expose an
import cycle.
- Existing caller tests still load the compatibility export from
`heartbeat.ts`.
- The same guidance constants and public task URL resolver remain in
use.
- No run-state writes, transactions, locks, shared process state, or
cleanup paths moved.
- No database, API, telemetry, or UI contracts changed.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

OpenAI GPT-6 via Codex. The exact serving model ID and context window
were not exposed in this session. Used reasoning, repository inspection,
shell tools, and code execution.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-08 09:06:04 -05:00