Skip to content

fix(tasks): parse task dependency lists in linear time - #752

Open
aliasunder wants to merge 6 commits into
mainfrom
worktree-task-dependency-hang
Open

aliasunder wants to merge 6 commits into
mainfrom
worktree-task-dependency-hang

Conversation

@aliasunder

@aliasunder aliasunder commented Nov 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

A task line whose dependency list puts a space before each comma (⛔ a , b , c …, or the Dataview [dependsOn:: …] form) made the dependency pattern try every way of splitting the spaces, so each added ID doubled the work: about 2 seconds at 26 IDs, and far longer beyond. Task lines are parsed for every note when the index is built and whenever a note changes, so one such line could keep the server from starting or answering.

Changes

src/vault-mcp/obsidian-markdown/tasks.ts:

  • TASK_ID_SEQUENCE changes from ID( *, *ID *)* to ID(?:(?: *, *ID)+ *(?! ))?, built from the same ID piece. Each run of spaces now has one place to match, so a failing line is rejected in linear time. It accepts exactly the same text in both the emoji and Dataview forms; the old pattern's last-repetition capture group, which no caller reads, is gone.
  • The metadata-stripping loop runs the Tasks plugin's 21 passes, not 20, so a line with 21 trailing fields parses as it does in the plugin.
  • Text a task edit adds after a bare - [ ] gets the one space the Tasks plugin writes after a checkbox (- [ ] 🆔 abc123, not - [ ]🆔 abc123).
  • Field edits leave a line unchanged when its task body contains a line or paragraph separator, since the task parser does not read such a line as a task.

src/vault-mcp/obsidian-markdown/__tests__/tasks.test.ts: a 60-ID line in each form parses within a fixed time (run in a child process with a hard deadline), a differential test compares the old and new patterns' matches and dependency group over a fixed-seed random corpus plus every dependency string in the existing tests, and boundary tests cover the pass limit, the checkbox space and separator-containing bodies.

Verification

  • The parser tests (src/vault-mcp/obsidian-markdown) and the task-mutation tests: 2,438 passed; the differential test finds no differences.
  • Restoring the old pattern makes both 60-ID tests miss their deadline; restoring the 20-pass limit or removing the checkbox space fails their tests.
  • npm run build, npm run lint (no errors) and npx knip pass; CI passes at a014a325.

Live Test Results

Head a014a32544df51447e14836c92297a6853308810.

Built image, isolated. The Dockerfile's local target built from that head (sha256:7e592ab166c2…), run with its own fresh vault and index volumes, a 2 GiB limit and no Sync. The trigger is a 40-ID list with a space on each side of every comma and a failing ! ending.

Check /healthz Search finds the trigger note Bound
Trigger present before boot 1.1 s 2.7 s 30 s, pass
Trigger written after the server was ready 0.08 s after the write 3.5 s after the write 30 s, pass

The same 40 IDs without the failing ending list as a1…a40 in order, and ⛔ a1, a2, a3 lists exactly those three. The old pattern, run alone on the trigger line, was still working when its 10-second timeout killed it.

Shared test instance. Test deploy run 42437302738; the running container's image is ghcr.io/aliasunder/vault-cortex:test at sha256:d3152dca6cfd…, labelled with this head. No trigger line was written there. A throwaway note held a 24-ID spaced list in each form, a 3-ID list and a list with trailing spaces:

  • vault_list_tasks returned each depends_on list exactly, in order (24, 24, 3 and 2 IDs).
  • A search for the note's unique word returned it first.
  • /healthz answered 200 in under 2 ms before, during and after the writes; the container did not restart.
  • Setting a task ID on a bare - [ ] line wrote - [ ] 🆔 pr752id.

The note and its folder were deleted afterwards, and the instance was returned to the released image (Force Deploy run 42440084835, revision 405d9ab2, healthy).

🤖 Generated with Claude Code

Move trailing spaces outside the repeated comma-separated IDs to avoid exponential backtracking while preserving dependency matches in emoji and Dataview fields.

Implemented-By: codex · gpt-6.1-sol
@umm-actually

umm-actually Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

umm-actually re-reviewed at a014a32

1 new finding(s) posted (5 tracked finding(s) across all runs).


umm-actually · z-ai/glm-5.3-flash

Ship-Check: code-quality · gpt-6.1-sol
Ship-Check: test-audit · gpt-6.1-sol
Keep the dependency list's final space run maximal so a malformed Dataview
field cannot repartition it with the surrounding field matcher. Cover the
long suffix through the bounded parser regression and correct factual task
parser comments about termination, ancestry and recurring copies. Keep
mutations from editing bodies the task-line recognizer already rejects.

