Skip to content

fix: tasks.py crashes on malformed tasks.json in archive dirs#28

Open
benslockedin wants to merge 1 commit intomainfrom
fix/tasks-py-archive-crash
Open

fix: tasks.py crashes on malformed tasks.json in archive dirs#28
benslockedin wants to merge 1 commit intomainfrom
fix/tasks-py-archive-crash

Conversation

@benslockedin
Copy link
Copy Markdown
Contributor

Summary

  • tasks.py walks recursively to find tasks.json files but doesn't skip _archive/, _references/, 01_Archive/, or raw/ directories
  • Legacy tasks.json files in these paths (e.g. Airtable exports with different schema) cause _read_json() to sys.exit(1), killing the entire save operation
  • Reproduced on supernormal-systems walnut where _archive/reference/airtable-full-export/supernormal-systems/tasks.json exists with a v1 schema (no "tasks" key)

Changes

  • _all_task_files() — added _archive, _references, 01_Archive, raw to skip_dirs
  • _read_json() — added strict=False mode that returns None + prints a warning instead of exiting. Default remains strict=True for direct operations (add, done, edit) where you want hard failures
  • _collect_all_tasks() + _find_task() — use strict=False and skip None results gracefully

Test plan

  • Run tasks.py add on a walnut with legacy tasks.json in archive paths — should succeed
  • Run tasks.py list on same walnut — should list tasks, print warning to stderr for skipped files
  • Run tasks.py add on a walnut with no archive cruft — should work exactly as before
  • Run tasks.py done targeting a task in _kernel/tasks.json — strict mode still exits on malformed kernel file

🤖 Generated with Claude Code

Two bugs fixed, one feature added:

**Bug 1: Archive directory crash**
_all_task_files() walked recursively into archive and reference
directories containing legacy tasks.json files with non-v3 schemas.
_read_json() called sys.exit(1), killing the entire save operation.

Fix: skip any directory with "archive" in the name, plus _references
and raw. Collection functions (_collect_all_tasks, _find_task) now
use strict=False mode — warn and skip instead of crashing.

**Bug 2: v2 JSON format not recognized**
Existing tasks.json files using v2 schema ("text" instead of "title",
no "id" field, "normal" priority) passed validation but produced
incorrect output in list/summary commands.

**Feature: auto-migrate v2 tasks on first touch**
Three migration paths, all triggered transparently:

1. v2 JSON upgrade — tasks with "text" and no "id" are upgraded
   in-place. Priority "normal" maps to "todo". IDs assigned.

2. Markdown migration — tasks.md files (with or without YAML
   frontmatter) are parsed into tasks.json. Section headers
   (## Urgent, ## Active, ## To Do, ## Done) map to priority.
   Checkboxes, @session assignees, due dates, and completion
   dates extracted. Original backed up as tasks.md.v2-backup.

3. Archive skip — directories matching "archive" in name are
   excluded from recursive walks.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@patrickSupernormal
Copy link
Copy Markdown

Merge safety review (advisory) — Patrick Brosnan, non-collaborator on alivecontext/alive. Comment weight only, not an approving review.

Verdict: YELLOW — author response needed.

Headline: The PR body advertises a ~40-line archive-skip crash fix (skip_dirs expansion in _all_task_files + strict=False mode for _read_json). Both of those are correct in isolation and would be a GREEN PR on their own. But the actual diff is +304/-21, and the undisclosed ~260 lines add a full v2→v3 auto-migration subsystem (_migrate_tasks_md, _upgrade_v2_json, _ensure_tasks_json, _parse_task_line) with 4 concrete bugs:

  1. Read operations mutate the filesystem. _all_task_files now calls _ensure_tasks_json (line 341) AND _migrate_tasks_md (line 347) for every file found — meaning tasks.py list silently renames tasks.mdtasks.md.v2-backup, writes new files, and modifies disk. Violates least-astonishment.
  2. Destructive fallback in _migrate_tasks_md (lines 193-198): if tasks.md.v2-backup already exists, the fallback is os.remove(md_path) — silent data loss of the original tasks.md.
  3. Duplicate-ID bug in _upgrade_v2_json (lines 244-264): counter starts at 1 and ignores existing v3 task IDs. Mixed v2/v3 files can produce two tasks with the same id t001. Fix: seed counter from _next_id(tasks) or use a used-IDs set.
  4. Over-broad "archive" not in d.lower() wildcard (line 319): catches user-named walnuts/bundles like website-archive-rebuild, email-archive-export. Drop the wildcard, keep the explicit set.

Plus zero unit tests for the +260 lines of parsing/migration/writing code.

Recommendation: Either (a) split the PR — land the archive-skip + strict=False changes as the minimal ~40-line PR the body describes, put the migration subsystem in a separate PR with test coverage, OR (b) keep as one PR but fix + disclose + test — update body to describe the migration work, fix the 4 bugs, add unit tests, document the read-path mutation intentionality.

The underlying crash fix IS urgent (actively breaking save operations on supernormal-systems today). This is not a reason to block it; it's a reason to land the small fix separately while the migration subsystem gets proper review.

Conflict with #29: Textual only (both touch tasks.py in disjoint regions — #29's import-block + line-291 cmd_done edit vs #28's new helpers + _all_task_files rewrite). Whichever lands first, the other rebases in under a minute.

Full cross-PR synthesis (conflict matrix, recommended merge order for all 7 open PRs, per-PR verdicts, blocker list): see the cover comment on #29#29 (comment)

Generated via flow-next epic fn-8-vly (stackwalnuts walnut). 🐿️

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