Skip to content

Conversation

@katzdave
Copy link
Collaborator

@katzdave katzdave commented Aug 26, 2025

Use tool version + name as the key for flexibility to migrate these configs.

alexhancock and others added 5 commits August 26, 2025 09:53
Co-authored-by: David Katz <dkatz@squareup.com>
Remove special handling for token limit errors - all errors from providers
should now show the retry/summarize UI buttons in BaseChat
@katzdave katzdave changed the title Nest TODO State in session data DRAFT: Nest TODO State in session data Aug 26, 2025
Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

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

some preliminary thoughts...

}

/// Remove tool state for a specific tool and version
pub fn remove_tool_state(&mut self, tool_name: &str, version: &str) -> Option<Value> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this being used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No can prune; generally looking to get something where we're confident we could migrate the state, but without implementing all of that yet.

Maybe even we should go one layer up onto SessionMetadata as that's a bit sensitive to field changes now (although not much there but some constants yet).

accumulated_input_tokens: Some(50),
accumulated_output_tokens: Some(50),
todo_content: None,
session_data: crate::session::SessionData::new(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

might be easier if this was just nullable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I gave this a quick try, but seems not much different and then need to null check it.

content: String::new(),
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should not live here; sessions shouldn't know about specific tools

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Where would it go? Seems kind of session adjacent to me if we want it to be scoped to that + serialized and deserialized with the session.

None
};

let todo_content = if let Some(path) = session_file_path {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should figure out how to lift this out of agent; finding the path to find the config to read the state for a specific tool is quite cumbersome; it also strikes me that the state here should be per extension, not per tool; we have a read_todo and a write_todo tool, but unfortunately they are not part of an actual extension. thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I'm with you that this is the messiest part. I was somewhat thinking of 'TODO' as an abstract tool but you're right with the current implementation it's really 2 different tools. So maybe re-implementing it as a pseudo extension with multiple tool calls (looks very similar in shape to how you'd make an mcp server) would help.

I think it would be really cool to in general support session scoped storage for tools, although not really part of the mcp spec so probably only relevant/helpful for the internal tools we build.

I guess trying to figure out what we want in the shortest term pre-next release.

Seems like options are:

  1. Roll back todo -> session state PR. This would leave the tool as disabled, as the original agent storage had the leaky state issue in desktop.
  2. Write todo state to a different file alongside session_state.txt file and just parse it as a string (we're touching the filesystem already). Then at least we don't pollute session metadata and when we change to a fancier session storage we can just stop reading this file.
  3. Continue from this direction and build out a v1 of session tool state.

I'm thinking (2).

…todo-state

* 'dkatz/todo-state' of github.com:block/goose:
  first pass
  adding flight school video to blog post (#4352)
  docs: AGENTS.md is read by default (#4346)
* 'main' of github.com:block/goose:
  Refactor Extensions Install Modal (#4328)
  fix: url path trailing slash for custom-providers (#4345)
  docs: update available and onboarding providers list (#4356)
  Fix refresh by waiting for document ready rather than a timeout (#4355)
  docs: add Tetrate Agent Router Service to available providers (#4339)
@katzdave katzdave marked this pull request as draft August 26, 2025 22:21
* 'main' of github.com:block/goose:
  Fix eleven labs audio transcription and added more logging (#4358)
  feat: re-introduce session sharing (#4370)
  remove duplicate blog post (#4369)
  fix focus ring under form submits (#4332)
  Trigger docs deployment
  update tetrate blog date to today (#4368)
  tetrate signup: blog/launch post (#4313)
  Implement graceful recipe error handling with filename display (#4363)
  docs: airgapped operation by bypassing hermit for desktop app (#4063)
  remove Ollama card from welcome screen (#4348)
  feat: initial implementation of extension malware check (#4272)
  Add Tetrate Agent Router Service to Provider Registry (#4354)
  Goose Simple Compact UX (#4202)
@katzdave katzdave changed the title DRAFT: Nest TODO State in session data Nest TODO State in session data Aug 28, 2025
@katzdave katzdave marked this pull request as ready for review August 28, 2025 14:52
@katzdave katzdave merged commit ef57f50 into main Aug 28, 2025
11 checks passed
@katzdave katzdave deleted the dkatz/todo-state branch August 28, 2025 18:11
michaelneale added a commit that referenced this pull request Aug 29, 2025
* main:
  new recipe to lint-check my code (#4416)
  removing a leftover syntax error (#4415)
  Iand/updating recipe validation workflow (#4413)
  Iand/updating recipe validation workflow (#4410)
  Fix (Ollama provider): Unsupported operation: streaming not implemented (#4303)
  change databricks default to claude sonnet 4 (#4405)
  Iand/updating recipe validation workflow (#4406)
  Add metrics for recipe metadata in scheduler, UI, and CLI (#4399)
  Iand/updating recipe validation workflow (#4403)
  making small updates to recipe validation workflow (#4401)
  Automate OpenRouter API Key Distribution for External Recipe Contributors (#3198)
  Enhance `convert_path_with_tilde_expansion` to handle Windows (#4390)
  make sure all cookbook recipes have a title and version, but no id (#4395)
  Nest TODO State in session data (#4361)
  Fast model falls back to regular (#4375)
  Update windows instructions (#4333)
lifeizhou-ap added a commit that referenced this pull request Aug 29, 2025
* main: (40 commits)
  new recipe to lint-check my code (#4416)
  removing a leftover syntax error (#4415)
  Iand/updating recipe validation workflow (#4413)
  Iand/updating recipe validation workflow (#4410)
  Fix (Ollama provider): Unsupported operation: streaming not implemented (#4303)
  change databricks default to claude sonnet 4 (#4405)
  Iand/updating recipe validation workflow (#4406)
  Add metrics for recipe metadata in scheduler, UI, and CLI (#4399)
  Iand/updating recipe validation workflow (#4403)
  making small updates to recipe validation workflow (#4401)
  Automate OpenRouter API Key Distribution for External Recipe Contributors (#3198)
  Enhance `convert_path_with_tilde_expansion` to handle Windows (#4390)
  make sure all cookbook recipes have a title and version, but no id (#4395)
  Nest TODO State in session data (#4361)
  Fast model falls back to regular (#4375)
  Update windows instructions (#4333)
  feat: linux computer control for android (termux) (#3890)
  feat: Added scroll state support for chat-session-list navigation (#4360)
  docs: typo fix (#4376)
  blog: goose janitor (#4131)
  ...
thisispete pushed a commit that referenced this pull request Aug 30, 2025
Co-authored-by: Alex Hancock <alexhancock@block.xyz>
dorien-koelemeijer pushed a commit to dorien-koelemeijer/goose that referenced this pull request Sep 2, 2025
Co-authored-by: Alex Hancock <alexhancock@block.xyz>
Signed-off-by: Dorien Koelemeijer <dkoelemeijer@squareup.com>
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.

4 participants