feat: TUI state persistence, new commands, and non-interactive improvements

[paxcalpt]

Summary

This PR adds several major improvements to TaskRepo:

🎯 TUI State Persistence

The TUI now remembers your preferences across sessions:

• Tree vs Flat view: Automatically restores your preferred display mode
• View selection: Returns to the last repo/project/assignee you were viewing
• View mode: Remembers whether you were in repo/project/assignee mode

Configuration:

• remember_tui_state: Master toggle (default: true)
• tui_tree_view: Tree view preference
• tui_last_view_item: Last selected view item

Bug fixes:

• Fixed --repo flag always overwriting restored state
• Changed tree toggle key from 'r' to 't' for better ergonomics

📝 New Commands

tsk add-link - Add URLs to tasks

tsk add-link 5 "https://github.com/org/repo/issues/123"

tsk append - Append text to task descriptions

tsk append 5 --text "Additional note from meeting"

tsk update - Batch update task fields

tsk update 5,6,7 --status in-progress --priority H

🤖 Non-Interactive Mode

Better support for automation and CI environments:

• delete: Requires --force flag in non-interactive terminals
• done/unarchive: Auto-confirms subtask operations
• sync: New --non-interactive flag to skip prompts

✨ Other Improvements

• Archive: New --all-completed flag to bulk-archive completed tasks
• Sync: Improved git operations with better interactive prompt handling
• Config display: Shows TUI state persistence settings

Testing

• ✅ TUI state persistence tested across sessions
• ✅ View restoration works correctly
• ✅ Tree view toggle persistence verified
• ✅ View mode switching resets saved item as expected

🤖 Generated with Claude Code

[claude]

Code Review - PR #11

Thank you for this comprehensive PR! This adds several valuable features to TaskRepo. Below is my detailed review covering code quality, potential issues, performance, security, and testing.

---

✅ Strengths

1. Well-Structured New Commands

The three new commands (add-link, append, update) follow existing patterns consistently and integrate well with the codebase.

2. TUI State Persistence

The implementation is clean with proper configuration management through properties and sensible defaults (remember_tui_state: true).

3. Non-Interactive Mode Improvements

Good safety-first approach requiring --force for destructive operations in non-interactive terminals.

4. Sync Command Refactoring

The switch from Rich's Progress to SimpleSyncProgress is a pragmatic solution to terminal freezing issues during interactive prompts.

---

🐛 Potential Bugs & Issues

HIGH PRIORITY

1. Type Error in update.py:128 ⚠️

task.due = parsed_date.isoformat()  # Line 128

Issue: task.due expects Optional[datetime], but you're assigning a string (isoformat() returns str).

Fix:

task.due = parsed_date  # Store datetime directly

The Task model already handles serialization to ISO format during save.

---

2. detect_conflicts Missing Parameter in sync.py:603 ⚠️

conflicts, _ = run_with_spinner(
    progress, spinner_task, "Checking for conflicts", 
    check_conflicts, verbose, operations_task
)

The lambda passes skip_fetch=True, but detect_conflicts may not accept this parameter. Verify the function signature in the conflict detection module.

---

3. Race Condition in Config Saves

Multiple TUI keybindings save config state independently:

• src/taskrepo/tui/task_tui.py:663-668 (right arrow)
• src/taskrepo/tui/task_tui.py:678-683 (left arrow)
• src/taskrepo/tui/task_tui.py:703-704 (tab)
• src/taskrepo/tui/task_tui.py:794-795 ('t' key)

Issue: Each keypress triggers a disk write (config.save()). In rapid navigation, this could cause:

• Performance degradation
• Potential data loss if writes overlap

Recommendation: Debounce config saves or batch them on TUI exit:

def cleanup(self):
    """Called on TUI exit."""
    if self.config.remember_tui_state:
        self.config.tui_tree_view = self.tree_view
        if self.current_view_idx == -1:
            self.config.tui_last_view_item = None
        else:
            self.config.tui_last_view_item = self.view_items[self.current_view_idx]
        self.config.save()  # Single save on exit

---

MEDIUM PRIORITY

4. Missing URL Validation in add_link.py:30-32

if not url.startswith(("http://", "https://")):
    click.secho("Error: URL must start with http:// or https://", fg="red", err=True)
    ctx.exit(1)

Issue: This is less strict than Task.validate_url() which checks for a valid netloc. Use the existing validation:

