From b5f62b33844e273de0d05e0fca665f72ec4ea186 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:19:31 -0600 Subject: [PATCH] docs(rules): correct the scope of the API route and company-scope rules (#20596) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - `rules/*.mdc` holds the architecture rules that reviewers and agents apply to a change > - Pull request #20585 added two rules: API route expectations and company-scope exceptions > - Review found that both rules state more than is true for the current code > - A rule that is too broad makes reviewers flag correct code and can make an agent add a wrong check > - This pull request corrects the scope of both rules and fixes one stale pointer > - The benefit is rules that match the code they describe ## Linked Issues or Issue Description Refs #20585 **Issue type** Incorrect contributor documentation. **Where is the issue?** `rules/api-route-expectations.mdc`, `rules/company-scope-exceptions.mdc`, `AGENTS.md` section 8, and `.agents/skills/honeycomb-tracing/SKILL.md`. **What's wrong?** - The API route rule said that every route checks company access. Instance-level routes such as `health.ts`, `liveness.ts`, `ready.ts`, `public-mcp.ts` and `org-chart-svg.ts` do not, and must not. - The API route rule told reviewers to flag each status code outside a list of seven. `429` and `503` are correct in some routes. - The company-scope rule said that it lists the only instance-wide data. `user_sidebar_preferences` is instance-wide and was not in the list. - The company-scope rule said that it applies to new files that its `globs` cannot match. - The `honeycomb-tracing` skill pointed to `AGENTS.md` for the Telemetry workflow. That text is now in `rules/data-paths.mdc`. **Suggested fix** Limit each rule to what is true, and repoint the skill. ## What Changed - The API route rule and `AGENTS.md` section 8 now apply to routes that read or write company data. - The API route rule names the instance-level routes and says not to add a company check to them. - The status code list is now a list of common codes. Reviewers flag an incorrect code, not an unlisted code. - The company-scope rule lists the sidebar company order as an instance-wide exception, in the text, the `globs`, and the Greptile scope. - The company-scope rule says "known data" and no longer claims to match new files. - The `honeycomb-tracing` skill points to `rules/data-paths.mdc`. ## Verification - Read the two rules and confirm that each named file exists. A local script checked the `globs` and the route names. - Confirm that `.greptile/files.json` is valid JSON and that its scope matches the rule `globs`. - No source code changed. No test suite ran locally. ## Risks - Low risk. The change is documentation only. - The list of instance-level routes can go out of date when a route is added. ## Model Used - Claude (Anthropic), in Claude Code with tool use. - `claude-fable-5-2`. ## 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 - [ ] 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude --- .agents/skills/honeycomb-tracing/SKILL.md | 3 ++- .greptile/files.json | 8 ++++++-- AGENTS.md | 6 +++--- rules/api-route-expectations.mdc | 19 ++++++++++++++----- rules/company-scope-exceptions.mdc | 14 +++++++++----- 5 files changed, 34 insertions(+), 16 deletions(-) diff --git a/.agents/skills/honeycomb-tracing/SKILL.md b/.agents/skills/honeycomb-tracing/SKILL.md index c3ffe023617..3f900685753 100644 --- a/.agents/skills/honeycomb-tracing/SKILL.md +++ b/.agents/skills/honeycomb-tracing/SKILL.md @@ -33,7 +33,8 @@ Keep the repository's three paths distinct: another OTLP destination. With no endpoint, this tracing stays off. - **Telemetry:** first-party events under `packages/shared/src/telemetry/`, sent to Paperclip by default unless opted out. Adding these requires the generated - contract, Telemetry README, and privacy-review workflow in `AGENTS.md`. + contract, Telemetry README, and privacy-review workflow in + `rules/data-paths.mdc`. - **Run log:** local `heartbeat_run_events` and measured performance events. Useful for correlation, but a local event is not proof of Honeycomb export. diff --git a/.greptile/files.json b/.greptile/files.json index ad422163fd6..080b8aa1fe8 100644 --- a/.greptile/files.json +++ b/.greptile/files.json @@ -145,7 +145,7 @@ }, { "path": "rules/company-scope-exceptions.mdc", - "description": "The only instance-wide data (announcement dismissals, publication-ID registry, personal workspace preferences) and the conditions that keep company boundaries intact.", + "description": "The known instance-wide data (announcement dismissals, publication-ID registry, personal workspace preferences, sidebar company order) and the conditions that keep company boundaries intact.", "scope": [ "packages/db/src/schema/announcement_dismissals.ts", "packages/shared/src/announcements.ts", @@ -155,7 +155,11 @@ "packages/db/src/schema/user_workspace_preferences.ts", "packages/shared/src/workspace-preferences.ts", "server/src/routes/workspace-preferences.ts", - "server/src/services/workspace-preferences.ts" + "server/src/services/workspace-preferences.ts", + "packages/db/src/schema/user_sidebar_preferences.ts", + "packages/shared/src/types/sidebar-preferences.ts", + "server/src/routes/sidebar-preferences.ts", + "server/src/services/sidebar-preferences.ts" ] } ] diff --git a/AGENTS.md b/AGENTS.md index 55ae64c7d00..58331074ec7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -178,9 +178,9 @@ for the full workflow. Structural checks are not production acceptance. ## 8. API and Auth -Every route under `server/src/routes/` checks company access and actor -permissions, logs mutations to the activity log, and returns consistent HTTP -errors. See [`rules/api-route-expectations.mdc`](rules/api-route-expectations.mdc). +Every route under `server/src/routes/` that reads or writes company data checks +company access and actor permissions, logs mutations to the activity log, and +returns consistent HTTP errors. See [`rules/api-route-expectations.mdc`](rules/api-route-expectations.mdc). ## 9. UI Expectations diff --git a/rules/api-route-expectations.mdc b/rules/api-route-expectations.mdc index c60d0c59dd5..7c2306f39b0 100644 --- a/rules/api-route-expectations.mdc +++ b/rules/api-route-expectations.mdc @@ -1,5 +1,5 @@ --- -description: Every API route checks company access and actor permissions, logs mutations to the activity log, and returns consistent HTTP errors. +description: Every API route that reads or writes company data checks company access and actor permissions, logs mutations to the activity log, and returns consistent HTTP errors. globs: "server/src/routes/**" alwaysApply: false --- @@ -21,14 +21,21 @@ company's data away from another company's actors. ## Requirements -When you add or change an endpoint: +When you add or change an endpoint that reads or writes company data: - Apply company access checks. Use the helpers in `server/src/routes/authz.ts`, such as `assertCompanyAccess`, instead of a new check. - Enforce actor permissions (board vs agent), for example with `assertBoard` or `assertBoardOrAgent`. - Write an activity log entry for each mutation. -- Return consistent HTTP errors (`400/401/403/404/409/422/500`). +- Return consistent HTTP errors. The common codes are + `400/401/403/404/409/422/500`. Other codes are valid when they are the correct + status, such as `429` for rate limits and `503` while the server starts or + drains. + +Instance-level routes such as `health.ts`, `liveness.ts`, `ready.ts`, +`public-mcp.ts`, and `org-chart-svg.ts` have no company to check. Do not add a +company access check to a route that serves no company data. The API reference for callers is in `docs/api/`. Update it when a public endpoint changes. @@ -37,5 +44,7 @@ endpoint changes. Flag a route that reads or writes company data without a company access check. Flag a route that lets an agent key reach another company. Flag a mutation with -no activity log entry. Flag an error response outside the status codes above or -an error that hides a failure behind a success status. +no activity log entry. Flag an error response with an incorrect status code or +an error that hides a failure behind a success status. Do not flag a correct +status code only because it is not in the common list. Do not flag an +instance-level route for a missing company check. diff --git a/rules/company-scope-exceptions.mdc b/rules/company-scope-exceptions.mdc index 40795ec0117..a68dbb9d2e6 100644 --- a/rules/company-scope-exceptions.mdc +++ b/rules/company-scope-exceptions.mdc @@ -1,6 +1,6 @@ --- -description: Only announcement dismissals, the announcement publication-ID registry, and personal workspace presentation preferences are instance-wide; they must still validate company membership for audit and grant no authority. -globs: "packages/db/src/schema/announcement_dismissals.ts,packages/shared/src/announcements.ts,server/src/routes/announcements.ts,server/src/services/announcements.ts,server/src/services/announcement-feed.ts,packages/db/src/schema/user_workspace_preferences.ts,packages/shared/src/workspace-preferences.ts,server/src/routes/workspace-preferences.ts,server/src/services/workspace-preferences.ts" +description: The known instance-wide data is announcement dismissals, the announcement publication-ID registry, personal workspace presentation preferences, and the per-user sidebar company order; these grant no authority, and a new exception must be added to this list. +globs: "packages/db/src/schema/announcement_dismissals.ts,packages/shared/src/announcements.ts,server/src/routes/announcements.ts,server/src/services/announcements.ts,server/src/services/announcement-feed.ts,packages/db/src/schema/user_workspace_preferences.ts,packages/shared/src/workspace-preferences.ts,server/src/routes/workspace-preferences.ts,server/src/services/workspace-preferences.ts,packages/db/src/schema/user_sidebar_preferences.ts,packages/shared/src/types/sidebar-preferences.ts,server/src/routes/sidebar-preferences.ts,server/src/services/sidebar-preferences.ts" alwaysApply: false --- @@ -9,9 +9,10 @@ alwaysApply: false ## Scope and purpose Every domain entity is scoped to a company, and routes and services enforce -company boundaries. This rule lists the only data that is instance-wide and the -conditions that keep it safe. It also applies to any change that adds a new -table, route, or service that is not scoped to a company. +company boundaries. This rule lists the known data that is instance-wide and the +conditions that keep it safe. Its `globs` cover only the files of the listed +exceptions. `AGENTS.md` section 5 sends any change that adds data without a +company scope here. ## The exceptions @@ -25,6 +26,9 @@ table, route, or service that is not scoped to a company. execution authority, change company capabilities or agent configuration, or rewrite canonical work records. Optional conversation guidance must retain the same action permissions. +- **The sidebar company order** in `user_sidebar_preferences` is instance-wide by + user, because it orders the user's companies. Sidebar preferences inside one + company stay company-scoped in `company_user_sidebar_preferences`. ## Requirements