Add Session Message API for Agor MCP - #758
Draft
danimcgoo wants to merge 5 commits into
Draft
Conversation
Reviewed proposed Session Message API design against codebase alignment, architecture patterns, and implementation feasibility. Key findings: Critical issues identified: - Permission model overengineered (descendant-based access too complex) - Schema changes (created_by_session_id) need reconsideration - message_count fix should use computed-on-read, not materialized - API design has usability issues (confusing defaults, binary flags) - Missing migration scripts, error handling specs, testing strategy Recommendations: - Simplify to same-session-only or user-based permissions - Compute message_count via MessagesRepository.countBySessionId() - Make sessionId parameter required - Start with minimal viable implementation, validate, then iterate Status updated to NEEDS REVISION in specs. Files added: - specs/DESIGN_REVIEW.md (detailed 12k word analysis) - specs/REVIEW_SUMMARY.md (quick reference) - specs/agor-session-message-api.md (status updated) - specs/product-reasoning.md (review findings added) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revised Session Message API design based on product requirement clarity: Agor Assistants need to read results from worktrees they manage. Key changes: - Permission model: worktree-scoped (not session-to-session) - Schema: single change to worktrees.created_by_worktree_id - Cleaner semantics: "worktree creates worktree" vs mixed relationships - No circular dependencies, stable scope Design now approved with minor revisions: - Migration scripts needed (SQLite + PostgreSQL) - Queued message behavior documentation - Error response examples - Test acceptance criteria Benefits over original design: ✅ Simpler (no session-to-session circular refs) ✅ All sessions in assistant worktree can access managed worktrees ✅ Heartbeats can debug their own worktree ✅ Clear mental model (worktrees manage worktrees) Timeline: 5-6 days, risk: low Status: APPROVED WITH MINOR REVISIONS Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Capture quality standards and expectations for design documentation: **Documentation Principles:** - Targeted and succinct (no historical thinking or rejected alternatives) - Complete implementation details upfront (migrations, errors, edge cases) - Explicit integration with existing systems - Practical examples with real numbers **Design Quality Criteria:** 1. Choose the right abstraction (match complexity to requirements) 2. Integration over isolation (enhance existing architecture) 3. Performance by default (no "optimize later") 4. Error handling as first-class design **Review Process:** - Pre-review checklist (architecture, implementation, quality) - 8 review criteria (interfaces, alignment, API, UX, errors, extensibility, complexity, completeness) - Clear outcomes (approved, minor revisions, needs revision, redesign) **Anti-Patterns to Avoid:** - "Figure it out during implementation" - "It's like X but different" (without clear integration docs) - "Multiple ways to do the same thing" - Schema changes without migration SQL - Vague performance handwaving **Spec Structure Template:** Standard sections for consistent, complete design docs. These guidelines codify lessons from Session Message API review (worktree-scoped vs session-scoped, computed vs materialized, etc.) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Added additional review documentation: - CRITICAL_ISSUES.md - Integration with existing RBAC system - FINAL_DESIGN_REVIEW.md - Detailed review with RBAC considerations - REVIEW_OUTCOME.md - Summary of review status - Updated agor-session-message-api.md with RBAC integration notes Key clarification: Message reading permissions operate independently of worktree RBAC (others_can, worktree_owners). This is agent-to-agent coordination, not user access control. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Removed document bloat: - 8 redundant files → 1 clear spec - Removed: DESIGN_REVIEW.md, product-reasoning.md, REVIEW_SUMMARY.md, etc. - Kept: session-message-api.md (complete, succinct) Single spec contains: - Problem and solution - API design (2 endpoints) - Implementation (4 phases, 5-6 days) - Migration SQL (SQLite + PostgreSQL) - Error handling, testing, performance - Integration notes (RBAC independence) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
@mistercrunch - would love your feedback on this. I'm trying to use the Agor Assistants in a meta way, have them review and assess the work they are managing to identify patterns, and reflect on how we can improve how we're working. As we discussed in the other issue - I'm using the Agor Assistant to create the zones and prompts, and the heartbeats to move work trees through. But the Agor Assistant has no visibility of what's actually happening within the work trees other than the status. That is limiting its ability to introspect and improve. The key complexity seems to be interactions with the existing permission model, particularly if we see the assistant as a first class entity. Is it a human proxy? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add MCP endpoints to read session messages with worktree-scoped permissions.
Key Innovation: Worktree-scoped access control enables Agor Assistants to read results from worktrees they manage.
Changes
Design Documents
specs/agor-session-message-api.md- Complete API specificationspecs/product-reasoning.md- Problem analysis and architecturespecs/DESIGN_REVIEW.md- Comprehensive design reviewspecs/DESIGN_REVIEW_GUIDELINES.md- Quality standards for future reviewsspecs/REVIEW_SUMMARY.md- Quick referenceKey Design Points
Permission Model:
worktrees.created_by_worktree_idImplementation Phases:
message_countbug (compute on read, no schema changes)agor_messages_listendpointagor_sessions_get_resultendpointTimeline: 5-6 days, low risk
Test Plan
Status
Design Review: Approved with Minor Revisions
Minor revisions needed:
🤖 Generated with Claude Code