Commit Graph
6 Commits
Author SHA1 Message Date
DottaandPaperclip b508a05c43 feat: add internal agent complaints and suggestions (#15367)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agents use legacy skills or native runner tools to work on tasks.
> - Those agents can encounter friction that does not belong in the task
thread.
> - A complaint should preserve the raw reaction. A suggestion should
describe an improvement.
> - This pull request adds attributed local storage and both submission
paths.
> - Agents can submit feedback once and continue their primary work.

## Linked Issues or Issue Description

**Subsystem affected**

Server, database, shared contracts, runtime skills, and native runner
tools.

**Problem or motivation**

Agents have no default internal channel for incidental complaints and
suggestions. Sending this feedback through task comments adds noise and
can alter task workflows.

**Proposed solution**

Store free-form feedback in the current instance database. Derive agent,
run, company, and task attribution from active authority. Provide
default legacy skills and provider-neutral native actions. Keep the
instructions close to Warp's MIT-licensed originals.

**Alternatives considered**

Task comments and external Slack delivery add unwanted side effects.
Mandatory suggestion fields and short editorial limits would discard
useful feedback. This release has no listing API, UI, read tool,
automatic triage, or external forwarding.

**Roadmap alignment**

This is a maintainer-requested addition to the existing runtime skills
and runner tool paths. It does not duplicate a listed roadmap milestone.
Searches for complaint tooling, suggestion-box, and agent commentary
found no overlapping public PR or issue.

## What Changed

- Add the company-scoped `agent_commentary` table, shared validation,
and idempotent migration `0310`.
- Add one transactional service and the agent-only POST route. Validate
active authority before writes or replay. Redact known credentials.
Commit a content-free audit with each new record.
- Add `submit_complaint` and `submit_suggestion` to standard, ask, and
planning modes. Keep review, revocation, and completion restrictions.
Store replay identity on the commentary row.
- Mount `complain` and `suggestion-box` by default for legacy agents.
Bundle a dependency-free Node.js stdin helper in the operational skill
and allow its POST through the sandbox bridge.
- Preserve Warp's complaint voice and suggestion guidance, with
attribution and local transport adaptations. Keep source attribution and
MIT notices in each skill's LICENSE, outside runtime instructions.
- Document custom-runtime HTTP use and database inspection. Add
real-database tests and a repeatable live Codex smoke for local and
Daytona execution.
- Pin the lagging-source migration fixture before the identity-repair
migration so later migrations preserve its regression coverage.

## Verification

- Personally ran real Codex submissions in all four environments on
2026-10-06. Local runs passed at 20:35 UTC. Daytona native passed at
20:31 UTC; Daytona legacy passed at 20:33 UTC. Each stored exactly two
rows with company, agent, run, and task attribution, wrote the
continuation marker, exited zero, created no task comments, and left
task status unchanged. Each recorded two content-free activity entries.
- Daytona used production provider hooks, real remote execution and file
transfer, the legacy queue callback bridge, and native private WebSocket
ingress. The current Linux runner was built from `abf47b595`, staged,
and verified against controller contracts. Both sandboxes were confirmed
deleted. This is a focused feedback transport smoke; it does not claim
full Runner E2E catalog or browser qualification.
- The immutable base image and Linux binary digest are recorded in [the
verification
documentation](https://github.com/paperclipai/paperclip/blob/codex/agent-commentary/doc/agent-commentary.md#verification).
The smoke script can save content-free JSON evidence. No credentials or
feedback bodies are in these reports.

| Environment | Runner | Complaint row | Suggestion row |
| --- | --- | --- | --- |
| local | legacy Codex | `59413a00-1de2-4bb1-bcc6-9c4b54c64aa6` |
`3db2364d-3e15-4f47-846f-875d3902999d` |
| local | native Codex | `5da22b5f-41df-4de5-8ba0-d9345ab01267` |
`2d5abe17-dd41-403c-a5ee-4729f2d58921` |
| daytona | legacy Codex | `27c9d0aa-8477-409f-9da0-e8ffa48dee50` |
`209681c9-d1e9-4ce1-999e-48fa07692389` |
| daytona | native Codex | `6eb001bb-4bcf-43f7-8717-f662f53dc7c3` |
`77c383d8-a997-49e5-a33e-25c70e15c0b2` |

- Run the local check with `node cli/node_modules/tsx/dist/cli.mjs
server/scripts/verify-agent-commentary-live.ts`. The documentation gives
the Daytona invocation. Both use disposable instance databases and
normal Codex provider usage.
- Repository `pnpm -r typecheck` and `pnpm build` passed after the test
extension. The build includes runner generation, contracts, and replay
checks. The smoke scripts also passed a separate TypeScript check. The
lagging-source migration regression passed. All equivalent current-head
Vitest CI shards passed. The local monolithic `pnpm test:run` invocation
was stopped after CI supplied that coverage; it did not complete
locally.
- Focused tests cover company isolation, spoofing, revoked credentials,
stale ownership, post-finish rejection, concurrent replay, conflicting
keys, atomic rollback, and deletion through existing services. Boundary
tests cover empty text, Unicode, text beyond 8,000 characters, and the
524,288-character ceiling without truncation. Mounting tests cover
Codex, Claude, and sandbox staging. Helper tests cover standalone Node
execution, stdin, invalid UTF-8, redirects, HTTP failure, and its
deadline. Privacy and bridge tests cover successful and rejected
requests.
- Instructions were compared with Warp's originals. MIT notices and
source credits live only in LICENSE files. Native tools preserve
truthful disclosure when asked, without routine announcements.
- [Full
CI](https://github.com/paperclipai/paperclip/actions/runs/37508559190)
and Greptile 5/5 passed on the earlier feature commit `5209c3501`. The
later head found the migration-fixture assumption fixed in this update.
On `8a4965164`, all 55 check contexts passed after one browser shard
rerun. Its initial reviewer signoff failure also passed an isolated
local browser run (1 test). Greptile scored that head 5/5 and identified
one smoke cleanup gap. `6ecbafb0b` fixes failed-acquisition cleanup with
four passing tests and a passing smoke-script typecheck. Fresh CI is
pending for this final test-only fix. No commentary production code
changed during verification.

## Risks

- Feedback is internally attributed. It is not anonymous. Existing
redaction removes known credentials, but agents must still omit
sensitive content. Normal provider transcripts can include their
submitted arguments.
- Default skill availability changes for existing legacy agents. Runtime
policy filtering still applies. The helper uses the existing Node.js
runtime with no extra dependencies; custom runtimes can call the HTTP
endpoint.
- Feedback is removed with its run, agent, or company. Task deletion
clears only the issue pointer. Normal database backups include the
table.
- The migration is additive and has no backfill. Writes serialize on the
active run for replay consistency. No server suggestion quota is
imposed.

## Model Used

OpenAI `gpt-6-astra` through Codex, with `xhigh` reasoning effort and a
reported 258,400-token context window. Capabilities used: repository
inspection, code execution, and live runtime verification. No subagents
were used.

## 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
- [ ] All Paperclip CI gates are green
- [ ] 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 16:14:06 -05:00
DottaandPaperclip a9d0927fe8 fix(adapters): restore Paperclip skill for legacy runners (#12225)
<!-- Write all pull request text in Simplified Technical English
(ASD-STE100): short sentences, one instruction per sentence, simple
approved vocabulary, and the active voice. -->

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Legacy local adapters run agents that use the Paperclip skill for
the control-plane workflow.
> - PR #7029 removed the required-skill fallback and made runtime skill
selection depend only on stored preferences.
> - No migration or runtime fallback replaced that behavior for existing
agents or non-CEO agents.
> - PR #12138 added core skills to new CEOs, and PR #12147 added Claude
skill discovery. These changes did not mount the operational skill for
all legacy agents.
> - This pull request makes the operational skill a legacy adapter
runtime invariant. It keeps all other skills configurable.
> - The native runner stays unchanged because its protocol supplies the
control-plane contract.
> - The benefit is that new and existing legacy agents can always
operate through Paperclip.

## Linked Issues or Issue Description

Refs #7029

Refs #12138

Refs #12147

**What happened?**

A skill-capable legacy local agent could start without
`paperclipai/paperclip/paperclip`. This happened when the agent had no
stored skill preference. An explicit empty preference also removed the
skill. The agent then reported that the Paperclip skill was not
available.

**Expected behavior**

Every skill-capable legacy local adapter must mount the Paperclip
operational skill when the runtime inventory contains it. Optional
skills must remain configurable. The native runner must keep its current
protocol-based behavior.

**Steps to reproduce**

1. Create a non-CEO `codex_local` agent without `paperclipSkillSync`
preferences.
2. Start a legacy heartbeat.
3. Inspect the managed `CODEX_HOME/skills` directory.
4. Observe that the Paperclip skill is absent before this change.

**Paperclip version or commit**

The problem reproduces on `master` before this pull request. PR #7029
introduced the configured-only selection behavior.

**Deployment mode**

Local development and self-hosted legacy local adapters.

## What Changed

- Added a shared legacy skill resolver that always selects the canonical
Paperclip operational skill when it is available.
- Applied the resolver to direct adapter execution, ACPX execution,
skill snapshots, and persistent skill sync.
- Added Hermes skill materialization at sync and run boundaries.
- Aligned Cursor, Gemini, and OpenCode execution-time injection with the
configured child `HOME`.
- Made Hermes stop execution when another installation blocks the
required operational skill.
- Kept optional skills controlled by `paperclipSkillSync.desiredSkills`.
- Kept `paperclip_runner` on the configurable-only resolver.
- Added regression coverage for missing preferences, empty preferences,
each skill-capable legacy adapter, ACPX, Hermes, and native runner
isolation.
- Documented the legacy runtime invariant.

## Verification

- `pnpm -r typecheck` passed on the pushed commit.
- `pnpm build` passed on the pushed commit.
- The adapter utility regression suites passed: 236 tests.
- The changed server adapter suites passed: 48 tests across 12 files.
- The OpenCode adapter suite passed: 8 tests.
- The Hermes adapter suite passed: 7 tests.
- `git diff --check` passed.
- `pnpm test:run` is not clean on this macOS host. The command reported
failures in unchanged workspace and filesystem suites. An isolated rerun
of `company-skills.test.ts` and `company-skills-service.test.ts`
reproduced 11 failures because macOS resolved `/var/...` paths as
`/private/var/...`. The changed adapter suites pass independently.

## Risks

- This change deliberately makes the operational skill non-removable for
skill-capable legacy local adapters.
- Existing agents receive the skill on their next list, sync, or run
boundary. No database migration is required.
- The resolver does not create a skill when the runtime inventory does
not contain the canonical entry.
- Hermes aborts a run if another installation occupies the required
operational skill target.
- Hermes removes only an undesired Paperclip-owned symlink that still
points to the known Paperclip source.
- The native runner does not receive the legacy default.

> 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 Codex based on GPT-5. The exact serving model ID and context
window were not exposed. The agent used reasoning, tool use, 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-08-26 09:45:24 -05:00
Devin FoleyandPaperclip 33353ce62b feat(skills): remove bundled paperclip-dev skill and retire required skill attribute (#7029)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Local adapters (Claude, Codex, Cursor, Gemini, Grok, OpenCode, Pi,
ACPX) ship bundled "skills" — opinionated Markdown prompt bundles
materialized into the agent's runtime
> - One of those bundled skills, `paperclip-dev`, existed to let agents
develop Paperclip itself; it has now moved to its own external repo and
no longer belongs in the core tree
> - The adapter skill model also carried a `required` / `requiredReason`
attribute plus a `paperclip_required` `AdapterSkillOrigin` variant, all
of which only existed to mark bundled skills as non-optional in the UI
and adapter sync logic
> - With `paperclip-dev` gone, no bundled skill is "required" anymore,
and the type / runtime surface for `required` is dead weight — but it is
computed at request time and never persisted, so a clean removal is safe
(no compatibility shim needed)
> - This pull request deletes `skills/paperclip-dev/` and removes every
trace of the `required` / `requiredReason` field and the
`paperclip_required` origin across shared types, validators,
adapter-utils, all eight local adapters, server routes, the
company-skills service, the UI, the storybook fixtures, and the test
suite
> - The benefit is a smaller, simpler adapter-skill surface: one origin
(`company_managed`) for managed bundled skills,
`resolvePaperclipDesiredSkillNames` collapses to "just the configured
desired set", and the AgentDetail skills tab no longer renders a
"Required by Paperclip" section that no longer applies

## Linked Issues or Issue Description

<!-- No existing public GitHub issue; describing the underlying work
inline (feature_request template fields). -->

**Summary**

Remove the bundled `paperclip-dev` skill (now maintained in its own
external repo) and retire the `required` / `requiredReason` skill
attribute and the `paperclip_required` skill origin, which only existed
to support it.

**Problem or motivation**

`paperclip-dev` is the only bundled skill that was ever marked
"required". Now that it lives in a separate repository, shipping it
inside the core tree is wrong, and the entire `required` surface (a type
field, a validator field, a synthesized `paperclip_required` origin, UI
"Required by Paperclip" section, and required-skill merging in the
desired-skills calculation) becomes dead weight. The `required` value is
computed at request time and never persisted, so it can be removed
cleanly without a migration or compatibility shim.

**Proposed solution**

Delete `skills/paperclip-dev/`, drop the `required` / `requiredReason`
fields and `paperclip_required` origin everywhere they are produced or
consumed, collapse managed-skill origin to a single `company_managed`
value, and simplify `resolvePaperclipDesiredSkillNames` to return only
the configured desired set.

**Alternatives considered**

Keeping the `required` attribute as a no-op for forward compatibility —
rejected because it is request-time only (nothing persists it), so
leaving it in place is pure dead surface area with no callers.

**Roadmap alignment**

Internal cleanup / dead-code removal that simplifies the adapter-skill
surface; it does not introduce or duplicate any planned core feature in
ROADMAP.md.

## What Changed

- Deleted bundled `skills/paperclip-dev/` (moved to a separate repo).
- Dropped `required`, `requiredReason`, and the `paperclip_required`
origin from `packages/shared/src/types/adapter-skills.ts`,
`packages/shared/src/validators/adapter-skills.ts`, and
`packages/adapter-utils/src/types.ts`.
- In `packages/adapter-utils/src/server-utils.ts`: removed
`readSkillRequired()`; dropped `required`/`requiredReason` from
`listPaperclipSkillEntries()`,
`normalizeConfiguredPaperclipRuntimeSkills()`,
`buildPersistentSkillSnapshot()`, and `PaperclipSkillEntry`; collapsed
`buildManagedSkillOrigin()` to always return `company_managed`;
simplified `resolvePaperclipDesiredSkillNames()` to return only the
configured desired set (signature preserved so adapter call sites are
untouched).
- Walked all eight local adapters (`acpx-local`, `claude-local`,
`codex-local`, `cursor-local`, `gemini-local`, `grok-local`,
`opencode-local`, `pi-local`) and removed every remaining
`requiredReason` / `paperclip_required` reference.
- `server/src/services/company-skills.ts`: dropped the `required =
sourceKind === "paperclip_bundled"` synthesis when listing runtime skill
entries.
- `server/src/routes/agents.ts`: removed required-skill merging from the
desired-skills calculation in the persist-config path and the
unsupported-snapshot path (keeping the current version-aware
`desiredSkillEntries` structure).
- `ui/src/pages/AgentDetail.tsx`: dropped required-based filters, the
required tooltip, and the entire "Required by Paperclip" section from
the agent skills tab; storybook fixtures in
`ui/storybook/stories/acpx-local.stories.tsx` cleaned up to match.
- Tests: deleted the `required: false` case in
`paperclip-skill-utils.test.ts` and the "keeps required bundled skills
installed" case in every `*-local-skill-sync.test.ts`;
`acpx-local-execute.test.ts`, `cursor-local-execute.test.ts`,
`cursor-local-skill-sync.test.ts`, `agent-skills-routes.test.ts`, and
`packages/adapter-utils/src/server-utils.test.ts` were updated to drop
removed fields and map `origin: "paperclip_required"` →
`"company_managed"`.
- `server/src/adapters/registry.ts`: two `as unknown as
ServerAdapterModule["..."]` casts on `hermesListSkills` /
`hermesSyncSkills` (matching the existing `executeHermesLocal` pattern).
`hermes-paperclip-adapter@0.2.0` still depends on the published
`@paperclipai/adapter-utils` which keeps the retired
`paperclip_required` variant; the cast bridges the
workspace-vs-published type mismatch at the registry seam and can drop
once hermes upgrades.

## Verification

Run from the workspace root:

```sh
grep -rn "skills/paperclip-dev" .
grep -rn "paperclip_required" --include="*.ts" --include="*.tsx" .
grep -rn "requiredReason" --include="*.ts" --include="*.tsx" .

pnpm -w typecheck
pnpm --filter @paperclipai/server exec vitest run paperclip-skill-utils
pnpm --filter @paperclipai/server exec vitest run skill-sync
```

The first three greps return only the explanatory comment in
`server/src/adapters/registry.ts` (no live `paperclip_required` /
`requiredReason` usage) and zero `skills/paperclip-dev` source hits.

Locally:

- `pnpm -w typecheck` → all packages this PR touches pass
(adapter-utils, shared, server, ui, cli, and the
cursor/gemini/opencode/pi adapters).
- Affected vitest suites pass: `paperclip-skill-utils`, `server-utils`,
all eight `*-local-skill-sync`, `agent-skills-routes`, and the
`acpx`/`cursor`/`pi` execute suites.

## Risks

- Behavioral shift in the agent skills UI: the "Required by Paperclip"
section disappears. No bundled skill is required anymore, so this only
affects environments that previously surfaced `paperclip-dev` as a
forced-on row; those installs will see the skill move into the regular
"company-managed" list (and be uninstalled on next sync unless
explicitly listed as desired).
- Existing agents may still have the string `"paperclip-dev"` in their
persisted `desiredSkills`. That entry is inert (no source for it to
install from); a one-time DB cleanup is out of scope. Low risk.
- Hermes adapter type bridge: two casts in `registry.ts` paper over a
type-only divergence between the workspace `@paperclipai/adapter-utils`
and the published version still pinned by
`hermes-paperclip-adapter@0.2.0`. Runtime behavior is unaffected because
the retired `paperclip_required` value is no longer produced by anything
in this tree. The casts can be removed once hermes upgrades its
dependency.

## Model Used

- Provider: Anthropic
- Model: Claude Opus 4.7 (`claude-opus-4-7`)
- Capability: agent tool use via Paperclip's `claude_local` adapter

## 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)
- [ ] 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
- [ ] All Paperclip CI gates are green
- [ ] 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-06-20 21:58:44 -07:00
DottaandPaperclip 5890b318c4 Namespace company skill identities
Persist canonical namespaced skill keys, split adapter runtime names from skill keys, and update portability/import flows to carry the canonical identity end-to-end.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-16 18:27:20 -05:00
Dotta 7675fd0856 Fix runtime skill injection across adapters 2026-03-15 07:05:01 -05:00
Dotta e619e64433 Add skill sync for remaining local adapters 2026-03-14 19:22:23 -05:00