Skip to content

Derive a conversation id for tasks instead of using the task file path - #84

Open
JoseWalker wants to merge 1 commit into
ClawRunr:mainfrom
JoseWalker:fix/task-conversation-id
Open

Derive a conversation id for tasks instead of using the task file path#84
JoseWalker wants to merge 1 commit into
ClawRunr:mainfrom
JoseWalker:fix/task-conversation-id

Conversation

@JoseWalker

Copy link
Copy Markdown
Contributor

TaskHandler.executeTask passes the task id straight to the agent as the conversation id:

TaskResult result = agent.prompt(taskId, agentInput, TaskResult.class);

A task's id is the absolute path of its markdown file — FileSystemTaskRepository.save returns
path.toAbsolutePath().toString(). That value ends up in
FileSystemChatMemoryRepository.resolveFile, which builds
conversationsDir.resolve("chat-" + conversationId + ".yaml"). Because the string still contains
path separators, the result is not one file but a directory tree mirroring the task's own location
on disk:

workspace/conversations/chat-/Users/me/JavaClaw/workspace/tasks/2026-08-11/103000-buy_milk.md.yaml

Consequences:

  • every task's memory is scattered instead of living in one flat conversation file;
  • none of it is listed by findConversationIds(), which only looks at the top level, so a task
    conversation is invisible in the UI;
  • on Windows the colon in the drive letter makes the file name invalid outright.

The fix

A task conversation id is now derived from the last two segments of the task path —
task-{date}-{name}, or task-recurring-{name} for recurring tasks:

workspace/tasks/2026-08-11/103000-buy_milk.md  ->  task-2026-08-11-103000-buy_milk
workspace/tasks/recurring/water_plants.md      ->  task-recurring-water_plants

Keeping the date directory matters: task files are {date}/{time}-{name}.md, so dropping it would
make 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 id
can be derived the method throws rather than falling back to the raw path, which would quietly
recreate the bug.

Notes for review

  • Existing task conversations start fresh. Any memory already written under the old
    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.
  • One assertion in TaskManagerTest pinned the old format and is updated accordingly.
  • TaskHandler had no tests; this adds 9.
  • This does not address the wider point that FileSystemChatMemoryRepository accepts any
    conversation 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.

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>
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.

1 participant