mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 00:26:28 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors and agents read the docs under `doc/` before they change code > - `doc/SPEC.md` and `doc/SPEC-implementation.md` were out of date. `doc/SPEC-implementation.md` became an append-only log: in 60 days it got about 1,645 new lines and lost 181, across 140 commits > - #20557 changed `AGENTS.md` so the code and its tests are the source of truth, and the specs are only background > - This pull request removes the two spec docs. It moves only the sections that live code comments cite into focused docs, and it repoints all other references > - The benefit is one less stale document that agents read and append to ## Linked Issues or Issue Description Refs #20557 (merged). This PR is the follow-up that removes the docs. It is based on `master`, which already contains #20557. **Issue type** Outdated content. The docs are out of date and conflict with the code. **Where is the issue?** `doc/SPEC.md`, `doc/SPEC-implementation.md`, and the code comments and docs that cite them. **What's wrong?** `doc/SPEC.md` and `doc/SPEC-implementation.md` are out of date, but agents still read them and append a paragraph with each fix. **Suggested fix** Delete both files. Move the sections that code cites into focused docs. Repoint the references. ## What Changed - Deleted `doc/SPEC.md` and `doc/SPEC-implementation.md`. - Moved these spec sections, because live code comments cite them: - §9.3.1 "Shared default-open issue writes" → new section in `doc/agent-permission-defaults.md`. - §9.8.1 "Issue-thread interaction resolver contract" → `doc/execution-semantics.md`, "Issue-thread interaction resolution". That section already had most of the contract. I added only the missing parts: the `supersedeOnUserComment` default, the canonical policy list, the addressee and cap rules, the rule that surfaces show the server's effective audience, and the cross-issue budget charge. The short policy summary at the top of the section now points to the list, so the definitions occur one time only. - §9.9 "Task Watchdog Authority Contract" → new "Authority contract" section in `doc/TASK-WATCHDOG.md`. It holds the allowed and disallowed mutation lists and the deferred capabilities. The atomic recovery batch, restoration verification, and interaction rules were already in `doc/execution-semantics.md` §11, so the new section links there and does not repeat them. I did not move the "Downstream acceptance criteria" checklist or the sentence about the CTO and Security sign-off. These were implementation-time notes; the tests now hold that coverage. - §9.10 "Company Skill Policy Contract" → new `doc/company-skill-policy.md`. No focused doc had this contract. I did not move the "Required regression tests" phase checklist. - Did not move any other part of the specs. - Repointed code comments that cited spec sections: - `packages/shared/src/issue-write-denial.ts` (§9.3) → `doc/agent-permission-defaults.md` - `server/src/routes/issues.ts` (§9.9) → `doc/TASK-WATCHDOG.md`, "Authority contract" - `server/src/routes/issues.ts` (§9.3, §9.8.1 cross-issue budget) → `doc/agent-permission-defaults.md` - `ui/src/lib/interaction-audience.ts` and `ui/src/components/InteractionGovernancePanel.tsx` (§9.8.1) → `doc/execution-semantics.md` - `ui/src/lib/skill-policy-denial.ts`, `ui/src/pages/CompanySkills.tsx`, `ui/src/pages/CompanySkills.production.tsx`, `ui/src/pages/SkillStudio.tsx` (§9.10) → `doc/company-skill-policy.md` - `server/src/services/recovery/ownership-invariant-report.ts` (monitor deadline wait path) → `doc/execution-semantics.md` §8. That section already lists the one-shot monitor as a valid wait path. - Resynced the git blob pin for `packages/shared/src/issue-write-denial.ts` in `tests/runner-e2e/source-sdk-public-inputs.json`. The comment edit changed the file's hash, and the Runner support suite checks that pin. I used `node tests/runner-e2e/source-sdk-public-inputs.mjs <HEAD> --write`; only that one pin changed. - Updated `AGENTS.md` §1 so it does not name the deleted files. - Removed the links to the deleted files from `doc/plugins/PLUGIN_SPEC.md`. - Updated the `doc-maintenance` agent skill (`.agents/skills/doc-maintenance/`) so it no longer audits or stages `doc/SPEC.md`. - Removed or replaced references in `maps/README.md`, `maps/feature-map/README.md`, `maps/storybook-map/README.md`, `doc/PRODUCT.md`, `doc/CLIPHUB.md`, `doc/DEPLOYMENT-MODES.md`, `doc/DEVELOPING.md`, `doc/evals.md`, `doc/spec/agent-runs.md`, `doc/AGENTCOMPANIES_SPEC_INVENTORY.md`, `doc/connections/CONNECTOR-PLAYBOOK.md`, `doc/plugins/ideas-from-opencode.md`, `doc/execution-semantics.md`, `doc/TASK-WATCHDOG.md`, and `.devin/wiki.json`. - In `doc/execution-semantics.md`, the "admission is independent of cleanup and accounting" reference now states the rule inline: only a hard stop, an explicit pause, or an inactive company refuses admission. - Kept these references as they are: dated plans in `doc/plans/` and `docs/plans/`, the dated log `doc/logs/2026-05-24-cli-api-parity-e2e-log.md`, `releases/`, the transcript fixture `ui/src/fixtures/runTranscriptFixtures.ts` (it uses the filename only as sample text), and `docs/specs/external-task-protocol.md` (it refers to the Symphony project's own `SPEC.md`). ## Verification - `git grep -n "SPEC-implementation\|SPEC\.md"` (without `releases/`, and ignoring `PLUGIN_SPEC.md` citations) finds only the kept references listed above. - `grep -rn "§9\.\(3\|8\|9\|10\)" ui/src server/src packages/shared/src` finds no results. - `node scripts/check-maps.mjs`: "Maps aligned: 35 areas, 192 capability IDs". - `pnpm check:token-gates`: "All gates clean". - `pnpm exec vitest run --config tests/runner-e2e/vitest.config.ts tests/runner-e2e/source-sdk-public-type.test.ts tests/runner-e2e/source-sdk-evidence.test.ts`: 45 passed. These failed in CI before the pin resync. - `pnpm verify`: exit 2 ("Broader verification required") because `AGENTS.md` is a global CI input. Required CI supplies the broad coverage for this revision. - All code changes are comments only. No runtime behavior changes. The title uses `refactor:` because comments in source files changed. ## Risks - Low risk. The change removes docs and edits comments only. - A reader can lose context from spec sections that I did not move. The text stays in git history. The code and its tests are the source of truth. - External links to the two deleted files will return 404. > 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 - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), in Claude Code with tool use (shell, file edits). Standard context window. ## 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 (focused checks above; CI supplies broad coverage) - [x] I have added or updated tests where applicable (not applicable: docs and comments only) - [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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
355 lines
14 KiB
TypeScript
355 lines
14 KiB
TypeScript
/**
|
|
* Copy contract for denied issue writes (open cross-task writes: failure UX).
|
|
*
|
|
* Cross-issue issue writes are default-open for standard-trust agents on issues
|
|
* they can already read (see `doc/agent-permission-defaults.md`, "Shared
|
|
* default-open issue writes"). The remaining walls are
|
|
* rare — but a real incident burned a full detour discovering a workaround
|
|
* behind an opaque 403, so every one of them must say three things:
|
|
*
|
|
* 1. which boundary fired,
|
|
* 2. who *can* act,
|
|
* 3. the sanctioned path forward.
|
|
*
|
|
* This module is the single source of truth for that copy, so the API error
|
|
* body an agent reads and the notice a human sees in the UI are the same words.
|
|
* It is the issue-write sibling of `responsible-user-denial.ts`: the two
|
|
* responsible-user ceiling codes delegate to that module's copy so terminology
|
|
* ("on behalf of {user}", never "impersonate") stays consistent.
|
|
*/
|
|
|
|
import {
|
|
describeResponsibleUserDenial,
|
|
responsibleUserLabel,
|
|
type ResponsibleUserDenialCode,
|
|
} from "./responsible-user-denial.js";
|
|
|
|
export const ISSUE_WRITE_DENIAL_CODES = [
|
|
"issue_write_not_visible",
|
|
"issue_write_actor_class_excluded",
|
|
"issue_write_responsible_user_ceiling",
|
|
"issue_write_responsible_user_unavailable",
|
|
"issue_write_assignee_run_lock",
|
|
"issue_status_change_live_run",
|
|
"cross_issue_influence_cap_exceeded",
|
|
"cross_issue_influence_run_context_required",
|
|
"issue_write_attribution_spoof_rejected",
|
|
] as const;
|
|
|
|
export type IssueWriteDenialCode = (typeof ISSUE_WRITE_DENIAL_CODES)[number];
|
|
|
|
/**
|
|
* Why the write stopped, which drives icon + colour. `boundary` is an
|
|
* authorization wall, `lock` is run-lifecycle machinery that will clear on its
|
|
* own, `cap` is a rate backstop, and `attribution` is a rejected spoof.
|
|
*/
|
|
export type IssueWriteDenialTone = "boundary" | "lock" | "cap" | "attribution";
|
|
|
|
export interface IssueWriteDenialCopy {
|
|
code: IssueWriteDenialCode;
|
|
/** HTTP status the server pairs with this code. */
|
|
status: 403 | 409 | 422 | 429;
|
|
tone: IssueWriteDenialTone;
|
|
/** The boundary that fired, as a short noun phrase for a banner title. */
|
|
boundary: string;
|
|
/** Short heading. */
|
|
title: string;
|
|
/** What happened and why, in one or two sentences. */
|
|
description: string;
|
|
/** Who is able to perform this write instead. */
|
|
whoCanAct: string;
|
|
/** The supported way to get the work moving. */
|
|
sanctionedPath: string;
|
|
}
|
|
|
|
export interface IssueWriteDenialContext {
|
|
/** Display name of the agent or user that attempted the write. */
|
|
actorLabel?: string | null;
|
|
/** Display name of the responsible ("on behalf of") user for the attempt. */
|
|
responsibleUserName?: string | null;
|
|
/** Display name of the target issue's current assignee. */
|
|
assigneeLabel?: string | null;
|
|
/** Target issue identifier, e.g. `TASK-482`. */
|
|
issueIdentifier?: string | null;
|
|
/** Board path of the target issue, e.g. `/TASK/issues/TASK-482`. */
|
|
issuePath?: string | null;
|
|
/** Per-run cross-issue influence cap. */
|
|
cap?: number | null;
|
|
/** Attempt count that tripped the cap. */
|
|
count?: number | null;
|
|
/** ISO timestamp at which log-only rollout becomes enforcement. */
|
|
enforceAt?: string | null;
|
|
}
|
|
|
|
export function isIssueWriteDenialCode(
|
|
code: string | null | undefined,
|
|
): code is IssueWriteDenialCode {
|
|
return ISSUE_WRITE_DENIAL_CODES.includes(code as IssueWriteDenialCode);
|
|
}
|
|
|
|
/**
|
|
* Bridge the two responsible-user ceiling codes emitted by the authorization
|
|
* layer into this module's code space, so a single UI notice covers every way
|
|
* an issue write can be refused.
|
|
*/
|
|
export function issueWriteDenialCodeForResponsibleUserDenial(
|
|
code: ResponsibleUserDenialCode,
|
|
): IssueWriteDenialCode {
|
|
return code === "RESPONSIBLE_USER_UNAVAILABLE"
|
|
? "issue_write_responsible_user_unavailable"
|
|
: "issue_write_responsible_user_ceiling";
|
|
}
|
|
|
|
/** "this task" when the identifier is unknown, so copy never shows a raw id. */
|
|
function issueLabel(identifier: string | null | undefined): string {
|
|
const trimmed = identifier?.trim();
|
|
return trimmed && trimmed.length > 0 ? trimmed : "this task";
|
|
}
|
|
|
|
/** "the assignee" when the name is unknown. */
|
|
function assigneeLabel(name: string | null | undefined): string {
|
|
const trimmed = name?.trim();
|
|
return trimmed && trimmed.length > 0 ? trimmed : "the current assignee";
|
|
}
|
|
|
|
/** "this agent" when the name is unknown. */
|
|
function actorLabel(name: string | null | undefined): string {
|
|
const trimmed = name?.trim();
|
|
return trimmed && trimmed.length > 0 ? trimmed : "this agent";
|
|
}
|
|
|
|
export function describeIssueWriteDenial(
|
|
code: IssueWriteDenialCode,
|
|
context: IssueWriteDenialContext = {},
|
|
): IssueWriteDenialCopy {
|
|
const issue = issueLabel(context.issueIdentifier);
|
|
const actor = actorLabel(context.actorLabel);
|
|
const assignee = assigneeLabel(context.assigneeLabel);
|
|
const responsible = responsibleUserLabel(context.responsibleUserName);
|
|
|
|
switch (code) {
|
|
case "issue_write_not_visible":
|
|
return {
|
|
code,
|
|
status: 403,
|
|
tone: "boundary",
|
|
boundary: "Issue visibility",
|
|
title: "Task is outside this actor's visibility",
|
|
description:
|
|
`Issue writes are open by default, but only for tasks the actor can already ` +
|
|
`read. ${issue} is not visible to ${actor}, so its comment, update, child, and ` +
|
|
`assignment channels are all closed — the wall is visibility, not the write itself.`,
|
|
whoCanAct:
|
|
`${assignee}, and any agent or board member the task is visible to.`,
|
|
sanctionedPath:
|
|
`Ask the board to widen visibility for ${actor}.`,
|
|
};
|
|
|
|
case "issue_write_actor_class_excluded":
|
|
return {
|
|
code,
|
|
status: 403,
|
|
tone: "boundary",
|
|
boundary: "Actor-class boundary",
|
|
title: "This actor class cannot write to tasks",
|
|
description:
|
|
`Default-open issue writes are a standard-trust privilege. Low-trust, ` +
|
|
`skill-test, and task-bridge scopes keep their existing tight walls, so ` +
|
|
`${actor} cannot write to ${issue} no matter who it acts for.`,
|
|
whoCanAct:
|
|
`A standard-trust agent in this company, or a board member.`,
|
|
sanctionedPath:
|
|
`Report the request upward and let a standard-trust agent make the write — ` +
|
|
`actor-class scope cannot be widened per task.`,
|
|
};
|
|
|
|
case "issue_write_responsible_user_ceiling": {
|
|
const ceiling = describeResponsibleUserDenial("RESPONSIBLE_USER_UNAUTHORIZED", {
|
|
userName: context.responsibleUserName,
|
|
});
|
|
return {
|
|
code,
|
|
status: 403,
|
|
tone: "boundary",
|
|
boundary: "Responsible-user ceiling",
|
|
title: ceiling.title,
|
|
description: `${ceiling.description} The write to ${issue} was refused for that reason.`,
|
|
whoCanAct:
|
|
`${responsible} once authorized, or anyone already permitted to write to ${issue}.`,
|
|
sanctionedPath: ceiling.recommendedAction,
|
|
};
|
|
}
|
|
|
|
case "issue_write_responsible_user_unavailable": {
|
|
const unavailable = describeResponsibleUserDenial("RESPONSIBLE_USER_UNAVAILABLE", {
|
|
userName: context.responsibleUserName,
|
|
});
|
|
return {
|
|
code,
|
|
status: 403,
|
|
tone: "boundary",
|
|
// Distinct from the title, which already says "unavailable" — the
|
|
// boundary names the *mechanism*, so the two do not read as a stutter.
|
|
boundary: "Responsible-user availability",
|
|
title: unavailable.title,
|
|
description: `${unavailable.description} The write to ${issue} was refused for that reason.`,
|
|
whoCanAct: `A board member, or ${actor} once it has an active responsible user.`,
|
|
sanctionedPath: unavailable.recommendedAction,
|
|
};
|
|
}
|
|
|
|
case "issue_write_assignee_run_lock":
|
|
return {
|
|
code,
|
|
status: 409,
|
|
tone: "lock",
|
|
boundary: "Run checkout lock",
|
|
title: "Another agent's run owns this task",
|
|
description:
|
|
`${assignee} has ${issue} checked out and a run is live. Checkout and run ` +
|
|
`ownership stay assignee-scoped even though writes are open, so field edits ` +
|
|
`belong to the run that holds the lock until it finishes.`,
|
|
whoCanAct:
|
|
`${assignee}'s live run, or an agent holding the manage-active-checkouts permission.`,
|
|
sanctionedPath:
|
|
`Comment instead of patching — comments stay open and wake ${assignee} — or ` +
|
|
`wait for the run to release the lock and retry.`,
|
|
};
|
|
|
|
case "issue_status_change_live_run": {
|
|
const identifier = context.issueIdentifier?.trim();
|
|
const path = context.issuePath?.trim();
|
|
const taskLink = identifier && path ? `[${identifier}](${path})` : issue;
|
|
return {
|
|
code,
|
|
status: 409,
|
|
tone: "boundary",
|
|
boundary: "Live run execution lock",
|
|
title: "Another run is executing this task",
|
|
description:
|
|
`A run is live on ${issue} and holds its execution lock. Changing the status ` +
|
|
`would release that lock without stopping the run, so the run would keep ` +
|
|
`going with no task bound to it. Agents cannot stop or interrupt another ` +
|
|
`agent's run, so this status change is refused.`,
|
|
whoCanAct:
|
|
`A board user, or the run of ${assignee} that holds the lock.`,
|
|
sanctionedPath:
|
|
`This refusal is final: do not retry with another status, assignee or route. ` +
|
|
`Reply now and send the person to ${taskLink}. To stop the live run they use its ` +
|
|
`Stop or Interrupt control there; to hold this task and the tasks under it, they ` +
|
|
`open the parent task and choose More task actions, then Pause subtree. To pass ` +
|
|
`a request to the running agent, comment on the task instead.`,
|
|
};
|
|
}
|
|
|
|
case "cross_issue_influence_cap_exceeded": {
|
|
const cap = context.cap ?? 20;
|
|
const attempt = context.count ?? null;
|
|
return {
|
|
code,
|
|
status: 429,
|
|
tone: "cap",
|
|
// No parentheses: surfaces render the boundary inside their own parens.
|
|
boundary: `Per-run cross-issue cap of ${cap} writes`,
|
|
title: "This run has spent its cross-issue write budget",
|
|
description:
|
|
`A single heartbeat run may make at most ${cap} cross-issue comments or task ` +
|
|
`updates combined${attempt !== null ? `; this was attempt ${attempt}` : ""}. The cap ` +
|
|
`bounds runaway comment sprays and loops — it is a rate backstop, not a ` +
|
|
`permission decision, so ${actor} is still allowed to write to ${issue}.`,
|
|
whoCanAct:
|
|
`${actor} on its next heartbeat run, or ${assignee} on ${issue} directly.`,
|
|
sanctionedPath:
|
|
`Consolidate what is left into one comment on your own task, or end the run and ` +
|
|
`continue on the next heartbeat — the budget resets per run.`,
|
|
};
|
|
}
|
|
|
|
case "cross_issue_influence_run_context_required":
|
|
return {
|
|
code,
|
|
status: 403,
|
|
tone: "boundary",
|
|
boundary: "Heartbeat run context",
|
|
title: "Cross-issue writes need a run to attribute them to",
|
|
description:
|
|
`Every agent comment and task update is attributed to a heartbeat run so the ` +
|
|
`cross-issue cap can be counted and the audit trail can name who acted for whom. ` +
|
|
`This request arrived without a valid run, so it could not be contained.`,
|
|
whoCanAct: `${actor}, once the request carries its own run id.`,
|
|
sanctionedPath:
|
|
`Send the \`X-Paperclip-Run-Id\` header with your current run (\`$PAPERCLIP_RUN_ID\`) ` +
|
|
`and retry.`,
|
|
|
|
};
|
|
|
|
case "issue_write_attribution_spoof_rejected":
|
|
return {
|
|
code,
|
|
status: 422,
|
|
tone: "attribution",
|
|
boundary: "Server-derived attribution",
|
|
title: "Responsible user cannot be chosen by the caller",
|
|
description:
|
|
`\`onBehalfOfUserId\` is derived from the authenticated actor, never from the ` +
|
|
`request body — an agent cannot pick the human whose authority it rides. The ` +
|
|
`attempt was recorded in the audit log.`,
|
|
whoCanAct:
|
|
`${actor} itself: the write is allowed, only the chosen attribution is not.`,
|
|
sanctionedPath:
|
|
`Remove \`onBehalfOfUserId\` from the request and retry; the server fills in ` +
|
|
`${responsible} from your run.`,
|
|
};
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Flatten a denial into the single `error` string an API client sees.
|
|
*
|
|
* Agents typically surface only `error`, so all three §6 obligations — boundary,
|
|
* who can act, sanctioned path — have to survive the flattening.
|
|
*/
|
|
export function issueWriteDenialApiMessage(copy: IssueWriteDenialCopy): string {
|
|
return [
|
|
`${copy.title} (${copy.boundary}).`,
|
|
copy.description,
|
|
`Who can act: ${copy.whoCanAct}`,
|
|
`Try this: ${copy.sanctionedPath}`,
|
|
].join(" ");
|
|
}
|
|
|
|
/**
|
|
* Build the full `{ error, details }` body for a denied issue write. Keeping the
|
|
* machine-readable `code` next to the prose lets the board UI render the same
|
|
* copy without parsing sentences.
|
|
*/
|
|
export function issueWriteDenialResponse(
|
|
code: IssueWriteDenialCode,
|
|
context: IssueWriteDenialContext = {},
|
|
): {
|
|
status: IssueWriteDenialCopy["status"];
|
|
body: {
|
|
error: string;
|
|
details: {
|
|
code: IssueWriteDenialCode;
|
|
boundary: string;
|
|
whoCanAct: string;
|
|
sanctionedPath: string;
|
|
} & Record<string, unknown>;
|
|
};
|
|
} {
|
|
const copy = describeIssueWriteDenial(code, context);
|
|
return {
|
|
status: copy.status,
|
|
body: {
|
|
error: issueWriteDenialApiMessage(copy),
|
|
details: {
|
|
code: copy.code,
|
|
boundary: copy.boundary,
|
|
whoCanAct: copy.whoCanAct,
|
|
sanctionedPath: copy.sanctionedPath,
|
|
},
|
|
},
|
|
};
|
|
}
|