from taskrepo.core.task import Task

if not Task.validate_url(url):
    click.secho("Error: Invalid URL format", fg="red", err=True)
    ctx.exit(1)

---

5. Inconsistent Error Handling in append.py

if task.description:
    task.description = task.description.rstrip() + "\n\n" + text
else:
    task.description = text

Potential Issue: If task.description is None (not just empty string), this will fail. The Task model uses description: str = "" so it should be safe, but defensive coding is better:

current = (task.description or "").rstrip()
task.description = f"{current}\n\n{text}" if current else text

---

6. Detached HEAD Recovery Logic Could Be More Robust (sync.py:437-467)

The detached HEAD recovery tries to reattach automatically, which is good, but:

if current_sha == branch_sha:
    git_repo.heads[target_branch].checkout()

Concern: If checkout fails (e.g., due to local changes), the exception is caught but should_push is set to False silently. The user should be informed more clearly.

Suggestion:

try:
    git_repo.heads[target_branch].checkout()
    progress.console.print(f"  [green]✓[/green] Re-attached to '{target_branch}'")
except GitCommandError as checkout_error:
    progress.console.print(f"  [red]✗[/red] Failed to checkout {target_branch}: {checkout_error}")
    should_push = False

---

7. --all-completed Without Confirmation (archive.py:21-66)

The new --all-completed flag archives all completed tasks without confirmation. This could be surprising for users with many completed tasks.

Recommendation: Add a confirmation prompt unless --yes is also provided:

if all_completed and not task_ids:
    # ... find completed tasks ...
    
    if not yes:
        click.echo(f"About to archive {len(completed_ids)} completed task(s).")
        if not click.confirm("Continue?"):
            click.echo("Cancelled.")
            return
    
    task_ids = tuple(completed_ids)

---

🔒 Security Considerations

1. Command Injection Risk is Low ✅

The run_git_verbose function uses subprocess.run with a list (not shell=True), which prevents injection:

result = subprocess.run(["git"] + args, cwd=repo_path, check=False)

This is secure - well done!

2. Path Traversal in delete_unexpected_files (file_validation.py:177-206)

full_path = repo_path / file_path
if full_path.is_file():
    full_path.unlink()

Low Risk: The file_path comes from git_repo.untracked_files, which should be safe. However, explicitly checking that the resolved path is within repo_path would be more defensive:

full_path = (repo_path / file_path).resolve()
if not full_path.is_relative_to(repo_path):
    click.secho(f"  ⚠️  Skipping file outside repo: {file_path}", fg="yellow")
    continue

---

⚡ Performance Considerations

1. Config Save Frequency (mentioned above)

Saving config on every TUI keypress is inefficient. Batch saves on exit.

2. --all-completed Loads All Tasks

all_tasks = manager.list_all_tasks(include_archived=False)

For users with hundreds of tasks across many repos, this could be slow. Consider adding progress feedback:

with click.progressbar(all_tasks, label="Scanning for completed tasks") as bar:
    completed_tasks = [task for task in bar if task.status == "completed"]

3. SimpleSyncProgress Doesn't Update Progress

The new SimpleSyncProgress.update() method doesn't print updates (line 260-261: commented out). This means users lose visual feedback during long operations. Consider printing significant updates:

def update(self, task_id, advance=None, description=None, **kwargs):
    # ... existing code ...
    if description and "completed" in description.lower():
        self.console.print(f"  {description}")  # Print completion messages

---

🧪 Test Coverage

Missing Tests ⚠️

Based on the file listing, there are no tests for the new commands:

• tests/unit/test_add_link.py - missing
• tests/unit/test_append.py - missing
• tests/unit/test_update.py - missing

Recommended Test Cases

For add-link:

def test_add_link_valid_url():
    # Test adding a valid URL to a task

def test_add_link_duplicate():
    # Test adding the same URL twice (should skip)

def test_add_link_invalid_url():
    # Test with invalid URL (should error)

def test_add_link_task_not_found():
    # Test with non-existent task ID

For append:

def test_append_to_existing_description():
    # Test appending to task with existing description

def test_append_to_empty_description():
    # Test appending to task with no description

def test_append_preserves_existing_content():
    # Verify original content is preserved

For update:

def test_update_single_task():
    # Test updating one field on one task

