Derive a conversation id for tasks instead of using the task file path - #84
Open
JoseWalker wants to merge 1 commit into
Open
Derive a conversation id for tasks instead of using the task file path#84JoseWalker wants to merge 1 commit into
JoseWalker wants to merge 1 commit into
Conversation
TaskHandler passed the task id straight to the agent as the conversation id.
A task id is the absolute path of its markdown file, so it reached
FileSystemChatMemoryRepository.resolveFile, which builds
conversations/chat-{id}.yaml. With separators still in the string that is not
one file but a directory tree mirroring the task's own location on disk: the
memory is scattered, findConversationIds() never lists it because it only looks
at the top level, and on Windows the colon in the drive letter makes the name
invalid.
Derive the id from the last two segments of the task path instead, so
{date}/{time}-{name}.md becomes task-{date}-{time}-{name} and
recurring/{name}.md becomes task-recurring-{name}. Keeping the date directory
matters: without it two tasks with the same name and time on different days
would share one conversation. The path is normalized first and anything outside
the character set task file names already use is replaced, so the result is
always a single safe file name component. Deriving nothing throws rather than
falling back to the raw path, which would quietly reintroduce the bug.
TaskHandler had no test coverage; add TaskHandlerTest. One assertion in
TaskManagerTest pinned the old format and is updated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
TaskHandler.executeTaskpasses the task id straight to the agent as the conversation id:A task's id is the absolute path of its markdown file —
FileSystemTaskRepository.savereturnspath.toAbsolutePath().toString(). That value ends up inFileSystemChatMemoryRepository.resolveFile, which buildsconversationsDir.resolve("chat-" + conversationId + ".yaml"). Because the string still containspath separators, the result is not one file but a directory tree mirroring the task's own location
on disk:
Consequences:
findConversationIds(), which only looks at the top level, so a taskconversation is invisible in the UI;
The fix
A task conversation id is now derived from the last two segments of the task path —
task-{date}-{name}, ortask-recurring-{name}for recurring tasks:Keeping the date directory matters: task files are
{date}/{time}-{name}.md, so dropping it wouldmake two tasks with the same name and time on different days share one conversation.
The path is normalized first, and anything outside the character set task file names already use
(
[a-zA-Z0-9._-]) is replaced, so the result is always a single safe file-name component. If no idcan be derived the method throws rather than falling back to the raw path, which would quietly
recreate the bug.
Notes for review
path-shaped id keeps its files but is no longer found under the new id. Given those files are
currently unreachable from the UI anyway, migrating them seemed worse than leaving them; happy to
add a migration if you would rather.
TaskManagerTestpinned the old format and is updated accordingly.TaskHandlerhad no tests; this adds 9.FileSystemChatMemoryRepositoryaccepts anyconversation id without containment checks — that is a separate concern and I am filing it on its
own.
Verified with
./gradlew test --rerun-tasks(156 tests, 0 failures) and the Modulith / ArchUnit gates.