Skip to content

docs: clarify read-only scope fallback#883

Open
TurboTheTurtle wants to merge 1 commit into
CortexReach:masterfrom
TurboTheTurtle:codex/pr878-readonly-scope-followup
Open

docs: clarify read-only scope fallback#883
TurboTheTurtle wants to merge 1 commit into
CortexReach:masterfrom
TurboTheTurtle:codex/pr878-readonly-scope-followup

Conversation

@TurboTheTurtle

Copy link
Copy Markdown
Contributor

Follow-up from PR #878 review after the original PR was merged and its branch deleted.

Changes:

  • Align memory_debug with other read-only tools by resolving agent identity from execute-time runtime context.
  • Document that read-only tools return ignoredScope / accessibleScopes when a requested scope is inaccessible, while mutation tools still hard-deny with scope_access_denied.
  • Add focused regression coverage for memory_debug runtime scope resolution.

Validation:

  • node --test test/memory-governance-tools.test.mjs
  • npx tsc -p tsconfig.json --noEmit

@rickthomasjr rickthomasjr 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.

Agent Code Review: PR #883 — docs: clarify read-only scope fallback

Verdict: APPROVE

Summary

Fixes memory_debug tool scope resolution to use execute-time runtime context (resolveRuntimeAgentId) instead of hardcoding ?? "main". The runtime agentId is now properly resolved from the 5th execute() argument (runtimeCtx). Also adds a test case and updates README docs for read-only scope fallback behavior.

P1 — Scope resolution fix: APPROVED

Before: memory_debug always defaulted to agentId = "main" when toolCtx.agentId was not set, meaning agents calling the debug tool would get global scope results regardless of their actual agent identity. After: resolveRuntimeAgentId(staticAgentId, runtimeCtx) resolves the true caller agent from the 5th execute() parameter. This is the correct pattern used by other tools in the same codebase.

P2 — Test coverage: APPROVED

New test case "resolves memory_debug scopes from execute-time runtime context" exercises the exact path: passes runtimeCtx.agentId = "debugger", asserts the resolved scope filter includes agent:debugger and reflection:agent:debugger, and verifies the ignored scope notice and details fields are correct. Good.

P3 — README documentation: APPROVED

Clarifies that read-only tools fail soft with inaccessible scopes (search accessible scopes, return ignoredScope/accessibleScopes in details). Write tools still return scope_access_denied. Accurate and helpful.

P4 — Backward compatibility: APPROVED

The change is forward-compatible: agents that were already passing toolCtx.agentId are unaffected. Agents that relied on the old ?? "main" default now get correct behavior. No breaking change.

Conclusion

Clean correctness fix with test coverage and docs. APPROVE.

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