Repository navigation
feat(m29): add deterministic saved queue views - #51
Merged
Merged
Conversation
Add queue view CRUD operations for saving and reusing queue filter configurations. Views are stored in-memory (daemon lifetime) and can be applied to queue inspection operations. Protocol additions: - QueueViewFilters, QueueViewSort, QueueView types - Create/Update/Delete/Get/List params and results - Method enum variants for queue_view.* Daemon handlers: - handle_queue_view_create: create view with name uniqueness check - handle_queue_view_update: update view fields - handle_queue_view_delete: delete view - handle_queue_view_get: retrieve single view - handle_queue_view_list: list all views (optional name filter) MCP tools (TypeScript): - create_queue_view - update_queue_view - delete_queue_view - get_queue_view - list_queue_views No backend autonomy added.
Reviewer's GuideImplements in-memory CRUD support for deterministic saved queue views, wiring new protocol types and methods through the Rust daemon and exposing them as MCP tools/schemas in the TypeScript chatgpt-mcp app. Sequence diagram for create_queue_view MCP tool to daemon handlersequenceDiagram
actor User
participant ChatGPT
participant McpServer
participant DaemonClient
participant Daemon
participant Handlers
participant QueueViewsStore
User->>ChatGPT: Ask to create queue view
ChatGPT->>McpServer: Invoke tool create_queue_view
McpServer->>DaemonClient: call method queue_view.create
DaemonClient->>Daemon: send JSON RPC params
Daemon->>Handlers: dispatch Method QueueViewCreate
Handlers->>QueueViewsStore: lock in_memory HashMap
QueueViewsStore-->>Handlers: provide mutable views map
Handlers->>Handlers: validate name non_empty and unique
Handlers->>Handlers: construct QueueView with timestamps and id
Handlers->>QueueViewsStore: insert QueueView into HashMap
QueueViewsStore-->>Handlers: ok
Handlers-->>Daemon: CreateQueueViewResult as JSON
Daemon-->>DaemonClient: send response
DaemonClient-->>McpServer: return result
McpServer-->>ChatGPT: tool result content
ChatGPT-->>User: show created queue view details
Class diagram for deterministic queue view protocol typesclassDiagram
class QueueViewFilters {
+String workspace_id
+String status
+String label
+String assignee
+bool include_archived
+bool include_snoozed
+bool pinned_only
+bool snoozed_only
+bool archived_only
+RunPriority priority_filter
+RunEffort effort_filter
+bool blocked_only
+String blocked_by_run_id
+String due_on_or_before
+RunTriageBucket triage_bucket_filter
+bool stale_only
+bool fresh_only
+usize blocking_run_count_at_least
+String today
}
class QueueViewSort {
+bool sort_by_priority
+bool sort_by_due_date
+bool sort_by_effort
+bool sort_by_triage
+bool sort_by_staleness
}
class QueueView {
+String view_id
+String name
+String description
+QueueViewFilters filters
+QueueViewSort sort
+usize limit
+String created_at
+String updated_at
}
class CreateQueueViewParams {
+String name
+String description
+QueueViewFilters filters
+QueueViewSort sort
+usize limit
}
class CreateQueueViewResult {
+QueueView view
}
class UpdateQueueViewParams {
+String view_id
+String name
+Option~String~ description
+QueueViewFilters filters
+QueueViewSort sort
+Option~usize~ limit
}
class UpdateQueueViewResult {
+QueueView view
}
class DeleteQueueViewParams {
+String view_id
}
class DeleteQueueViewResult {
+String deleted_view_id
}
class GetQueueViewParams {
+String view_id
}
class ListQueueViewsParams {
+String name_contains
}
class ListQueueViewsResult {
+Vec~QueueView~ views
+usize count
}
QueueView --> QueueViewFilters : has
QueueView --> QueueViewSort : has
CreateQueueViewParams --> QueueViewFilters : uses
CreateQueueViewParams --> QueueViewSort : uses
CreateQueueViewResult --> QueueView : returns
UpdateQueueViewParams --> QueueViewFilters : uses
UpdateQueueViewParams --> QueueViewSort : uses
UpdateQueueViewResult --> QueueView : returns
ListQueueViewsResult --> QueueView : contains
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- The TypeScript
CreateQueueViewInputmarksfiltersandsortas optional, but the RustCreateQueueViewParamsrequires them; this mismatch will cause runtime deserialization errors when the client omits those fields and should be aligned (either make them required in TS or optional in Rust with sensible defaults). - In
schemas.tsthere are duplicated queue view schemas (QueueViewFilters/QueueViewSortvsQueueViewFiltersSchema/QueueViewSortSchema) with conflicting enum values (e.g.,effortFilterandtriageBucketFilteroptions differ); consider consolidating to a single, consistent definition that matches the Rust protocol types. handle_queue_view_getserializes and returns a bareQueueView, while other CRUD handlers wrap their payloads in*Resultstructs; for consistency and clearer protocol evolution, consider introducing and using aGetQueueViewResulttype similar to the others.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The TypeScript `CreateQueueViewInput` marks `filters` and `sort` as optional, but the Rust `CreateQueueViewParams` requires them; this mismatch will cause runtime deserialization errors when the client omits those fields and should be aligned (either make them required in TS or optional in Rust with sensible defaults).
- In `schemas.ts` there are duplicated queue view schemas (`QueueViewFilters`/`QueueViewSort` vs `QueueViewFiltersSchema`/`QueueViewSortSchema`) with conflicting enum values (e.g., `effortFilter` and `triageBucketFilter` options differ); consider consolidating to a single, consistent definition that matches the Rust protocol types.
- `handle_queue_view_get` serializes and returns a bare `QueueView`, while other CRUD handlers wrap their payloads in `*Result` structs; for consistency and clearer protocol evolution, consider introducing and using a `GetQueueViewResult` type similar to the others.
## Individual Comments
### Comment 1
<location path="codex-rs/deterministic-daemon/src/handlers.rs" line_range="1584-1593" />
<code_context>
+ anyhow::bail!("view name cannot be empty");
+ }
+
+ // Check for duplicate name
+ {
+ let views = QUEUE_VIEWS.lock().map_err(|e| anyhow::anyhow!("lock error: {e}"))?;
+ for v in views.values() {
+ if v.name.to_lowercase() == name.to_lowercase() {
+ anyhow::bail!("a view with this name already exists");
+ }
+ }
+ }
+
+ let now = chrono::Utc::now().to_rfc3339();
+ let view_id = format!("qv-{}", uuid::Uuid::new_v4());
+
+ let view = QueueView {
+ view_id: view_id.clone(),
+ name,
+ description: p.description,
+ filters: p.filters,
+ sort: p.sort,
+ limit: p.limit,
+ created_at: now.clone(),
+ updated_at: now,
+ };
+
+ {
+ let mut views = QUEUE_VIEWS.lock().map_err(|e| anyhow::anyhow!("lock error: {e}"))?;
+ views.insert(view_id, view.clone());
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Name uniqueness check is not actually race-free; two concurrent creates can still insert duplicate names.
The name check and the insert run under separate lock acquisitions. Another thread can create a view with the same name in between, leading to duplicate names (case-insensitive). To ensure uniqueness, hold the lock across both the check and the insert, or re-check under the same lock immediately before inserting.
</issue_to_address>
### Comment 2
<location path="apps/chatgpt-mcp/src/schemas.ts" line_range="680" />
<code_context>
+export const CreateQueueViewInput = {
+ name: z.string().describe("Human-readable name for the view (required, non-empty after trim)"),
+ description: z.string().optional().describe("Optional description"),
+ filters: z.object(QueueViewFilters).optional().describe("Filter configuration"),
+ sort: z.object(QueueViewSort).optional().describe("Sort configuration"),
+ limit: z.number().optional().describe("Maximum runs to return"),
</code_context>
<issue_to_address>
**issue (bug_risk):** `filters` is optional in the MCP schema but required in the Rust `CreateQueueViewParams`, which will cause deserialization failures.
On the Rust side, `CreateQueueViewParams.filters` is a non-optional `QueueViewFilters` without `#[serde(default)]`, so JSON requests that omit `filters` will fail to deserialize. In contrast, the schema and `tools.ts` allow `filters` to be `undefined` and pass it through as-is. Please either make `filters` required in the schema or update Rust to `Option<QueueViewFilters>` and/or add `#[serde(default)]` so it is genuinely optional in the protocol.
</issue_to_address>
### Comment 3
<location path="apps/chatgpt-mcp/src/schemas.ts" line_range="726-730" />
<code_context>
+ snoozedOnly: z.boolean().optional().describe("Only snoozed runs"),
+ archivedOnly: z.boolean().optional().describe("Only archived runs"),
+ priorityFilter: z.enum(["low", "normal", "high", "urgent"]).optional().describe("Filter by priority"),
+ effortFilter: z.enum(["tiny", "small", "medium", "large", "huge"]).optional().describe("Filter by effort"),
+ blockedOnly: z.boolean().optional().describe("Only blocked runs"),
+ blockedByRunId: z.string().optional().describe("Only runs blocked by specific run ID"),
+ dueOnOrBefore: z.string().optional().describe("Only runs due on or before date (YYYY-MM-DD)"),
+ triageBucketFilter: z.enum(["urgent", "attention", "stale", "blocked", "ready"]).optional().describe("Filter by triage bucket"),
+ staleOnly: z.boolean().optional().describe("Only stale runs"),
+ freshOnly: z.boolean().optional().describe("Only fresh (not stale) runs"),
</code_context>
<issue_to_address>
**issue (bug_risk):** The MCP filter enums (`effortFilter`, `triageBucketFilter`) are inconsistent with the earlier `QueueViewFilters` definition and likely with the Rust types.
`QueueViewFilters` (used in `CreateQueueViewInput`) and `QueueViewFiltersSchema` define different value sets:
- `effortFilter`: `"tiny" | "small" | "medium" | "large" | "xlarge"` vs `"tiny" | "small" | "medium" | "large" | "huge"`
- `triageBucketFilter`: `"ready" | "blocked" | "deferred"` vs `"urgent" | "attention" | "stale" | "blocked" | "ready"`
This ambiguity makes it unclear what the daemon actually accepts and risks values that don’t map to `RunEffort`/`RunTriageBucket` in Rust. Please consolidate these to a single shared enum definition aligned exactly with the Rust variants.
</issue_to_address>
### Comment 4
<location path="apps/chatgpt-mcp/src/schemas.ts" line_range="715-724" />
<code_context>
+export const QueueViewFiltersSchema = {
</code_context>
<issue_to_address>
**suggestion:** Duplicated queue view filter/sort schema objects introduce drift and confusion.
We now have two parallel schema definitions for filters/sort (`QueueViewFilters` / `QueueViewSort` vs `QueueViewFiltersSchema` / `QueueViewSortSchema`), and they already differ in descriptions and enum values. Unless there’s a strong reason to keep both, prefer a single shared definition to avoid drift between what tools accept and what the daemon expects.
Suggested implementation:
```typescript
/** Filter configuration for saved queue views.
*
* This is an alias of the canonical QueueViewFilters schema used by the daemon,
* to avoid drift between what tools accept and what the backend expects.
*/
export const QueueViewFiltersSchema = QueueViewFilters;
```
1. Ensure that `QueueViewFilters` is imported or defined in this file. If it lives in another module (e.g. a shared daemon schema file), add an appropriate import such as:
`import { QueueViewFilters } from "path/to/queue-view-schemas";`
2. Apply the same pattern for the sort schema to eliminate the second duplication: replace any `QueueViewSortSchema = { ... }` object literal with `QueueViewSortSchema = QueueViewSort`, and ensure `QueueViewSort` is imported from the canonical location.
3. If the existing `QueueViewFilters` / `QueueViewSort` are Zod object schemas with different descriptions or enum values, reconcile those in the canonical definitions rather than reintroducing them here, so both daemon and tools stay in sync.
</issue_to_address>
### Comment 5
<location path="codex-rs/deterministic-daemon/src/handlers.rs" line_range="1566" />
<code_context>
+use std::collections::HashMap;
+use std::sync::Mutex;
+
+lazy_static::lazy_static! {
+ static ref QUEUE_VIEWS: Mutex<HashMap<String, deterministic_protocol::types::QueueView>> =
+ Mutex::new(HashMap::new());
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the queue view CRUD code by centralizing mutex access and name validation helpers to make the handlers shorter and less repetitive while preserving behavior.
You can reduce the complexity of this addition without changing behavior by:
1. Centralizing access to `QUEUE_VIEWS`
2. Removing redundant existence checks
3. Extracting name uniqueness validation
### 1. Centralize locking / access
Instead of repeating the `Mutex` lock and error mapping in every handler, add a small helper:
```rust
use std::collections::HashMap;
use std::sync::{Mutex, MutexGuard};
lazy_static::lazy_static! {
static ref QUEUE_VIEWS: Mutex<HashMap<String, deterministic_protocol::types::QueueView>> =
Mutex::new(HashMap::new());
}
fn queue_views() -> Result<MutexGuard<'static, HashMap<String, deterministic_protocol::types::QueueView>>> {
QUEUE_VIEWS
.lock()
.map_err(|e| anyhow::anyhow!("lock error: {e}"))
}
```
Then handlers become simpler and more consistent:
```rust
fn handle_queue_view_get(
params: serde_json::Value,
_store: &Store,
) -> Result<(serde_json::Value, Option<RunState>)> {
use deterministic_protocol::types::*;
let p: GetQueueViewParams = serde_json::from_value(params)?;
let views = queue_views()?;
let view = views
.get(&p.view_id)
.ok_or_else(|| anyhow::anyhow!("view not found: {view_id}", view_id = p.view_id))?
.clone();
Ok((serde_json::to_value(view)?, None))
}
```
Apply the same `queue_views()?` in all CRUD handlers.
### 2. Remove redundant existence check in update
You can rely solely on `get_mut(...).ok_or_else(...)` instead of a prior `contains_key`:
```rust
fn handle_queue_view_update(
params: serde_json::Value,
_store: &Store,
) -> Result<(serde_json::Value, Option<RunState>)> {
use deterministic_protocol::types::*;
let p: UpdateQueueViewParams = serde_json::from_value(params)?;
let mut views = queue_views()?;
// Validate name uniqueness if updating name
if let Some(ref name) = p.name {
ensure_unique_name(&views, Some(&p.view_id), name)?;
}
let view = views
.get_mut(&p.view_id)
.ok_or_else(|| anyhow::anyhow!("view not found: {view_id}", view_id = p.view_id))?;
// ... rest unchanged ...
}
```
### 3. Extract name uniqueness validation
The name uniqueness logic is duplicated and slightly different between create/update. A helper makes it consistent and easier to reason about:
```rust
fn ensure_unique_name(
views: &HashMap<String, deterministic_protocol::types::QueueView>,
self_id: Option<&str>,
raw_name: &str,
) -> Result<String> {
let name = raw_name.trim().to_string();
if name.is_empty() {
anyhow::bail!("view name cannot be empty");
}
let lower = name.to_lowercase();
for v in views.values() {
if self_id.map_or(true, |id| v.view_id != id) && v.name.to_lowercase() == lower {
anyhow::bail!("a view with this name already exists");
}
}
Ok(name)
}
```
Usage in `create`:
```rust
fn handle_queue_view_create(
params: serde_json::Value,
_store: &Store,
) -> Result<(serde_json::Value, Option<RunState>)> {
use deterministic_protocol::types::*;
let p: CreateQueueViewParams = serde_json::from_value(params)?;
let mut views = queue_views()?;
let name = ensure_unique_name(&views, None, &p.name)?;
// build view using `name` ...
}
```
Usage in `update` (as shown above) returns the normalized name if you want to apply it:
```rust
if let Some(name) = p.name {
view.name = ensure_unique_name(&views, Some(&p.view_id), &name)?;
}
```
This keeps all current behavior but reduces repetition, centralizes error messages, and makes the CRUD handlers shorter and easier to follow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1584
to
+1593
| // Check for duplicate name | ||
| { | ||
| let views = QUEUE_VIEWS.lock().map_err(|e| anyhow::anyhow!("lock error: {e}"))?; | ||
| for v in views.values() { | ||
| if v.name.to_lowercase() == name.to_lowercase() { | ||
| anyhow::bail!("a view with this name already exists"); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
issue (bug_risk): Name uniqueness check is not actually race-free; two concurrent creates can still insert duplicate names.
The name check and the insert run under separate lock acquisitions. Another thread can create a view with the same name in between, leading to duplicate names (case-insensitive). To ensure uniqueness, hold the lock across both the check and the insert, or re-check under the same lock immediately before inserting.
| export const CreateQueueViewInput = { | ||
| name: z.string().describe("Human-readable name for the view (required, non-empty after trim)"), | ||
| description: z.string().optional().describe("Optional description"), | ||
| filters: z.object(QueueViewFilters).optional().describe("Filter configuration"), |
There was a problem hiding this comment.
issue (bug_risk): filters is optional in the MCP schema but required in the Rust CreateQueueViewParams, which will cause deserialization failures.
On the Rust side, CreateQueueViewParams.filters is a non-optional QueueViewFilters without #[serde(default)], so JSON requests that omit filters will fail to deserialize. In contrast, the schema and tools.ts allow filters to be undefined and pass it through as-is. Please either make filters required in the schema or update Rust to Option<QueueViewFilters> and/or add #[serde(default)] so it is genuinely optional in the protocol.
Comment on lines
+726
to
+730
| effortFilter: z.enum(["tiny", "small", "medium", "large", "huge"]).optional().describe("Filter by effort"), | ||
| blockedOnly: z.boolean().optional().describe("Only blocked runs"), | ||
| blockedByRunId: z.string().optional().describe("Only runs blocked by specific run ID"), | ||
| dueOnOrBefore: z.string().optional().describe("Only runs due on or before date (YYYY-MM-DD)"), | ||
| triageBucketFilter: z.enum(["urgent", "attention", "stale", "blocked", "ready"]).optional().describe("Filter by triage bucket"), |
There was a problem hiding this comment.
issue (bug_risk): The MCP filter enums (effortFilter, triageBucketFilter) are inconsistent with the earlier QueueViewFilters definition and likely with the Rust types.
QueueViewFilters (used in CreateQueueViewInput) and QueueViewFiltersSchema define different value sets:
effortFilter:"tiny" | "small" | "medium" | "large" | "xlarge"vs"tiny" | "small" | "medium" | "large" | "huge"triageBucketFilter:"ready" | "blocked" | "deferred"vs"urgent" | "attention" | "stale" | "blocked" | "ready"
This ambiguity makes it unclear what the daemon actually accepts and risks values that don’t map toRunEffort/RunTriageBucketin Rust. Please consolidate these to a single shared enum definition aligned exactly with the Rust variants.
Comment on lines
+715
to
+724
| export const QueueViewFiltersSchema = { | ||
| workspaceId: z.string().optional().describe("Filter by workspace path"), | ||
| status: z.string().optional().describe("Filter by status prefix"), | ||
| label: z.string().optional().describe("Filter by exact label (case-insensitive)"), | ||
| assignee: z.string().optional().describe("Filter by exact assignee"), | ||
| includeArchived: z.boolean().optional().describe("Include archived runs"), | ||
| includeSnoozed: z.boolean().optional().describe("Include snoozed runs"), | ||
| pinnedOnly: z.boolean().optional().describe("Only pinned runs"), | ||
| snoozedOnly: z.boolean().optional().describe("Only snoozed runs"), | ||
| archivedOnly: z.boolean().optional().describe("Only archived runs"), |
There was a problem hiding this comment.
suggestion: Duplicated queue view filter/sort schema objects introduce drift and confusion.
We now have two parallel schema definitions for filters/sort (QueueViewFilters / QueueViewSort vs QueueViewFiltersSchema / QueueViewSortSchema), and they already differ in descriptions and enum values. Unless there’s a strong reason to keep both, prefer a single shared definition to avoid drift between what tools accept and what the daemon expects.
Suggested implementation:
/** Filter configuration for saved queue views.
*
* This is an alias of the canonical QueueViewFilters schema used by the daemon,
* to avoid drift between what tools accept and what the backend expects.
*/
export const QueueViewFiltersSchema = QueueViewFilters;- Ensure that
QueueViewFiltersis imported or defined in this file. If it lives in another module (e.g. a shared daemon schema file), add an appropriate import such as:
import { QueueViewFilters } from "path/to/queue-view-schemas"; - Apply the same pattern for the sort schema to eliminate the second duplication: replace any
QueueViewSortSchema = { ... }object literal withQueueViewSortSchema = QueueViewSort, and ensureQueueViewSortis imported from the canonical location. - If the existing
QueueViewFilters/QueueViewSortare Zod object schemas with different descriptions or enum values, reconcile those in the canonical definitions rather than reintroducing them here, so both daemon and tools stay in sync.
| use std::collections::HashMap; | ||
| use std::sync::Mutex; | ||
|
|
||
| lazy_static::lazy_static! { |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the queue view CRUD code by centralizing mutex access and name validation helpers to make the handlers shorter and less repetitive while preserving behavior.
You can reduce the complexity of this addition without changing behavior by:
- Centralizing access to
QUEUE_VIEWS - Removing redundant existence checks
- Extracting name uniqueness validation
1. Centralize locking / access
Instead of repeating the Mutex lock and error mapping in every handler, add a small helper:
use std::collections::HashMap;
use std::sync::{Mutex, MutexGuard};
lazy_static::lazy_static! {
static ref QUEUE_VIEWS: Mutex<HashMap<String, deterministic_protocol::types::QueueView>> =
Mutex::new(HashMap::new());
}
fn queue_views() -> Result<MutexGuard<'static, HashMap<String, deterministic_protocol::types::QueueView>>> {
QUEUE_VIEWS
.lock()
.map_err(|e| anyhow::anyhow!("lock error: {e}"))
}Then handlers become simpler and more consistent:
fn handle_queue_view_get(
params: serde_json::Value,
_store: &Store,
) -> Result<(serde_json::Value, Option<RunState>)> {
use deterministic_protocol::types::*;
let p: GetQueueViewParams = serde_json::from_value(params)?;
let views = queue_views()?;
let view = views
.get(&p.view_id)
.ok_or_else(|| anyhow::anyhow!("view not found: {view_id}", view_id = p.view_id))?
.clone();
Ok((serde_json::to_value(view)?, None))
}Apply the same queue_views()? in all CRUD handlers.
2. Remove redundant existence check in update
You can rely solely on get_mut(...).ok_or_else(...) instead of a prior contains_key:
fn handle_queue_view_update(
params: serde_json::Value,
_store: &Store,
) -> Result<(serde_json::Value, Option<RunState>)> {
use deterministic_protocol::types::*;
let p: UpdateQueueViewParams = serde_json::from_value(params)?;
let mut views = queue_views()?;
// Validate name uniqueness if updating name
if let Some(ref name) = p.name {
ensure_unique_name(&views, Some(&p.view_id), name)?;
}
let view = views
.get_mut(&p.view_id)
.ok_or_else(|| anyhow::anyhow!("view not found: {view_id}", view_id = p.view_id))?;
// ... rest unchanged ...
}3. Extract name uniqueness validation
The name uniqueness logic is duplicated and slightly different between create/update. A helper makes it consistent and easier to reason about:
fn ensure_unique_name(
views: &HashMap<String, deterministic_protocol::types::QueueView>,
self_id: Option<&str>,
raw_name: &str,
) -> Result<String> {
let name = raw_name.trim().to_string();
if name.is_empty() {
anyhow::bail!("view name cannot be empty");
}
let lower = name.to_lowercase();
for v in views.values() {
if self_id.map_or(true, |id| v.view_id != id) && v.name.to_lowercase() == lower {
anyhow::bail!("a view with this name already exists");
}
}
Ok(name)
}Usage in create:
fn handle_queue_view_create(
params: serde_json::Value,
_store: &Store,
) -> Result<(serde_json::Value, Option<RunState>)> {
use deterministic_protocol::types::*;
let p: CreateQueueViewParams = serde_json::from_value(params)?;
let mut views = queue_views()?;
let name = ensure_unique_name(&views, None, &p.name)?;
// build view using `name` ...
}Usage in update (as shown above) returns the normalized name if you want to apply it:
if let Some(name) = p.name {
view.name = ensure_unique_name(&views, Some(&p.view_id), &name)?;
}This keeps all current behavior but reduces repetition, centralizes error messages, and makes the CRUD handlers shorter and easier to follow.
There was a problem hiding this comment.
Pull request overview
Adds a new “saved queue view” feature (Milestone 29) spanning the Rust protocol + daemon and the TypeScript MCP gateway, enabling CRUD for reusable queue filter/sort configurations stored in-memory for the daemon lifetime.
Changes:
- Introduces
QueueView*protocol types and JSON-RPC method variants (queue_view.*). - Implements daemon handlers for create/update/delete/get/list backed by an in-process in-memory map.
- Adds MCP tools + Zod schemas for queue view CRUD.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| codex-rs/deterministic-protocol/src/types.rs | Adds QueueViewFilters/Sort/View and CRUD param/result DTOs. |
| codex-rs/deterministic-protocol/src/methods.rs | Registers new JSON-RPC methods for queue view CRUD. |
| codex-rs/deterministic-daemon/src/handlers.rs | Dispatch + handler implementations for queue view CRUD using in-memory storage. |
| codex-rs/deterministic-daemon/Cargo.toml | Adds lazy_static dependency for global in-memory storage. |
| codex-rs/Cargo.lock | Locks lazy_static as a new dependency of deterministic-daemon. |
| apps/chatgpt-mcp/src/tools.ts | Registers new MCP tools that call queue_view.* daemon methods. |
| apps/chatgpt-mcp/src/schemas.ts | Adds Zod schemas for queue view filters/sort and CRUD tool inputs (with duplicated schema blocks). |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+1571
to
+1615
| fn handle_queue_view_create( | ||
| params: serde_json::Value, | ||
| _store: &Store, | ||
| ) -> Result<(serde_json::Value, Option<RunState>)> { | ||
| use deterministic_protocol::types::*; | ||
|
|
||
| let p: CreateQueueViewParams = serde_json::from_value(params)?; | ||
|
|
||
| let name = p.name.trim().to_string(); | ||
| if name.is_empty() { | ||
| anyhow::bail!("view name cannot be empty"); | ||
| } | ||
|
|
||
| // Check for duplicate name | ||
| { | ||
| let views = QUEUE_VIEWS.lock().map_err(|e| anyhow::anyhow!("lock error: {e}"))?; | ||
| for v in views.values() { | ||
| if v.name.to_lowercase() == name.to_lowercase() { | ||
| anyhow::bail!("a view with this name already exists"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let now = chrono::Utc::now().to_rfc3339(); | ||
| let view_id = format!("qv-{}", uuid::Uuid::new_v4()); | ||
|
|
||
| let view = QueueView { | ||
| view_id: view_id.clone(), | ||
| name, | ||
| description: p.description, | ||
| filters: p.filters, | ||
| sort: p.sort, | ||
| limit: p.limit, | ||
| created_at: now.clone(), | ||
| updated_at: now, | ||
| }; | ||
|
|
||
| { | ||
| let mut views = QUEUE_VIEWS.lock().map_err(|e| anyhow::anyhow!("lock error: {e}"))?; | ||
| views.insert(view_id, view.clone()); | ||
| } | ||
|
|
||
| let result = CreateQueueViewResult { view }; | ||
| Ok((serde_json::to_value(result)?, None)) | ||
| } |
Comment on lines
+665
to
+667
| server.tool( | ||
| "create_queue_view", | ||
| "Create a saved queue view with deterministic filter/sort configuration. " + |
| const result = await client.call("queue_view.create", { | ||
| name: params.name, | ||
| description: params.description, | ||
| filters: params.filters, |
Comment on lines
+655
to
+664
| priorityFilter: z.enum(["low", "normal", "high", "urgent"]).optional().describe("Filter by exact priority"), | ||
| effortFilter: z.enum(["tiny", "small", "medium", "large", "xlarge"]).optional().describe("Filter by effort bucket"), | ||
| blockedOnly: z.boolean().optional().describe("Only blocked runs"), | ||
| blockedByRunId: z.string().optional().describe("Only runs blocked by specific run ID"), | ||
| dueOnOrBefore: z.string().optional().describe("Only runs with due date on or before threshold (YYYY-MM-DD)"), | ||
| triageBucketFilter: z.enum(["ready", "blocked", "deferred"]).optional().describe("Filter by triage bucket"), | ||
| staleOnly: z.boolean().optional().describe("Only stale runs"), | ||
| freshOnly: z.boolean().optional().describe("Only fresh (not stale) runs"), | ||
| blockingRunCountAtLeast: z.number().optional().describe("Only runs blocking at least N other runs"), | ||
| today: z.string().optional().describe("ISO date for overdue computation"), |
Comment on lines
+659
to
+664
| dueOnOrBefore: z.string().optional().describe("Only runs with due date on or before threshold (YYYY-MM-DD)"), | ||
| triageBucketFilter: z.enum(["ready", "blocked", "deferred"]).optional().describe("Filter by triage bucket"), | ||
| staleOnly: z.boolean().optional().describe("Only stale runs"), | ||
| freshOnly: z.boolean().optional().describe("Only fresh (not stale) runs"), | ||
| blockingRunCountAtLeast: z.number().optional().describe("Only runs blocking at least N other runs"), | ||
| today: z.string().optional().describe("ISO date for overdue computation"), |
Comment on lines
+714
to
+735
| /** Filter configuration for saved queue views. */ | ||
| export const QueueViewFiltersSchema = { | ||
| workspaceId: z.string().optional().describe("Filter by workspace path"), | ||
| status: z.string().optional().describe("Filter by status prefix"), | ||
| label: z.string().optional().describe("Filter by exact label (case-insensitive)"), | ||
| assignee: z.string().optional().describe("Filter by exact assignee"), | ||
| includeArchived: z.boolean().optional().describe("Include archived runs"), | ||
| includeSnoozed: z.boolean().optional().describe("Include snoozed runs"), | ||
| pinnedOnly: z.boolean().optional().describe("Only pinned runs"), | ||
| snoozedOnly: z.boolean().optional().describe("Only snoozed runs"), | ||
| archivedOnly: z.boolean().optional().describe("Only archived runs"), | ||
| priorityFilter: z.enum(["low", "normal", "high", "urgent"]).optional().describe("Filter by priority"), | ||
| effortFilter: z.enum(["tiny", "small", "medium", "large", "huge"]).optional().describe("Filter by effort"), | ||
| blockedOnly: z.boolean().optional().describe("Only blocked runs"), | ||
| blockedByRunId: z.string().optional().describe("Only runs blocked by specific run ID"), | ||
| dueOnOrBefore: z.string().optional().describe("Only runs due on or before date (YYYY-MM-DD)"), | ||
| triageBucketFilter: z.enum(["urgent", "attention", "stale", "blocked", "ready"]).optional().describe("Filter by triage bucket"), | ||
| staleOnly: z.boolean().optional().describe("Only stale runs"), | ||
| freshOnly: z.boolean().optional().describe("Only fresh (not stale) runs"), | ||
| blockingRunCountAtLeast: z.number().optional().describe("Only runs blocking at least N other runs"), | ||
| today: z.string().optional().describe("ISO date for overdue computation"), | ||
| }; |
Comment on lines
+1566
to
+1569
| lazy_static::lazy_static! { | ||
| static ref QUEUE_VIEWS: Mutex<HashMap<String, deterministic_protocol::types::QueueView>> = | ||
| Mutex::new(HashMap::new()); | ||
| } |
| uuid = { workspace = true, features = ["v4"] } | ||
| chrono = { workspace = true } | ||
| rusqlite = { workspace = true } | ||
| lazy_static = "1.4" |
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
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
What's included
Protocol (deterministic-protocol)
QueueViewFilters,QueueViewSort,QueueViewtypesCreateQueueViewParams,UpdateQueueViewParams,DeleteQueueViewParams,GetQueueViewParams,ListQueueViewsParamsCreateQueueViewResult,UpdateQueueViewResult,DeleteQueueViewResult,ListQueueViewsResultQueueViewCreate,QueueViewUpdate,QueueViewDelete,QueueViewGet,QueueViewListDaemon (deterministic-daemon)
handle_queue_view_create: create view with name uniqueness checkhandle_queue_view_update: update view fields (name, description, filters, sort, limit)handle_queue_view_delete: delete viewhandle_queue_view_get: retrieve single view by IDhandle_queue_view_list: list all views with optional name filterMCP Tools (TypeScript)
create_queue_view- create a named saved viewupdate_queue_view- update view configurationdelete_queue_view- delete a viewget_queue_view- retrieve view definitionlist_queue_views- list all views (optional name_contains filter)Filter options
Views can include:
Sort options
Test Plan
Closes #51
Summary by Sourcery
Add support for deterministic saved queue views with CRUD operations across the protocol, daemon, and MCP tools.
New Features:
Enhancements:
Build: