Skip to content

Rebased /todo command + auto-todos onto 0.5.0 (review feedback from #3 addressed) - #13

Merged
aiev merged 5 commits into
aiev:masterfrom
famewolf:todo-message-log-read-rebased
Oct 12, 2026
Merged

aiev merged 5 commits into
aiev:masterfrom
famewolf:todo-message-log-read-rebased

Conversation

@famewolf

@famewolf famewolf commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Rebased /todo command + auto-todos onto 0.5.0, addressing the review on #3.

Required changes:

  1. Dropped the src/tui-data.ts / tests/tui-data.test.ts hunks — master's todowritesFromMessages + writeTimestamp already cover the timestamp-ranked read. The branch keeps the /todo command + message-log reader.
  2. Message sources now use typed ctx.session.context({ sessionID }) with same-plugin storage fallback; deleted the ctx.client cast, the serverBaseUrls/serverHeaders helpers, and the node:fs import.

Cleanups folded in: joined split async lines; explicit undefined (never []) when no source has a list, keeping empty vs failed distinguishable; rebased on current master, keeping master's rewritten context hook and only adding the command registration/disposal. Also adds MAX_TODOS=100 / MAX_TODO_CONTENT_LENGTH=500 validation to normalizeTodos.

Tests: 69/69 green (master suite untouched + rewritten todo-command/auto-todos tests against the typed API).

… addressed

- /todo command reads via typed ctx.session.context + latestTodosFromMessages,
  falls back to same-plugin storage; returns undefined (never []) when empty
- auto-todos resolves the session directory via typed ctx.session.get;
  drops raw client cast, serverBaseUrls/serverHeaders helpers, node:fs /proc reads
- keeps master's rewritten context hook; adds MAX_TODOS=100 and
  MAX_TODO_CONTENT_LENGTH=500 validation to normalizeTodos
- joins split async lines; explicit 'could not read' vs [] distinction kept
- tests: 69/69 green (master suite + rewritten todo-command/auto-todos tests)
aiev added 4 commits October 12, 2026 15:08
An entry without `always` is meant to inject when the incoming list is empty, but the check ran against the list as it was being built: the first injected entry made the list non-empty and every later default entry was skipped. Decide against the incoming list once.

Also hold the config to the `todowrite` limits: entries over MAX_TODO_CONTENT_LENGTH are dropped and injection stops at MAX_TODOS so the merged list can never exceed the caps.
The command collapsed `undefined` (no source has a list) into `[]` and rendered "the todo list is empty" for both. Post "No todo list found for this session." when no list exists, and keep the empty-list report for an explicitly empty list. Also restore the formatting of the `ctx.command.transform` call and use a valid delivery value in the test.
The MAX_TODOS / MAX_TODO_CONTENT_LENGTH rejections added with the caps had no tests.
/todo posts the current list into the conversation; auto-todos.json seeds it per project. Both shipped without README coverage.
@aiev

aiev commented Oct 12, 2026

Copy link
Copy Markdown
Owner

Thanks @famewolf — I went through the rebase and pushed four fixup commits on top of 032099e (using the "edits by maintainers" flag) for the remaining items:

  • d283e4c fix(auto-todos): seed every default entry and respect the list caps — the default (always: false) check ran against the list as it was being built, so only the first default entry injected; it now decides against the incoming list once. Entries over MAX_TODO_CONTENT_LENGTH are dropped and injection stops at MAX_TODOS, so the merged list can no longer bypass the caps added in 032099e (an auto-todos.json could previously push unbounded content into plugin storage and the injected context).
  • 520223e fix(todo-command): distinguish a missing list from an empty one — undefined (no source has a list) is no longer collapsed into []; the command posts "No todo list found for this session." instead of "the todo list is empty". Also fixed the joined ctx.command.transform line from the rebase and a test's delivery value.
  • 1cef0a4 test(todos): cover the normalizeTodos limits — the MAX_TODOS / MAX_TODO_CONTENT_LENGTH rejections had no tests.
  • 01d1b8b docs: document the /todo command and auto-todos.json.

On prompt caching: the fixups touch neither the context hook, the reminder text, nor the unchanged-list skip — src/tui-data.ts, src/timing.ts and tests/plugin.test.ts are byte-identical to master, and the only src/index.ts diff remains the tool-call wrapping plus the command registration/dispose from your commit. I added an idempotence test (two identical writes with an always entry store the same list) so the skip added in #8 keeps working.

CI is green on 01d1b8b (Node 20/22); locally: 76/76 tests, typecheck and build clean.

@aiev
aiev merged commit 31dd716 into aiev:master Oct 12, 2026
2 checks passed
@famewolf
famewolf deleted the todo-message-log-read-rebased branch October 12, 2026 20:08
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.

2 participants