def test_update_multiple_tasks():
    # Test updating multiple tasks at once

def test_update_multiple_fields():
    # Test updating priority, status, tags together

def test_update_no_options():
    # Test error when no update options provided

def test_update_due_date_parsing():
    # Test natural language date parsing

def test_update_due_date_invalid():
    # Test handling of invalid dates

For TUI state persistence:

def test_tui_state_persistence():
    # Test that view state is restored across sessions

def test_tui_state_disabled():
    # Test behavior when remember_tui_state is False

def test_tui_tree_view_toggle():
    # Test tree view preference is saved

For non-interactive mode:

def test_delete_non_interactive_without_force():
    # Should fail without --force

def test_done_non_interactive_subtasks():
    # Should auto-confirm subtasks

def test_sync_non_interactive_skip_unexpected():
    # Should skip repos with unexpected files

---

📝 Code Quality & Best Practices

Good Practices ✅

1. Docstrings: All new functions have Google-style docstrings
2. Type hints: Used consistently (Optional[str], Tuple[str, ...])
3. Error messages: Clear and actionable
4. Code reuse: Leverages existing helpers (find_task_by_title_or_id, select_task_from_result)

Style Suggestions

1. Unused Import in add_link.py

import sys  # Line 3 - never used

Remove this or suppress with # noqa: F401 if planned for future use.

2. Magic Numbers in file_validation.py:106

for file_path in sorted(files)[:5]:  # Show max 5 files per pattern
    console.print(f"    - {file_path}")
if file_count > 5:
    console.print(f"    ... and {file_count - 5} more")

Extract to a named constant:

MAX_FILES_TO_DISPLAY = 5

3. Duplicate Code in TUI Key Bindings

Lines 663-668 and 678-683 in task_tui.py have identical logic for saving view state. Extract to a helper:

def _save_current_view(self):
    if self.config.remember_tui_state:
        if self.current_view_idx == -1:
            self.config.tui_last_view_item = None
        else:
            self.config.tui_last_view_item = self.view_items[self.current_view_idx]

---

📋 Documentation

PR Description ✅

Excellent! Clear summary with examples and testing notes.

Missing Documentation

• Update main README with new commands (add-link, append, update)
• Document --all-completed flag in archive command docs
• Document new config options (remember_tui_state, tui_tree_view, tui_last_view_item)
• Add --non-interactive flag to sync command docs

---

🎯 Summary & Recommendations

Must Fix Before Merge:

1. ✅ Fix type error in update.py:128 (datetime vs string)
2. ✅ Verify detect_conflicts signature for skip_fetch parameter
3. ✅ Add tests for new commands (minimum: happy path tests)

Should Fix:

4. Use Task.validate_url() in add_link.py
5. Add confirmation for --all-completed without --yes
6. Batch TUI config saves on exit instead of per-keypress
7. Remove unused import sys from add_link.py

Nice to Have:

8. Add defensive path checking in delete_unexpected_files
9. Extract duplicate TUI save logic to helper method
10. Add progress feedback for --all-completed scanning

---

Overall Assessment

This is a solid PR with valuable features. The code follows existing patterns well and the non-interactive improvements are thoughtful. The main concerns are:

1. The type error in update.py
2. Missing test coverage for new functionality
3. Frequent config saves in TUI

With the critical fixes applied and basic tests added, this will be ready to merge.

Great work on the comprehensive feature additions! 🚀

[Copilot]

Pull request overview

This PR introduces comprehensive TUI state persistence, three new CLI commands, and improved non-interactive terminal support for automation scenarios.

Changes:

• TUI state persistence system that remembers tree view preference, view mode, and last selected item across sessions with configurable toggle
• New commands: add-link for adding URLs to tasks, append for appending text to descriptions, and update for batch field updates
• Non-interactive mode improvements with --force requirements, auto-confirmation of subtask operations, and a new --non-interactive sync flag

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/taskrepo/core/config.py	Adds three new config properties for TUI state persistence (remember_tui_state, tui_tree_view, tui_last_view_item)
src/taskrepo/tui/task_tui.py	Implements state restoration on TUI initialization and state saving on navigation events; changes tree toggle key from 'r' to 't'
src/taskrepo/cli/commands/tui.py	Fixes --repo flag to only override state when explicitly provided, preventing unintended state overwrites
src/taskrepo/cli/commands/main.py	Registers three new commands (add-link, append, update) in the CLI
src/taskrepo/cli/commands/add_link.py	New command for adding URLs to task links with basic URL validation
src/taskrepo/cli/commands/append.py	New command for appending text to task descriptions
src/taskrepo/cli/commands/update.py	New command for batch updating task fields (status, priority, tags, assignees, etc.)
src/taskrepo/cli/commands/delete.py	Adds non-interactive terminal detection requiring --force flag for safety
src/taskrepo/cli/commands/done.py	Auto-confirms subtask operations in non-interactive terminals
src/taskrepo/cli/commands/unarchive.py	Auto-confirms subtask operations in non-interactive terminals
src/taskrepo/cli/commands/sync.py	Adds --non-interactive flag, SimpleSyncProgress class for better prompt handling, improved git operations with verbose output
src/taskrepo/cli/commands/archive.py	Adds --all-completed flag for bulk archiving completed tasks
src/taskrepo/cli/commands/config.py	Displays TUI state persistence settings in config output
src/taskrepo/core/repository.py	Adds silent_errors parameter to list_tasks and list_archived_tasks with improved conflict detection messages
src/taskrepo/utils/file_validation.py	Switches from click to Rich Console and prompt_toolkit for better interactive prompt handling

Comments suppressed due to low confidence (1)

src/taskrepo/utils/file_validation.py:85

• The import statement for Console is placed in the middle of the file, after the detect_unexpected_files function. This violates Python best practices where all imports should be at the top of the file. Move this import to the top with the other imports.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Logic issue with --all-completed flag. If both --all-completed and task_ids are provided, the flag is ignored (line 34 condition: "if all_completed and not task_ids"). This could be confusing for users. Consider either: 1) rejecting when both are provided with an error message, or 2) documenting that --all-completed is ignored when explicit task IDs are provided.

Suggested change

	Use --all-completed to archive all tasks with status 'completed' in one command.
	"""
	config = ctx.obj["config"]
	manager = RepositoryManager(config.parent_dir)
	Use --all-completed to archive all tasks with status 'completed' in one command.
	Note: --all-completed cannot be used together with explicit TASK_IDS.
	"""
	config = ctx.obj["config"]
	manager = RepositoryManager(config.parent_dir)
	# Disallow using --all-completed together with explicit task IDs to avoid confusion
	if all_completed and task_ids:
	click.secho(
	"Error: --all-completed cannot be used together with TASK_IDS. "
	"Either specify task IDs, or use --all-completed without IDs.",
	fg="red",
	err=True,
	)
	ctx.exit(1)

[Copilot]

Unused import. The sys module is imported but never used in this file. Remove this unused import.

[Copilot]

Unused import. The sys module is imported but never used in this file. Remove this unused import.

[Copilot]

Key binding conflict. Both the "extend task due date" functionality (line 768-771) and the "toggle tree view" functionality (line 790-796) are bound to the 't' key. This will cause the first binding to be overridden by the second, making the extend functionality unreachable. One of these keybindings needs to be changed to a different key.

[Copilot]

Duplicate variable initialization. The variables repo_timings and total_start_time are initialized twice in a row at lines 389-390 and 393-394. Remove the duplicate initialization.

[Copilot]

Inconsistent indentation - line 596 uses a space for indentation instead of the standard 4 spaces or tabs. This should be corrected to use proper indentation to match the surrounding code.

[Copilot]

Inconsistent indentation - line 859 uses a space for indentation instead of the standard 4 spaces or tabs. This should be corrected to use proper indentation to match the surrounding code.

[Copilot]

Import of 'sys' is not used.

[claude]

Pull Request Review - PR 11

This PR adds TUI state persistence, three new commands, and non-interactive mode improvements. Overall well-structured code.

HIGH PRIORITY ISSUES

1. MISSING TEST COVERAGE - The three new commands (add-link, append, update) have ZERO tests
2. TYPE SAFETY BUG in update.py:138 - Task.due expects datetime but dateparser returns datetime or date
3. SimpleSyncProgress replaces all Rich Progress - loses visual feedback

MEDIUM PRIORITY

4. Duplicate declarations in sync.py:387-388
5. URL validation too permissive in add_link.py - accepts malformed URLs
6. silent_errors parameter needs documentation
7. Inconsistent exit codes in add_link.py:49

