Add to-do list functionality to QCLI #2533

kiran-garre · 2025-08-08T18:37:07Z

Adds a to-do list tool (called todo_list) with several commands that allow Q to create a to-do list and update it as it completes tasks, along with a slash command (/todos) that allows users to view and manage their in-progress to-do lists.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Current functionality: - Supports interally creating a todo list and completing tasks - Displays list to the user - Can be somewhat reliably invoked by explicitly asking Q for a todo list - Q sometimes automatically generates a todo list on some prompts Future chagnes: - Add context so todo lists can be used across sessions - Allow Q to add and remove tasks

Updates: - Users can select past to-do lists to complete - Q will load the selected to-do list and finish the remaining tasks Future changes - Improve consistency of todo_list tool - Add more checks for argument validity - Add more detailed context to todo_lists for cross-session usage

…mand of todo_list

Updates: - To-do lists now only use the database rather than the filesystem for managing past and unfinished tasks

Also: - Ran formatter - Resolved most relevant clippy issues - Switched from printing to error handling in todos.rs - Changed /todos subcommand name from "select" to "resume"

Also: - Formatted - Added command and subcommand descriptions - Added newlines and things for cleanliness

Updates: - The model now sees the updated todo list after invoking the todo_list tool - Todo lists shouldn't get stuck on marking a task as complete (in theory)

Updates: - As tasks are completed, only the completed tasks and next task are shown - Full todo list is only displayed on creation and after finishign - /todos show was removed (view is enough)

Updates: - `/todos show` displays the current todo list - Note: This is different from the previous /todos show

Updates: - Todo lists now seem to work as they did before - Added spinner for resuming Future changes: - Clean up and refactor code before merge

…an-garre/todo-list

dingfeli · 2025-08-08T22:11:18Z

build-config/buildspec-linux.yml

@@ -28,10 +28,10 @@ phases:
      - mise install
      - eval "$(mise activate bash --shims)"
      # Install python deps
-      - pip3 install -r scripts/requirements.txt
+      - pip3 install -r build-scripts/requirements.txt


I think there are some errors here with how the merge conflict is handled.
This is the current state of the file: https://github.com/aws/amazon-q-developer-cli/blob/main/build-config/buildspec-linux.yml#L31

dingfeli · 2025-08-08T22:12:22Z

build-config/buildspec-macos.yml

@@ -25,11 +25,11 @@ phases:
      - eval "$(mise activate zsh --shims)"


dingfeli · 2025-08-08T23:07:09Z

crates/chat-cli/src/cli/chat/tools/todo.rs

+#[derive(Debug, Clone, Deserialize)]
+#[serde(tag = "command")]
+pub enum TodoInput {
+    #[serde(rename = "create")]


I think you can just use #[serde(rename_all = "camelCase")].

dingfeli · 2025-08-08T23:43:48Z

crates/chat-cli/src/database/mod.rs

+    pub fn get_current_todo_id(&self) -> Result<Option<String>, DatabaseError> {
+        self.get_entry(Table::State, "current_todo_id")
+    }
+
+    pub fn set_current_todo_id(&self, id: &str) -> Result<(), DatabaseError> {
+        self.set_entry(Table::State, "current_todo_id", id)?;
+        Ok(())
+    }


I think there is an edge case here.

What happens if there are multiple instances of q chat running at the same time. Is the value going to be clobbered / overridden?

If it is, there are a couple of paths forward:

We make the look up key dynamic. We can derive it from conversation id + some sort of suffix. The downside here is that we might need some sort of logic to clean up to rid of ones that are stale. (Maybe introduce a TTL or something)

Make store this id somewhere else on the conversation state, serializable and deserializable.

dingfeli · 2025-08-08T23:45:33Z

crates/chat-cli/src/cli/chat/mod.rs

+                    .set_next_user_message(state.user_input_message.content)
+                    .await;
+            },
+            Err(_) => bail!("Todo could not be resumed"),


Might want to propagate the error upwards here.

dingfeli · 2025-08-08T23:58:04Z

crates/chat-cli/src/cli/chat/cli/todos.rs

+                                None
+                            };
+
+                            let todo_result = session.resume_todo(os, &entries[index].id).await;


Do we want to just make resume_todo return a Result<ChatState, ChatError> and have the spinner logic within resume_todo.

