Repository navigation
Correct contributor paths and API-owned chat flow - #2242
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (4)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
2 issues found across 4 files
Confidence score: 4/5
AGENTS.mdexceeds the repository’s 100-line limit, so the new guidance may not meet the repo’s structure rules. Shorten it or move some guidance into a focused doc.scripts/check-maintained-instructions.mjscan silently skip validation when a section heading is renamed or removed. Make the check fail when an expected section is missing.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="AGENTS.md">
<violation number="1" location="AGENTS.md:89">
P2: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
`AGENTS.md` now exceeds the repository's 100-line file limit (106 lines). Shorten the new guidance or move it to a focused documentation file so this file remains under 100 lines.</violation>
</file>
<file name="scripts/check-maintained-instructions.mjs">
<violation number="1" location="scripts/check-maintained-instructions.mjs:9">
P3: The check silently stops validating a section if its `## ` heading is reworded or removed: `sections` names are hardcoded, and the `paths.size` guard only catches total loss, so a rename like `## MCP Tools` → `## Tools (MCP)` would drop that section's paths from the check without any error and let stale paths back in undetected. Assert each expected section exists before scanning it.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| transport boundary; App tool-result renderers do not register server tools. | ||
| Implement tool/business-logic changes in the owning API repository. | ||
|
|
||
| Run `pnpm instructions:check` after changing the architecture sections. It checks |
There was a problem hiding this comment.
P2: Custom agent: Code Structure and Size Limits for Readability and Single Responsibility
AGENTS.md now exceeds the repository's 100-line file limit (106 lines). Shorten the new guidance or move it to a focused documentation file so this file remains under 100 lines.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At AGENTS.md, line 89:
<comment>`AGENTS.md` now exceeds the repository's 100-line file limit (106 lines). Shorten the new guidance or move it to a focused documentation file so this file remains under 100 lines.</comment>
<file context>
@@ -79,7 +78,16 @@ pnpm update-types # Regenerate Supabase types
+transport boundary; App tool-result renderers do not register server tools.
+Implement tool/business-logic changes in the owning API repository.
+
+Run `pnpm instructions:check` after changing the architecture sections. It checks
+only maintained paths in this file, not historical documents or example trees.
</file context>
| for (const section of sections) { | ||
| const text = instructions.split(`## ${section}`)[1]?.split(/\n## /)[0] ?? ""; |
There was a problem hiding this comment.
P3: The check silently stops validating a section if its ## heading is reworded or removed: sections names are hardcoded, and the paths.size guard only catches total loss, so a rename like ## MCP Tools → ## Tools (MCP) would drop that section's paths from the check without any error and let stale paths back in undetected. Assert each expected section exists before scanning it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/check-maintained-instructions.mjs, line 9:
<comment>The check silently stops validating a section if its `## ` heading is reworded or removed: `sections` names are hardcoded, and the `paths.size` guard only catches total loss, so a rename like `## MCP Tools` → `## Tools (MCP)` would drop that section's paths from the check without any error and let stale paths back in undetected. Assert each expected section exists before scanning it.</comment>
<file context>
@@ -0,0 +1,17 @@
+const instructions = readFileSync(resolve(root, "AGENTS.md"), "utf8");
+const sections = ["Architecture", "MCP Tools", "Constants"];
+const paths = new Set();
+for (const section of sections) {
+ const text = instructions.split(`## ${section}`)[1]?.split(/\n## /)[0] ?? "";
+ for (const [, path] of text.matchAll(/`([^`]+)`/g))
</file context>
| for (const section of sections) { | |
| const text = instructions.split(`## ${section}`)[1]?.split(/\n## /)[0] ?? ""; | |
| for (const section of sections) { | |
| const marker = `## ${section}`; | |
| if (!instructions.includes(marker)) | |
| throw new Error(`Missing maintained section ${marker} in AGENTS.md`); | |
| const text = instructions.split(marker)[1]?.split(/\n## /)[0] ?? ""; |
Contributor instructions referenced deleted
lib/tools,lib/browserandlib/chat/toolChainspaths and obscured the current API-owned chat/tool flow. Point agents to the workflow transport, API origin helper and owning API repository. Add a bounded, dependency-free path check to PR CI; it checks the maintained Architecture/MCP/Constants sections without treating historical docs as active policy.Validation: the check reproduced the three missing paths before the correction and now verifies 15 maintained paths. JavaScript syntax, package JSON and diff checks pass. No application or UI behavior changes.
Refs recoupable/mono#356 and recoupable/mono#318.
Summary by cubic
Corrects the contributor instructions in AGENTS.md that referenced deleted
lib/tools,lib/browser, andlib/chat/toolChainspaths. The docs now describe the API-owned chat flow withhooks/useChatTransport.ts,lib/chat/createWorkflowChatTransport.ts, andlib/api/getClientApiBaseUrl.ts, and direct tool/business-logic work to the owning API repository. Adds a dependency-free check that runs in PR CI and verifies maintained paths in the Architecture, MCP Tools, and Constants sections without treating historical docs as active policy.Written for commit 8a5e85e. Summary will update on new commits.