Ship-Check: bug-check · gpt-6.1-sol
@umm-actually

umm-actually Bot commented Nov 8, 2026

Copy link
Copy Markdown

Apply the multi-line no-op guard to updateTaskLineStatus too
Low severity · conventions · high confidence

src/vault-mcp/obsidian-markdown/tasks.ts:1671 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing behavior made visible by this PR's guard: updateTaskLineStatus rewrites the checkbox via replaceCheckboxChar before any line-boundary check runs, and assignBlockId/removeBlockId never pass through splitTaskLine at all, so the multi-line no-op contract the new tests pin for updateTaskLineTaskId does not hold uniformly across the exported mutators.

Failure scenario: tasks.updateTaskLineStatus({ taskLine: '- [ ] First' + String.fromCharCode(10) + 'second', newStatus: 'done', today: '2026-07-12', config }) returns '- [x] First' + String.fromCharCode(10) + 'second' — a body the PR's own tests assert isTaskLine rejects gets its checkbox flipped, while updateTaskLineTaskId on the same input returns it unchanged; no test pins updateTaskLineStatus's multi-line behavior.

Suggested fix
Add the same early return the sibling mutators get from splitTaskLine: at the top of updateTaskLineStatus, `if (!isTaskLine(params.taskLine)) return params.taskLine`, and either route assignBlockId/removeBlockId through the same check or add the pinning no-op tests for all three.

umm-actually · z-ai/glm-5.3-flash

…e timing tests' startup

Allow 21 metadata stripping passes to match the Tasks plugin's loop boundary and cover the 21- and 22-signifier cases. Explain why dependency whitespace must have one matching position, and allow ten seconds for child startup while retaining the 50 ms parsing assertion.

Implemented-By: codex · gpt-6.1-sol
@aliasunder

Copy link
Copy Markdown
Owner Author

Reply to the umm-actually finding "Apply the multi-line no-op guard to updateTaskLineStatus too" (comment 6715754095): Not changing this: every production caller passes these mutators a single line taken from the split note (task-mutations.ts:2009, 2137 and 2149, after the line is checked as a task), so a multi-line string cannot reach them; the guard in updateTaskLineTaskId sits on the parse path the dependency field shares.


🔍 ship-check · pr-monitor · claude-opus-5-5

@umm-actually

umm-actually Bot commented Nov 8, 2026

Copy link
Copy Markdown

Separate the checkbox from inserted fields on an empty task body
Low severity · correctness · medium confidence

src/vault-mcp/obsidian-markdown/tasks.ts:1150 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing: joinTaskLine joins the checkbox prefix straight onto a non-empty body, so mutating an empty task line like - [ ] glues the field or description text to the bracket (- [ ]🆔 abc123). A GFM/Obsidian task-list item requires whitespace after the marker, so the written line renders in Obsidian as a plain bullet rather than a checkbox while the server still indexes it as a task. The new empty-body test in this PR pins the glued output as the expected value, and replaceTaskLineDescription produces the same glue (- [ ]New name).

Failure scenario: A note holds an empty checkbox line - [ ]; vault_update_task sets task_id on it and reports success, writing - [ ]🆔 abc123. Obsidian requires whitespace after the task-list marker, so the line renders as a plain bullet with literal brackets instead of a checkbox, while vault_list_tasks still lists it as a task — the write silently broke the task's rendering.

Suggested fix
In joinTaskLine, add a separator space when the body is non-empty and the prefix does not already end with whitespace, e.g. `const separator = body !== "" && !/\s$/.test(prefix) ? " " : ""` before the return, and update the two new empty-body expectations to `- [ ] 🆔 abc123`.

umm-actually · z-ai/glm-5.3-flash

…t adds

The Tasks plugin writes one space after the checkbox, and a checkbox
glued to its text does not render as one, so setting a field or a
description on a bare "- [ ]" now writes "- [ ] ...".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aliasunder

Copy link
Copy Markdown
Owner Author

Reply to the umm-actually finding "Separate the checkbox from inserted fields on an empty task body" (comment 6715865069): Fixed in this push. Text a task edit adds after a bare checkbox now gets the one space the Tasks plugin itself writes (toFileLineString writes [status] then the text), so - [ ] becomes - [ ] 🆔 abc123 and - [ ] New for a description; the empty-body expectations are corrected and two description cases added.


🔍 ship-check · pr-monitor · claude-opus-5-5

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