Repository navigation
Conversation
…pace skills In a linked project the read-only analyst agent could not call any memory tool, yet the workspace identity section told it to save with `altimate_memory_write` and check `altimate_memory_read`. Its prompt named only the built-in data skills and required every reply to finish through `answer_commit`, which suits data values, not a list of conventions. - analyst may call `altimate_memory_read`, `altimate_memory_refresh` and `altimate_memory_audit`; write, delete and extract stay denied, and data-explorer is unchanged. - The team-memory line names only the memory tools the session's agent can call: a read-only agent is pointed at the read tool and told that builder saves; an agent with neither gets no line. - The analyst prompt loads a matching skill by source precedence, answers questions about team conventions in plain text from skills and memory, and keeps `answer_commit` for data questions. Fixes #1569 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 14 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: enabling Analyst to read workspace memory and use workspace skills. Description check The description includes all required sections, explains the issue and implementation, documents verification and limitations, addresses cost side effects, and completes the checklist. Linked Issues check Issue #1569requirements are met.analystallowsaltimate_memory_read,altimate_memory_refresh, andaltimate_memory_audit. Memory write, delete, and extract remain denied, anddata-explorer…Out of Scope Changes check The changes stay within Issue #1569. The empty-memory response removes guidance to an unavailable write tool. SharedLLM.removedToolslogic keeps identity text aligned with retained tools. The docum…Full details: Linked Issues check
-
Explanation
Issue
#1569requirements are met.analystallowsaltimate_memory_read,altimate_memory_refresh, andaltimate_memory_audit. Memory write, delete, and extract remain denied, anddata-explorerremains unchanged. The identity line uses the memory tools retained for the current agent and request. The analyst prompt loads matching workspace or project skills, answers non-data questions in plain text, and keepsanswer_commitfor data questions. Tests cover permissions, identity text, prompt behavior, and tool filtering.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within Issue
#1569. The empty-memory response removes guidance to an unavailable write tool. SharedLLM.removedToolslogic keeps identity text aligned with retained tools. The documentation updates describe the requested analyst access. Tests support the implementation. The listed audit-log and retrieval follow-ups are explicitly not changed and do not establish unrelated scope.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 14 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit reads the workspace lore,
And finds the skills beside the door.
It checks the memory, block by block,
While Builder keeps the writing lock.
For data, answers follow their flow,
For team conventions, plain words grow.
The rabbit nods: “Now off we go!”
Comment @coderabbitai help to get the list of available commands.
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…tool analyst can now call `altimate_memory_read` but not `altimate_memory_write`. An empty read told it to "Use altimate_memory_write to save information", steering a read-only agent toward a call it cannot make. The result now only says that no blocks were found. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…ization rule The analyst prompt now limits answer_commit to data questions and lets workspace-knowledge answers finish in plain text; update the pinned finalization assertion and pin the new exclusion sentence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Share one removal rule (`LLM.removedTools`) between tool resolution and the workspace identity section, so a memory tool switched off on the request, or not registered, is not named in the prompt. - A write-only agent gets the save line without the read-first step. - The analyst prompt checks `altimate_memory_read` only when it has it. - Update the analyst prompt assertion in sql-validation-e2e to the new finalization wording. - Cover all four read/write combinations and one cached outcome rendered for different agents. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… wiring - The analyst permission comment now says a reload syncs like the load every session runs and can drop a local copy of an archived block, rather than calling it read-only. - The prompt comment scopes the memory line to the tools the agent and request keep; tool-free steps withhold every tool the prompt names. - A prompt-loop test checks that the identity section is given the same memory access the tools handed to the model call imply. - Document analyst's memory and workspace-skill access. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Match the agent-modes page, and name the prompt-loop memory tests by what they check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (17 files)
Previous Review Summaries (2 snapshots, latest commit 212ff7b)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 212ff7b)Status: No Issues Found | Recommendation: Merge Files Reviewed (16 files)
Previous review (commit fb215f8)Status: No Issues Found | Recommendation: Merge Files Reviewed (16 files)
Reviewed by gemini-4-argon · Input: 77.1K · Output: 1.1K · Cached: 165.4K Review guidance: REVIEW.md from base branch |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 16 files
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="packages/opencode/test/session/prompt.test.ts">
<violation number="1" location="packages/opencode/test/session/prompt.test.ts:635">
P3: This test never checks the rendered memory text, because `systemSection` is mocked to return "". It only checks the flags passed in. Rename it to describe what it asserts, or add a no-access turn (both tools denied) so the loop covers the case the PR description lists.</violation>
</file>
<file name="packages/opencode/src/agent/agent.ts">
<violation number="1" location="packages/opencode/src/agent/agent.ts:343">
P2: `altimate_memory_audit` reads only this checkout's local `.log`, so it cannot show teammate changes made directly in workspace memory. Add a workspace-aware audit source before exposing it as workspace auditing.</violation>
</file>
<file name="packages/opencode/src/session/prompt.ts">
<violation number="1" location="packages/opencode/src/session/prompt.ts:1923">
P2: `tools` is only the pre-stream catalog: retrieval can still remove either memory tool, so this line can advertise an action the model cannot call. Derive these flags from the final tool set or ensure the named memory tools survive retrieval.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Add an analyst turn with no memory tools to the prompt-loop test and name the test for what it asserts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Multi-model consensus reviewVerdict: approve, with minor follow-ups. No critical or major issues. Five independent reviewers converged in one round, all approving. Read-only check. The three new analyst allows ( Minor
Nits
Done well
Multi-model consensus review (5 reviewers, 1 convergence round, unanimous). |
🤖 Code Review — OpenCodeReview (Gemini) — No Issues FoundNo supported files changed. |
… reload removes Analyst keeps training_save for corrections to its own behavior, so the read-only memory line now refers to workspace memory blocks only and no longer names one specific agent. The refresh tool description says it can remove local copies of archived blocks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
Thanks for the review. Here's what happened to each item. The fixes are in ecfba20.
Tests: 505 passed and 0 failed across the identity, analyst permission, prompt, LLM and memory suites. Typecheck is clean. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
This branch has not been deployed
3 participants
Issue for this PR
Closes #1569
Type of change
What does this PR do?
In a linked project, analyst could not answer read-only questions like "what are our conventions?": every memory tool was denied, the identity section named a save tool it lacked, and every reply had to go through answer_commit.
Small extras beyond the issue: the write-only case is reachable only through user config, and the docs lines.
Before and after
answer_commit, often with no useful answeranswer_commitanswer_commitFollow-ups (existing behaviour, not changed here)
The issues below came up in review. Each was reported in the issue tracker as a separate bug for follow-up, and neither is fixed in this PR.
altimate_memory_auditreads only this checkout's local audit log. It does not show changes teammates made directly in workspace memory.ALTIMATE_TOOL_RETRIEVAL=1, off by default), retrieval can drop a memory tool after the team-memory line has named it. Before this PR the line named both tools for every agent."*"oraltimate_memory_writeturns memory write, delete and extract back on for analyst, the same way it already turns onedit,writeandsql_execute_write. Onlybashis denied again after user config.bashis, or limit analyst to an allow-list"*": "deny"overrides the defaultexternal_directoryallow for skill directories, so analyst can't read files bundled with global skills stored outside the project. Workspace skills are unaffected.Truncate.GLOBalready isHow did you verify your code works?
Cost side effects
None. About 110 more tokens in the analyst system prompt and three more tool definitions. No new model calls, queries or resources that grow with user data.
Screenshots / recordings
N/A, no UI change.
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit