Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions apps/staged/src-tauri/src/session_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1527,6 +1527,7 @@ pub(crate) async fn resume_or_queue_session_for_store(
branch_id: Option<String>,
acp_config_selection: Option<store::AcpConfigSelection>,
) -> Result<ResumeSessionOutcome, String> {
ensure_session_chat_writable(&store, &session_id)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Gate queued-message resumes for child notes

Gate send_queued_session_message_for_store and the automatic queue drain as well as this direct-resume wrapper. Existing installations can already contain follow-ups queued for a running child-note session; when that turn finishes, drain_queued_message_for_session calls the ungated resume_session_for_store directly, so the queued prompt starts a new turn despite the chat now being read-only. The public send_queued_session_message command has the same bypass for manually selected queued messages.

Useful? React with 👍 / 👎.

let session = store
.get_session(&session_id)
.map_err(|e| e.to_string())?
Expand Down Expand Up @@ -2411,6 +2412,7 @@ pub(crate) async fn send_session_message_now_for_store(
message: &str,
image_ids: &[String],
) -> Result<ContinueOutcome, String> {
ensure_session_chat_writable(store, session_id)?;
match route_continue_session(
store,
registry,
Expand Down Expand Up @@ -2808,6 +2810,32 @@ fn continue_branch_and_type(
Ok((lookup(branch_id)?, None))
}

/// The error a user's chat message gets when its session's note is a child
/// aggregated under a project note; see [`ensure_session_chat_writable`].
pub(crate) const HIDDEN_CHILD_NOTE_CHAT_ERROR: &str =
"This chat is read-only: its note is part of \
a project note and is not shown in the branch's notes, so it can't take new messages.";

/// Refuse a user's chat message for a session whose note is a child
/// aggregated under a project note (`Note::parent_project_note_id`). Such
/// notes are hidden from the branch timeline and the project notes list
/// (see `Store::list_notes_for_branch`), so their chats are read-only: the
/// project session that spawned them owns any follow-up. Gates the
/// user-facing entry points only (resume, queue, send now, desktop and web);
/// the project MCP server's `continue_repo_session` reaches
/// `route_continue_session` directly and is unaffected.
pub(crate) fn ensure_session_chat_writable(store: &Store, session_id: &str) -> Result<(), String> {
let note = store
.get_note_by_session(session_id)
.map_err(|e| e.to_string())?;
match note {
Some(note) if note.parent_project_note_id.is_some() => {
Err(HIDDEN_CHILD_NOTE_CHAT_ERROR.to_string())
}
_ => Ok(()),
}
}

#[tauri::command]
pub fn queue_session_message(
store: tauri::State<'_, Mutex<Option<Arc<Store>>>>,
Expand All @@ -2817,6 +2845,7 @@ pub fn queue_session_message(
branch_id: Option<String>,
) -> Result<store::QueuedSessionMessage, String> {
let store = get_store(&store)?;
ensure_session_chat_writable(&store, &session_id)?;
store
.add_queued_session_message(
&session_id,
Expand Down Expand Up @@ -14967,6 +14996,83 @@ mod tests {
assert_eq!(std::fs::read(&media[0].target).unwrap(), PNG);
}

/// A child note's session (`parent_project_note_id` set) and a
/// standalone note's session, both finished, for the chat gate tests.
fn child_and_standalone_note_sessions(
store: &Store,
branch: &store::Branch,
) -> (String, String) {
let parent = store::ProjectNote::new(&branch.project_id, "Parent", "aggregated");
store.create_project_note(&parent).unwrap();

let child_session = store::Session::new_queued("child").with_branch(&branch.id);
store.create_session(&child_session).unwrap();
let child = store::Note::new(&branch.id, "Child", "child body")
.with_session(&child_session.id)
.with_parent_project_note(&parent.id);
store.create_note(&child).unwrap();

let standalone_session = store::Session::new_queued("standalone").with_branch(&branch.id);
store.create_session(&standalone_session).unwrap();
let standalone = store::Note::new(&branch.id, "Standalone", "top-level body")
.with_session(&standalone_session.id);
store.create_note(&standalone).unwrap();

(child_session.id, standalone_session.id)
}

/// The chat of a note hidden from the branch timeline (a child aggregated
/// under a project note) is read-only; every other session's chat is not.
#[test]
fn hidden_child_note_chat_refuses_user_messages() {
let (store, branch) = setup_branch_store();
let (child_session_id, standalone_session_id) =
child_and_standalone_note_sessions(&store, &branch);

assert_eq!(
ensure_session_chat_writable(&store, &child_session_id),
Err(HIDDEN_CHILD_NOTE_CHAT_ERROR.to_string())
);
assert_eq!(
ensure_session_chat_writable(&store, &standalone_session_id),
Ok(())
);

// A session with no note at all (a commit session, say) is untouched.
let commit_session = store::Session::new_queued("commit").with_branch(&branch.id);
store.create_session(&commit_session).unwrap();
assert_eq!(
ensure_session_chat_writable(&store, &commit_session.id),
Ok(())
);
}

/// The gate sits in the shared body of `send_session_message_now`, ahead
/// of the routing on status, so a bypassed composer is refused with the
/// same error on both the desktop and the web path.
#[tokio::test(flavor = "current_thread")]
async fn send_now_refuses_a_hidden_child_note_chat_before_routing() {
let (store, branch) = setup_branch_store();
let registry = test_registry();
let (child_session_id, _) = child_and_standalone_note_sessions(&store, &branch);

let err = send_session_message_now_for_store(
&store,
&registry,
&child_session_id,
"follow up",
&[],
)
.await
.unwrap_err();
assert_eq!(err, HIDDEN_CHILD_NOTE_CHAT_ERROR);
// Nothing was queued or delivered on its behalf.
assert!(store
.list_queued_session_messages(&child_session_id)
.unwrap()
.is_empty());
}

#[test]
fn note_timeline_entries_include_children_annotated_with_their_parent() {
let (store, branch) = setup_branch_store();
Expand Down
18 changes: 18 additions & 0 deletions apps/staged/src-tauri/src/web_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3783,6 +3783,7 @@ pub(crate) async fn dispatch(
let content: String = arg(&args, "content")?;
let image_ids: Option<Vec<String>> = opt_arg(&args, "imageIds")?;
let branch_id: Option<String> = opt_arg(&args, "branchId")?;
session_commands::ensure_session_chat_writable(&store, &session_id)?;
let message = store
.add_queued_session_message(
&session_id,
Expand Down Expand Up @@ -5042,6 +5043,23 @@ mod tests {
);
}

/// Queueing a chat message for a hidden child note's session is refused
/// on the web path as on the desktop one: the arm runs the shared gate
/// before the insert.
#[test]
fn web_queue_session_message_gates_hidden_child_note_chats() {
let arm = dispatch_arm_body(include_str!("web_server.rs"), "queue_session_message");
let gate = arm
.find("session_commands::ensure_session_chat_writable(")
.unwrap_or_else(|| {
panic!("the web `queue_session_message` arm must run the chat gate: {arm}")
});
let insert = arm
.find("add_queued_session_message(")
.unwrap_or_else(|| panic!("the web `queue_session_message` arm must queue: {arm}"));
assert!(gate < insert, "the gate must run before the insert: {arm}");
}

/// A web resume goes through the desktop command's body, `prepare_resume`
/// included, so it waits out a Stop's backfill and refuses a
/// `disconnected` row as the desktop one does, instead of a copy that
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
import { flushSync, mount, tick, unmount } from 'svelte';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { BranchNote, Session } from '../../types';

const { branchNote } = vi.hoisted(() => ({
/** What `getBranchNoteBySession` answers for the mounted session. */
branchNote: { current: null as BranchNote | null, fail: false },
}));

// Heavy leaf components are stubbed out: these tests exercise whether the
// pane offers a composer at all, not the composer's internals.
vi.mock('./ChatComposer.svelte', async () => {
const { default: PromptInputStub } = await import('./testStubs/PromptInputStub.svelte');
return { default: PromptInputStub, composerControlClass: '' };
});
vi.mock('../agents/AcpFixedConfigPicker.svelte', () => import('./testStubs/EmptyStub.svelte'));
vi.mock('./PlanCard.svelte', () => import('./testStubs/EmptyStub.svelte'));
vi.mock('./PipelineSteps.svelte', () => import('./testStubs/EmptyStub.svelte'));
vi.mock('./tool-calls/ToolCallCard.svelte', () => import('./testStubs/EmptyStub.svelte'));
vi.mock('./tool-calls/ToolCallHeader.svelte', () => import('./testStubs/EmptyStub.svelte'));
vi.mock('./NoteActivityCard.svelte', () => import('./testStubs/EmptyStub.svelte'));
vi.mock(
'../../shared/markdown/DiagramViewerModal.svelte',
() => import('./testStubs/EmptyStub.svelte')
);
vi.mock('../branches/dragDrop', () => ({ subscribeDragDrop: () => () => {} }));
vi.mock('../agents/agent.svelte', () => ({ agentState: { providers: [] } }));
vi.mock('../settings/preferences.svelte', () => ({ setAcpConfigPref: vi.fn() }));
vi.mock('../../transport', async (importOriginal) => ({
...(await importOriginal<typeof import('../../transport')>()),
listenToEvent: () => () => {},
}));
vi.mock('../../shared/draftCache', () => ({
createDraftWriteScheduler: () => ({
scheduleSave: vi.fn(),
saveNow: vi.fn(async () => {}),
clearNow: vi.fn(async () => {}),
flush: vi.fn(async () => {}),
}),
loadDraft: async () => null,
}));
vi.mock('../../commands', async (importOriginal) => ({
...(await importOriginal<typeof import('../../commands')>()),
getSession: async (id: string) =>
({
id,
status: 'completed',
completionReason: 'interrupted',
provider: null,
workingDir: null,
}) as unknown as Session,
getLatestSessionMessages: async () => ({ data: [] }),
getSessionMessages: async () => ({ data: [] }),
getSessionMessagesSince: async () => [],
getSessionAcpMetadataMessages: async () => [],
getSessionAcpMetadataMessagesSince: async () => [],
listQueuedSessionMessages: async () => [],
getBranchNoteBySession: async () => {
if (branchNote.fail) throw new Error('lookup failed');
return branchNote.current;
},
}));

import SessionChatPane from './SessionChatPane.svelte';
import { reactiveProps } from './testStubs/reactiveProps.svelte';

function note(sessionId: string, parentProjectNoteId: string | null): BranchNote {
return {
id: `note-${sessionId}`,
branchId: 'branch-1',
sessionId,
title: 'Note',
content: 'body',
createdAt: 1,
updatedAt: 1,
completedAt: 1,
suggestedNextCommitStep: null,
suggestedNextNoteStep: null,
parentProjectNoteId,
subtype: null,
readAt: null,
};
}

function mountPane(sessionId: string) {
const props = reactiveProps({ active: true, sessionId, hashtagItems: [] });
const target = document.createElement('div');
document.body.appendChild(target);
const pane = mount(SessionChatPane, { target, props });
return { pane, props };
}

/** Let the pane's async session and note loads and effect flushes settle. */
async function settle() {
for (let i = 0; i < 8; i++) {
await tick();
}
flushSync();
}

const composer = () => document.querySelector('[data-testid="prompt-input"]');
const readOnlyNotice = () => document.querySelector('[data-testid="read-only-chat-notice"]');
const resumeButton = () =>
Array.from(document.querySelectorAll('button')).find((b) => b.textContent?.trim() === 'Resume');

describe('SessionChatPane for a hidden child note', () => {
beforeEach(() => {
document.body.innerHTML = '';
branchNote.current = null;
branchNote.fail = false;
vi.spyOn(console, 'error').mockImplementation(() => {});
});

it('replaces the composer and Resume with a read-only notice', async () => {
branchNote.current = note('child', 'project-note-1');
const { pane } = mountPane('child');
flushSync();
await settle();

expect(composer()).toBeNull();
expect(resumeButton()).toBeUndefined();
expect(readOnlyNotice()?.textContent).toContain('read-only');
expect(readOnlyNotice()?.textContent).toContain('project note');

unmount(pane);
});

it("keeps the composer for a note that is shown in the branch's notes", async () => {
branchNote.current = note('standalone', null);
const { pane } = mountPane('standalone');
flushSync();
await settle();

expect(composer()).not.toBeNull();
expect(resumeButton()).toBeDefined();
expect(readOnlyNotice()).toBeNull();

unmount(pane);
});

it('keeps the composer when the note lookup fails or finds no note', async () => {
branchNote.fail = true;
const { pane, props } = mountPane('no-note');
flushSync();
await settle();
expect(composer()).not.toBeNull();
expect(readOnlyNotice()).toBeNull();

// A commit session has no note; the chat is writable.
branchNote.fail = false;
props.sessionId = 'commit';
flushSync();
await settle();
expect(composer()).not.toBeNull();
expect(readOnlyNotice()).toBeNull();

unmount(pane);
});

it('drops the notice when the pane moves on to a writable session', async () => {
branchNote.current = note('child', 'project-note-1');
const { pane, props } = mountPane('child');
flushSync();
await settle();
expect(readOnlyNotice()).not.toBeNull();

branchNote.current = note('standalone', null);
props.sessionId = 'standalone';
flushSync();
await settle();
expect(readOnlyNotice()).toBeNull();
expect(composer()).not.toBeNull();

unmount(pane);
});
});
Loading
Loading