dingfeli · 2025-08-09T00:02:46Z

crates/chat-cli/src/cli/chat/mod.rs

+        let mut response = match self.send_message(os, conv_state, request_metadata_clone, None).await {
+            Ok(res) => res,
+            Err(_) => bail!("Turn summary could not be created"),
+        };
+
+        // Since this is an internal tool call, manually handle the tool requests from Q
+        let mut tool_uses = Vec::new();
+        while let Some(res) = response.recv().await {
+            let res = res?;
+            match res {
+                parser::ResponseEvent::AssistantText(_) => (),
+                parser::ResponseEvent::EndStream { .. } => break,
+                parser::ResponseEvent::ToolUse(e) => tool_uses.push(e),
+                parser::ResponseEvent::ToolUseStart { .. } => (),
+            }
+        }
+        if !tool_uses.is_empty() {
+            self.validate_tools(os, tool_uses).await?;
+        }
+
+        // FIX: hacky? not really sure how this works honestly LOL
+        self.conversation.reset_next_user_message();


This seems like a deviation of the established control flow. Is there a reason why we can't just utilize the existing event loop for this?

dingfeli · 2025-08-09T00:07:41Z

crates/chat-cli/src/cli/chat/cli/todos.rs

+                            let list = match TodoState::load(os, &entries[index].id) {
+                                Ok(list) => list,
+                                Err(_) => {
+                                    return Err(ChatError::Custom("Could not load the selected to-do list".into()));
+                                },
+                            };


