Skip to content

fix(approvals): bind status-only escalations to source - #1224

Merged
kkroo merged 1 commit into
masterfrom
codex/supersede-pr-1211
Aug 11, 2026
Merged

fix(approvals): bind status-only escalations to source#1224
kkroo merged 1 commit into
masterfrom
codex/supersede-pr-1211

Conversation

@kkroo

@kkroo kkroo commented Aug 9, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open-source control plane used to run and govern AI agents at work.
  • Productivity reviews are recovery work: a manager can identify a stalled source issue and decide that a human unblock owner is needed.
  • Those reviews run on the cheap, status-only profile, which correctly bars ordinary approval mutations.
  • The one useful exception is a request_board_approval, because it creates an inert human decision card rather than executing the requested action.
  • The predecessor fix(approvals): let status-only recovery runs file board escalations #1211 showed that a type-only exception was too broad: it could create an unlinked card or attach an unrelated same-company issue.
  • This successor binds the exception to the trusted run-context sourceIssueId, authorizes that fully populated issue resource, and permits no other linked issue.
  • The benefit is that a valid review can reach a human with actionable context without creating a new cross-issue escalation capability.

Linked Issues or Issue Description

Fixes: BLO-23036

Supersedes the App-authored #1211. A GitHub duplicate search for status_only approval found #1211 as the only open overlap.

What Changed

  • Allows a cheap status-only run to create only request_board_approval.
  • Requires the trusted sourceIssueId in issueIds and rejects a missing source link, review-shell substitution, and additional arbitrary issue IDs.
  • Loads the source issue within the company and runs issue:mutate authorization with its complete ownership, origin, and scope fields before creating or linking the approval.
  • Keeps resubmit, withdraw, comment, and direct issue↔approval link/unlink operations barred for status-only runs.
  • Updates manager guidance and focused route regressions for successful, unavailable, and unauthorized source paths.

Verification

pnpm vitest run server/src/__tests__/approval-routes-idempotency.test.ts --reporter=dot
# 29 passed

pnpm vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts --reporter=dot
# 183 passed

pnpm --filter @paperclipai/server typecheck
git diff --check

The issue-route suite emitted expected warnings for an absent optional local chatjimmy adapter; the suite completed successfully.

Risks

Low and intentionally fail-closed. A status-only run whose source is absent, outside the company, or outside its issue:mutate authorization boundary now receives 403 rather than creating a human approval card. Normal-model approvals retain their existing link behavior. No schema or migration change is included.

Model Used

OpenAI Codex, GPT-5. Tool-assisted code inspection, GitHub review-context analysis, isolated-worktree editing, and local test/typecheck execution; context window is platform-managed.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have linked the existing issue and predecessor PR
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots — n/a, server-side only
  • I have updated relevant documentation to reflect my changes
  • I have considered and documented any risks above
  • All Paperclip CI gates are green — pending this push
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review
  • I will address all Greptile and reviewer comments before requesting merge

@allyblockcast

allyblockcast Bot commented Aug 9, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-23036

1 similar comment
@allyblockcast

allyblockcast Bot commented Aug 9, 2026

Copy link
Copy Markdown

🔗 Paperclip issue: BLO-23036

@cursor

cursor Bot commented Aug 9, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@kkroo

kkroo commented Aug 9, 2026

Copy link
Copy Markdown
Author

@ally please review the exact head b85988d4415d6ecb6e68ec864f9dbeea916da438.

Independent successor to #1211. Review focus: a status-only recovery escalation must contain its trusted sourceIssueId as the sole issueIds entry; the route must load that source in-company and obtain issue:mutate authorization from its complete resource before it creates or links the board approval.

The focused regressions cover the allowed productivity-review path plus unlinked, review-shell, arbitrary same-company, missing-source, and authorization-denied paths.

No auto-merge has been enabled.

@kkroo
kkroo requested a review from allyblockcast August 9, 2026 14:09

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: b85988d

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • The status-only exception is fail-closed: it permits only request_board_approval, requires the trusted source issue as the sole link, loads it in the run company, and authorizes its complete ownership and scope before creation (server/src/routes/approvals.ts:423).
  • Focused regressions cover the successful escalation and the missing, substituted, unrelated, unavailable, and denied-source cases (server/src/__tests__/approval-routes-idempotency.test.ts:706).

Recommended Action

  1. No Critical or Important issues found.

@allyblockcast allyblockcast left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: b85988d

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • The status-only exception is fail-closed: it permits only request_board_approval, requires the trusted source issue as the sole link, loads it in the run company, and authorizes its complete ownership and scope before creation (server/src/routes/approvals.ts:423).
  • Focused regressions cover the successful escalation and the missing, substituted, unrelated, unavailable, and denied-source cases (server/src/__tests__/approval-routes-idempotency.test.ts:706).

Recommended Action

  1. No Critical or Important issues found.

Status-only recovery runs may now create only request_board_approval cards, and only when their trusted sourceIssueId is the sole linked issue. The route loads that source in the current company and authorizes its fully populated issue resource before creating or linking the approval.

This preserves the productivity-review recovery path while refusing unlinked cards, review-shell substitutes, arbitrary same-company additions, missing sources, and authorization denials.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@kkroo
kkroo force-pushed the codex/supersede-pr-1211 branch from b85988d to d37dbfb Compare August 10, 2026 23:05
@kkroo
kkroo merged commit 2da82c5 into master Aug 11, 2026
14 of 18 checks passed
@kkroo
kkroo deleted the codex/supersede-pr-1211 branch August 11, 2026 02:34
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.

2 participants