Commit Graph
3 Commits
Author SHA1 Message Date
Dotta 46760cc588 fix(chat): honor dismissal for all composer requests (#17218)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Task conversations show requests in the feed and above the message
composer.
> - Earlier fixes kept dismissed questions in history, but excluded
completion reviews and other request kinds.
> - Closing those forms left a pending badge, and the server could still
demand a response.
> - This pull request saves human dismissal and removes the request from
composer reminders and ordinary completion gates.
> - People can continue the conversation and reopen the request from
history.

## Linked Issues or Issue Description

Related work: #15229 and #15392.

**What happened?**

Dismissing a completion review closed its form but left it counted as
pending. It could return after reload, block a retry, or cause the agent
to demand another response. Raw runtime requests could also lose their
only usable control when dismissed.

**Expected behavior**

Dismissal leaves an accessible request in the feed and clears the
composer form and pending count. It persists after reload. It does not
submit an answer, approve an action, or wake the agent.

**Steps to reproduce**

1. Open a task with a pending completion review or another composer
request.
2. Dismiss the request, then reload the task.
3. Check that the composer stays clear and the feed entry can reopen the
request.

## What Changed

- Add nullable dismissal fields and a human-only dismissal endpoint with
company, private-task access, resolver, and locked review-policy checks.
- Persist saved-request dismissal on the server. Keep browser
persistence for raw runtime requests and existing question dismissals.
- Keep confirmations, choices, item verdicts, suggested tasks, and
runtime requests accessible in the feed.
- Exclude dismissed ordinary requests from completion and retry gates.
Tell agents not to repeat deferred requests.
- Preserve tool, secret, connection, plan, company-policy, and
configured review authorization gates. Expire deferred non-question
requests when their task completes.
- Preserve the selected form when another dismissal finishes saving.
Keep reopened reviews visible through delayed comment refreshes.
- Add a completion-review Storybook test drive and update design, API,
database, and capability documentation.

## Verification

- `pnpm -r typecheck`: passed. The final UI changes also passed the UI
typecheck.
- `pnpm build`: passed. The final UI changes also passed the UI
production build.
- Focused UI and transcript tests: 550 passed across nine files after
merging the current base branch.
- Focused database, completion, and prompt tests: 105 passed across
three files, including two review-policy regressions that failed before
the fix. The tests reject a review requester excluded by current policy,
preserve the pending gate on denial, and permit an independent reviewer
to defer the request.
- Route regression suite: 81 passed, including denial before reading or
mutating a private-task request.
- `pnpm check:token-gates`: passed.
- Map and Storybook structural checks: passed; 30 map/catalog script
tests passed.
- Browser: used the production task-thread components in the
completion-review fixture. Dismissed, reloaded, reopened from the feed,
and dismissed again. No pending badge or composer takeover remained.
Story callbacks simulate API saves; separate server tests cover
persistence and permissions.
- `pnpm test:run`: stopped after the general-server group reported one
failure in `workspace-runtime.test.ts`: "returns a bounded structured
conflict and exposes only same-company workspace references". The same
test passed in isolation (1 passed, 178 skipped). The cause of the
full-suite failure is not established. The full PR CI suite subsequently
passed on e243cbb001; its two authorization review findings are fixed in
9e1b4478b3. Fresh [full
CI](https://github.com/paperclipai/paperclip/actions/runs/39871944846)
passes on 9e1b4478b3, including the previously failing server and
browser shards. All 54 executed checks passed; two optional Storybook
deployment/visual jobs were skipped. The legacy security status also
passed. Fresh Greptile review is 5/5, the security scan passes, and no
review threads remain open. This full CI run settles the merge gate; the
earlier local workspace-suite failure remains recorded above because its
cause was not established.
- Additional verification: the original failing browser journey passed
locally after the delayed-refresh fix. The expanded page-reload case
also passes on 9e1b4478b3 (52.7s): dismiss, reload, reopen, approve, and
receive the actual tool failure in the continuing task. It exposed a
fixture reading an accepted request before tool execution finished; the
fixture now waits for a stored execution result. The first reload
attempt stopped on a still-loading page at the default five-second
assertion; the reload now uses the suite’s navigation allowance. A
controlled server-restart run timed out waiting for server health before
the UI assertions. Local route-suite runs timed out during concurrent
module loading; sequential imports passed all 81 tests, including the
privacy regression.
- Initial verification failures: a missing SDK build and a story fixture
type error were fixed. PostgreSQL bootstrap then failed because the host
had exhausted its semaphore allowance. After explicit operator approval,
64 verified abandoned PostgreSQL semaphore sets were reclaimed; no live
process was stopped. The database regressions then passed. The
raw-runtime regression exposed a missing feed control; the
implementation now preserves that entry and the regression passes.

## Risks

- Apply the additive database migration before the new server code. It
adds two nullable columns and does not change existing decisions.
- A dismissal is shared task attention state. Only a human with task
read access and current resolver/review authority can persist it. Review
policy is checked under the task lock. The operation is idempotent and
records the dismissing user.
- Raw runtime dismissal remains specific to a browser. Saved interaction
dismissal persists across browsers.
- A deferred review is not an approval. Tests retain protected-action
and configured review gates.
- The browser walkthrough uses a fixture. This branch has not been
deployed to a live instance.

## Model Used

OpenAI Codex, GPT-6, with code execution and browser tools. The exact
backend model ID and context-window size are not exposed in this
session.

## 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
2026-10-22 18:37:36 -05:00
DottaandPaperclip 00d944f96c fix(chat-channels): resume follow-ups after Slack Stop (#15737)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Chat channels let verified users stop an agent and send another
message from Slack.
> - Native execution needs the stopping user's identity to recognize a
deliberate Stop.
> - Slack omitted that identity, so a stopped run could create a
recovery hold.
> - A pending AI login request for another user could then block the new
message.
> - This pull request records the verified stop actor and scopes AI
login waits to the current user.
> - Users can start a fresh turn after a verified Stop while access and
approval gates remain in force.

## Linked Issues or Issue Description

**What happened?**

A user stops a native run from its bound Slack thread. Slack confirms
the Stop. The user sends a follow-up and receives a queued
acknowledgement, but execution does not start. The run can acquire a
recovery hold. A pending AI login request addressed to another user then
blocks explicit continuation.

**Expected behavior**

An acknowledged operator Stop ends the current response. A new
authorized user message starts through normal admission after the
previous execution has stopped. An AI login request for another user
stays pending but does not gate this user's turn. Tool access,
approvals, unknown identities, and unconfirmed cleanup still block
execution.

**Steps to reproduce**

1. Start a native agent run in a Slack thread with a pending AI login
request addressed to a different user.
2. Stop the run with the signed Slack Stop action.
3. Send a new message from a verified linked user.
4. Observe the recovery hold and the pending-decision message on the
saved follow-up.

Refs: #15421 for the related verified linked-user authority path. That
change separates guest and trusted tasks; this change handles
cancellation and login wait scope.

## What Changed

- Pass the verified Slack user's identity to canonical run cancellation.
- Scope AI login waits by the current run's responsible user, with a
company-scoped lookup.
- Use the same wait predicate for explicit continuation, native
execution, and recovery decisions.
- Admit already-saved native follow-ups posted after Stop through the
existing continuation checks. Keep pre-stop and delivered messages held.
- Keep pending cards and their authorization state intact.
- Add regression tests for signed Slack Stop, fresh-turn admission, and
same-user, unknown-user, tool-access, and company-boundary gates.
- Give the two affected database setup hooks the standard 90-second
budget.
- Document Slack Stop and user-scoped AI login waits.

## Verification

- Red: signed Slack Stop omitted the cancelling user; direct
continuation and already-saved post-Stop continuation both returned
`null`.
- Green: 57 continuation/audience checks passed, including legacy
operator Stops, pre-stop input, delivered input, same-user login,
unknown identity, company boundaries, cleanup, ownership, pauses, and
approvals.
- The combined signed Slack Stop and original gate check passed: 39
tests.
- Existing native question/completion suite: 33 tests passed.
- `pnpm -r typecheck` passed.
- `pnpm build` passed.
- Rebased regression check: 18 tests passed on current master. The
reviewed database-support setup change passed all 7 audience checks.
- Full latest-head CI passed, including all server, chat, package,
serialized-server, Runner, browser, build, and canary gates.
- The duplicate local `pnpm test:run` was stopped after the full CI
matrix passed. It was slow on the contended host; no failure had been
reported. The focused reproduction command is:

```sh
pnpm exec vitest run server/src/services/issue-question-context.test.ts server/src/services/explicit-native-continuation.test.ts server/src/__tests__/chat-channels.integration.test.ts --fsModuleCache --testTimeout 60000 -t 'AI connection wait audience|scopes connection waits|resumes an already-saved follow-up|durably retries a signed Slack session stop'
```
- Manual evidence: inspected the bound thread's cancelled run, queued
messages, recovery hold, and login card. The staging deployment has not
been changed.

## Risks

- AI login wait behavior changes only when both the addressee and
current responsible user are known and differ. Missing identity retains
the gate.
- Starting a new turn does not grant access or answer the old card.
Credential selection still uses the new run's authority.
- Process, remote cleanup, recovery, task ownership, pause, budget, and
approval checks remain in place.
- No schema migration or public API change.

## Model Used

OpenAI GPT-6 through Codex, with reasoning, repository inspection,
browser inspection, and code execution. The exact runtime model ID and
context window are not exposed in this session.

## 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-09 16:47:21 -05:00
DottaandPaperclip faa8e452c7 fix(tasks): stop repeated reminders for historical questions (#15392)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Task questions remain saved so a person can answer them later.
> - The composer moves an old question into history after a newer human
message.
> - Native completion still treated every pending question as a required
response.
> - This caused agents to demand an old answer after the person moved
work forward.
> - This pull request shares the historical-question rule across context
and task execution.
> - Agents can finish verified work while the original question remains
answerable.

## Linked Issues or Issue Description

Refs #15229, Refs #14613, Refs #13130.

**What happened?**

An agent repeatedly asked a person to answer a question that had moved
into feed history. Completion feedback explicitly told the agent to
request a response. The saved pending row also blocked task completion.

**Expected behavior**

A question before newer human direction remains answerable in the feed.
Its pending state alone must not require another reminder or stop
completed work. A new input blocker, an approval, or a configured review
stage must keep its gate.

**Steps to reproduce**

1. Let a task agent create an ordinary question.
2. Dismiss the question and send a newer task message.
3. Let the agent finish the requested work and submit its completion
report.
4. Observe a demand to answer the old question and a retained completion
gate.

## What Changed

- Add one company-scoped predicate for historical questions. Only later
human comments count. Exclude agent attribution, run attribution, system
notices, and untrusted source data.
- Apply the predicate to completion feedback, native waits,
finalization, commit validation, retry validation, blocked routing, and
successful-run handoff.
- Include question classification and guidance in heartbeat context and
both native task-context tools. Add the guidance to fresh and resumed
task prompts.
- Replace automatic reminders for current ordinary questions with
instructions to assess the real blocker, continue independent work, and
withdraw obsolete questions through the existing API.
- Preserve historical question rows during completion while cancelling
their live native source runs through the existing post-commit and
recovery paths. Let an authorized human answer them after completion
without reopening work or creating a response wake. Preserve
cancellation, current-input, approval, permission, credential,
connection, and review gates. Add no dismissal storage or migration.
- Document the rule in the execution contract and agent skill. Refresh
generated capability source anchors. Add database-backed status,
context, attribution, and governance regression tests.

## Verification

- `pnpm build` passed. The server rebuild also passed after the
lifecycle fix. Generated capability contract and inventory checks passed
after the agent documentation update.
- `pnpm -r typecheck` passed. Final `pnpm --filter @paperclipai/server
exec tsc --noEmit` also passed after the last test additions.
- Lifecycle and interaction regressions passed: 224 tests in 3 suites.
Context and prompt tests also passed. Final historical-question cases
passed (33 tests), native cancellation/recovery cases passed (6 tests),
and the existing interaction/confirmation suites passed (72 tests). The
full local `pnpm test:run` was attempted and stopped after more than two
hours with unrelated fixture/hook timeout failures; it did not pass. All
52 successful GitHub checks are green on the latest commit, including
the complete test matrix; no checks are pending or failing. Greptile is
5/5 and both review threads are resolved.
- Regression cases cover the old-question/new-human-message sequence,
final task status, answering after completion with no wake, live-run
cancellation and crash recovery, current input blockers, both
task-context tools, API context, timestamp precision, attribution
boundaries, and protected gates.

## Risks

- A later human task message makes an earlier ordinary question
historical even if its input is still missing. The agent must identify
the current blocker and ask only for information that still prevents
work.
- Browser dismissal remains a local preference. Dismissal without a
later human message is not recorded by this change.
- No schema change or data migration. Completion retains ordinary
historical questions; cancellation still expires them. A completed task
accepts historical answers only from an authorized human and creates no
response-delivery outbox row. Governed requests retain their gates.

## Model Used

OpenAI Codex, GPT-6, with repository editing, code execution, and
browser diagnostics. The exact deployment model ID and context-window
size are not exposed in this session.

## 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-06 20:33:13 -05:00