let list = TodoState::load(os, &entries[index].id).ok_or(ChatError::Custom("Could not load the selected to-do list".into())?;

Though looking at fuzzy_select_todos it is not clear to me whether or not this should be an error.

dingfeli · 2025-08-09T00:12:42Z

crates/chat-cli/src/cli/chat/cli/todos.rs

+                    let state = match TodoState::load(os, &id) {
+                        Ok(s) => s,
+                        Err(_) => {
+                            return Err(ChatError::Custom("Could not load current to-do list".into()));
+                        },
+                    };
+                    match state.display_list(&mut session.stderr) {
+                        Ok(_) => execute!(session.stderr, style::Print("\n"))?,
+                        Err(_) => {
+                            return Err(ChatError::Custom("Could not display current to-do list".into()));
+                        },
+                    };


Do we want to propagate the error here as well.

Also a less verbose way of writing this would be:

let state = TodoSate::load(os, &id).map_err(|_| ChatError::Custom("Could not load current to-do list".into())?;

dingfeli · 2025-08-09T00:29:05Z

crates/chat-cli/src/cli/chat/cli/todos.rs

+                };
+                let mut cleared_one = false;
+                for (id, value) in entries.iter() {
+                    let temp_struct = match value.as_str() {


Perhaps a better name here would be item_status or todo_status or something more descriptive.

Also, instead of turning to value to str and turning the str to TodoState, maybe we can just use serde_json::from_value.

Updates: - Removed separation between `create_summary_request` and `resume_todo`; resuming a todo is now done in a single function (`resume_todo_request`) and is handled by the main event loop - Removed `current_id` entry in the database for tracking state; the model is now responsible for providing the id of the current todo list - Removed `/todos current` because the current todo list id is not tracked - Added more error propagation in slash command - Unstaged build script files

brandonskiser · 2025-08-12T19:55:05Z

crates/chat-cli/src/cli/chat/tools/tool_index.json

+            "type": "string"
+          }
+        },
+        "task_description": {


Is this the description for the todo list as a whole? It seems that a todo list is some number of tasks (so the create command requires a tasks array), but task_description is only a single parameter?

brandonskiser · 2025-08-12T19:57:50Z

crates/chat-cli/src/cli/chat/tools/tool_index.json

+          }
+        },
+        "new_description": {
+          "description": "Optional parameter of `add` and `remove` containing a new task description. Use this when the updated set of tasks significantly change the goal or overall procedure of the task.",


What is the definition of a task? It seems that tasks and task are being used interchangeably when the singular task is actually referring to an instance of a todo list that contains tasks.

brandonskiser · 2025-08-12T20:08:23Z

crates/chat-cli/src/database/mod.rs

+        Ok(())
+    }
+
+    pub fn get_all_todos(&self) -> Result<Map<String, Value>, DatabaseError> {


Could this map to a TodoState instead of Value?

brandonskiser · 2025-08-12T20:13:27Z

crates/chat-cli/src/database/mod.rs

I wonder how much sense it makes to have todo lists stored in state (accessible globally) versus in a current working directory (e.g. inside .amazonq/todo_lists/).

What use cases do we have where it makes sense that todo lists should be stored in state regardless of cwd? I could imagine things like MCP flows that never use the filesystem and instead just call API's like updating docs, etc. but then that brings into question how to handle todo lists that are working on feature changes, and then you try to resume a todo list in a completely different directory.

brandonskiser · 2025-08-12T20:17:49Z

crates/chat-cli/src/cli/chat/cli/todos.rs

+    /// Convert all to-do list state entries to displayable entries
+    fn get_descriptions_and_statuses(os: &Os) -> Result<Vec<TodoDisplayEntry>> {
+        let mut out = Vec::new();
+        let entries = os.database.get_all_todos()?;


This database.get_all_todos api could instead be (Vec<TodoState>, Vec<DatabaseError>) so we just get a list of valid todos, and todos that failed to load or deserialize for whatever reason, so we don't have to duplicate this deser logic multiple times maybe

crates/chat-cli/src/cli/chat/mod.rs

…tem store Updates: - Todo lists are now local to the user's cwd and are stored in files - Changed names from 'task_description'/'tasks' to 'todo_list_description'/'tasks'

brandonskiser

Appears to be some issue with the trust behavior with /tools - should be trusted

Also can you add tab completion? Add it to the array here - https://github.com/aws/amazon-q-developer-cli/blob/main/crates/chat-cli/src/cli/chat/prompt.rs#L43

brandonskiser · 2025-08-13T19:15:12Z

crates/chat-cli/src/cli/chat/tools/todo.rs

+
+#[derive(Debug, Clone, Deserialize)]
+#[serde(tag = "command", rename_all = "camelCase")]
+pub enum TodoInput {


nit - call this just TodoList to be aligned with the actual tool name in the tool index. Also add some small doc comments to these pub types.

also put the impl right below the enum definition

brandonskiser · 2025-08-13T19:17:34Z

crates/chat-cli/src/cli/chat/tools/todo.rs

+
+    /// Displays a single empty or marked off to-do list task depending on
+    /// the completion status
+    fn queue_next_without_newline(output: &mut impl Write, task: String, completed: bool) -> Result<()> {


In general, if functions don't require self then they should probably just be free functions, e.g. delete_todo parameterized on an id doesn't make sense to be associated with a single in-memory todo

brandonskiser · 2025-08-13T19:18:56Z

crates/chat-cli/src/cli/chat/tools/todo.rs

+    }
+
+    pub fn id_to_path(os: &Os, id: &str) -> Result<PathBuf> {
+        Ok(os.env.current_dir()?.join(TODO_LIST_DIR).join(id))


Wonder if we should also store these with a json extension? Just a personal preference of mine (for getting easier IDE support if for whatever reason we need to check these files directly)

brandonskiser · 2025-08-13T19:22:54Z

crates/chat-cli/src/cli/chat/tools/todo.rs

+}
+
+#[derive(Debug, Default, Serialize, Deserialize, Clone)]
+pub struct TodoState {


TodoListState ?

brandonskiser · 2025-08-13T20:59:26Z

crates/chat-cli/src/cli/chat/tools/todo.rs

+                if has_duplicates(remove_indices) {
+                    bail!("Removal indices must be unique")
+                } else if remove_indices.iter().any(|i| *i > state.tasks.len()) {
+                    bail!("Index is out of bounds");


We could prob make the error messaging here better for the model in case it hallucinates, e.g. tell it which index is out of bounds or give the length of tasks

brandonskiser · 2025-08-13T21:03:14Z

crates/chat-cli/src/cli/chat/tools/todo.rs

+
+    /// Loads a TodoState with the given id
+    pub async fn load(os: &Os, id: &str) -> Result<Self> {
+        let Ok(state_str) = os.fs.read_to_string(Self::id_to_path(os, id)?).await else {


Can we print the actual error here in the error message? This could fail for a reason other than the to-do list not existing

…o states Updates: - Todo list state files now have ".json" extensions

brandonskiser · 2025-08-13T23:49:44Z

crates/chat-cli/src/cli/chat/mod.rs

+        // Create for cleaner error handling for todo lists
+        // This is more of a convenience thing but is not required, so the Result
+        // is ignored
+        let _ = TodoListState::init_dir(os).await;


Any todo list directory creation should really be happening with the todo list code that depends on it, having it here is error prone should this ever be moved in the future.

brandonskiser and others added 18 commits July 1, 2025 15:27

feat: update build scripts to build qchat (#2198)

264c308

Slash command + persistence in progress commit

30fc467

Add append-only context and "modified files" fields to "complete" com…

2a09ea9

…mand of todo_list

refactor: Change to-do lists to use database over filesystem

398bdb4

Updates: - To-do lists now only use the database rather than the filesystem for managing past and unfinished tasks

feat: Add view subcommand for /todos

c000ffb

Also: - Ran formatter - Resolved most relevant clippy issues - Switched from printing to error handling in todos.rs - Changed /todos subcommand name from "select" to "resume"

feat: Add clear-finished subcommand for /todos

302cecd

Also: - Formatted - Added command and subcommand descriptions - Added newlines and things for cleanliness

feat: Add add and remove functionality for to-do lists

a9501e7

fix: Add feedback after calling todo_list commands

814a96a

Updates: - The model now sees the updated todo list after invoking the todo_list tool - Todo lists shouldn't get stuck on marking a task as complete (in theory)

feat: Cleaned up todo list UX and removed /todos show

216ce00

Updates: - As tasks are completed, only the completed tasks and next task are shown - Full todo list is only displayed on creation and after finishign - /todos show was removed (view is enough)

feat: Add /todos show

714474a

Updates: - `/todos show` displays the current todo list - Note: This is different from the previous /todos show

trying to merge changes

06837f2

fix: Fix all merge conflicts, back to working state

1a4e7e1

Updates: - Todo lists now seem to work as they did before - Added spinner for resuming Future changes: - Clean up and refactor code before merge

chore: Resolve merge conflicts, run formatter and clippy

3299478

chore: Fix real clippy errors (hopefully)

e31c7ed

Merge branch 'main' of github.com:aws/amazon-q-developer-cli into kir…

058cc62

…an-garre/todo-list

chore: Run formatter

12a70b5

kiran-garre requested review from brandonskiser and dingfeli August 8, 2025 18:37

kiran-garre added 2 commits August 8, 2025 12:35

chore: Remove old/unrelated files

ef44517

chore: Remove old/unrelated files again

cbbfa20

dingfeli reviewed Aug 9, 2025

View reviewed changes

kiran-garre added 6 commits August 11, 2025 14:48

chore: Fix typo in tool_index.json

5e76bc1

chore: Remove buildspec files

7f91788

chore: Edit buildspec files to be identical to main

70423f9

chore: Remove debugging in todos.rs

81b1dac

chore: Merging changes from main

e7ea8af

brandonskiser reviewed Aug 12, 2025

View reviewed changes

kiran-garre added 4 commits August 12, 2025 16:55

refactor: Convert from global database store to per-directory filesys…

d7aa647

…tem store Updates: - Todo lists are now local to the user's cwd and are stored in files - Changed names from 'task_description'/'tasks' to 'todo_list_description'/'tasks'

merge commit

ee6b932

Merge branch 'main' into kiran-garre/todo-list

6d00e0f

fix: Update dependencies to fix slab issue

4b684f9

brandonskiser reviewed Aug 13, 2025

View reviewed changes

kiran-garre added 4 commits August 13, 2025 15:29

chore: Change variable names, add comments, add file extension to tod…

f2fc3d1

…o states Updates: - Todo list state files now have ".json" extensions

Merge commit

2fcbe56

Removed merge markers from Cargo.lock

9e0b732

Fixed Cargo.lock

030c79a

brandonskiser approved these changes Aug 13, 2025

View reviewed changes

dingfeli approved these changes Aug 14, 2025

View reviewed changes

		@@ -25,11 +25,11 @@ phases:
		- eval "$(mise activate zsh --shims)"

Add to-do list functionality to QCLI #2533

Are you sure you want to change the base?

Add to-do list functionality to QCLI #2533

Conversation

kiran-garre commented Aug 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brandonskiser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!