SECURITY

Good: Proper subprocess usage, safe git operations, input validation
Minor: URL injection - ensure escaping if displayed in web UI

SUMMARY

High-quality PR but MUST FIX:

• Add unit tests for new commands (CRITICAL)
• Fix type safety in update.py:138
• Remove duplicate declarations

Target 80%+ test coverage per CONTRIBUTING.md

Review by Claude Code

[claude]

Detailed Code Review

Code Quality Strengths

• Excellent documentation with clear docstrings and usage examples
• Consistent style following PEP 8 and project patterns
• Proper type annotations throughout
• Comprehensive error messages and validation

New Features Analysis

TUI State Persistence

• Clean config properties with getters/setters in config.py
• Proper default values and separation of concerns
• Well-integrated with existing config system

New Commands (add-link, append, update)

• Consistent patterns across all three
• Good error handling and user feedback
• Support for both single and batch operations

Non-Interactive Mode

• Smart --non-interactive flag for CI/CD
• sys.stdin.isatty() checks for auto-confirmation
• Safe defaults (skip on uncertainty)

Issue Details

Type Safety Bug (update.py:138)

parsed_date = parse_date(due)  # Returns Optional[datetime | date]
if parsed_date:
    task.due = parsed_date  # Expects Optional[datetime]

Fix: Add isinstance check and convert date to datetime if needed

SimpleSyncProgress Concern
Replaces Rich Progress completely. While solving prompt freezing, this removes all progress indication. Consider:

• Hybrid approach using Rich Progress but pausing during prompts
• Or document this is intentional tradeoff

URL Validation
Current: url.startswith(("http://", "https://"))
Better: Use urllib.parse.urlparse() to validate netloc exists

Performance Notes

• LRU caching preserved correctly
• Batch operations efficient
• silent_errors could accumulate memory with many broken files (minor)

Documentation Quality

• Excellent CHANGELOG following Keep a Changelog
• Clear examples in all command docstrings
• Thorough PR description

Recommend documenting --non-interactive in README

[claude]

Suggested Code Fixes

1. Fix Type Safety in update.py

Add this import and fix:

from datetime import date, datetime

# Around line 136-139
if due:
    parsed_date = parse_date(due)
    if parsed_date:
        # Convert date to datetime if needed
        if isinstance(parsed_date, date) and not isinstance(parsed_date, datetime):
            parsed_date = datetime.combine(parsed_date, datetime.min.time())
        task.due = parsed_date
        changes.append(f"due → {due}")
    else:
        click.secho(f"Warning: Could not parse due date '{due}' for task {task_id}", fg="yellow")

2. Improve URL Validation in add_link.py

from urllib.parse import urlparse

# Replace line 29-32 with:
def is_valid_url(url: str) -> bool:
    try:
        result = urlparse(url)
        return all([result.scheme in ['http', 'https'], result.netloc])
    except:
        return False

if not is_valid_url(url):
    click.secho("Error: Invalid URL format", fg="red", err=True)
    ctx.exit(1)

3. Remove Duplicates in sync.py

Delete lines 387-388:

# DELETE THESE LINES:
repo_timings = {}
total_start_time = time.perf_counter()

4. Example Test for add-link Command

Create tests/unit/test_add_link.py:

import pytest
from click.testing import CliRunner
from taskrepo.cli.commands.add_link import add_link

def test_add_link_valid_url(setup_repo):
    runner = CliRunner()
    result = runner.invoke(add_link, ['1', 'https://github.com/test'])
    assert result.exit_code == 0
    assert "Added link" in result.output

def test_add_link_invalid_url(setup_repo):
    runner = CliRunner()
    result = runner.invoke(add_link, ['1', 'not-a-url'])
    assert result.exit_code == 1
    assert "Error" in result.output

[claude]

Test Coverage Requirements

Per CONTRIBUTING.md: Aim for 80%+ coverage for new code

Missing Tests:

• add_link.py (58 lines) - 0% coverage
• append.py (48 lines) - 0% coverage
• update.py (155 lines) - 0% coverage
• Config properties - 0% coverage
• Non-interactive mode - 0% coverage

Estimated 25-30 test cases needed

Test areas:

• Valid inputs and success cases
• Invalid inputs and error handling
• Edge cases (empty fields, duplicates)
• Batch operations
• Non-TTY behavior
• Config persistence