diff --git a/DECISIONS.md b/DECISIONS.md index 9650149..e2ea35e 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -528,6 +528,12 @@ sees right now. The cost is bounded by the cache (60s TTL per worktree in process memory) and the per-probe timeout (3s). Failures fail closed — the worktree field becomes `null` rather than crashing the state endpoint. +V3 extends the same probe with `dirty_paths` because the file-level conflict +signal also lives in git, not the database. Persisting "which files are dirty" +would have the same stale-truth problem as persisting dirty counts; showing +the first three `status --porcelain` paths inline keeps the state brief honest +while staying bounded and cheap. + ### Why the worktree probe cache is 60 seconds, not longer or shorter The cache exists to keep state-brief reads cheap when called repeatedly diff --git a/REPO_STRUCTURE.md b/REPO_STRUCTURE.md index e4e81f9..91a489e 100644 --- a/REPO_STRUCTURE.md +++ b/REPO_STRUCTURE.md @@ -64,7 +64,7 @@ smriti/ │ │ └── providers.yaml Your keys (gitignored, not committed) │ ├── alembic/ Database migrations (14 versions) │ ├── tests/ -│ │ ├── integration/ API integration tests (122 tests) +│ │ ├── integration/ API integration tests (124 tests) │ │ │ ├── test_api_v4_chat.py │ │ │ ├── test_api_v5_lineage.py │ │ │ ├── test_multi_branch_state.py @@ -73,7 +73,7 @@ smriti/ │ │ │ ├── test_worktrees.py │ │ │ ├── test_checkpoint_extract.py │ │ │ └── test_delete_endpoints.py -│ │ └── unit/ Unit tests (125 tests) +│ │ └── unit/ Unit tests (129 tests) │ │ ├── test_config_loader.py │ │ ├── test_extractor.py │ │ ├── test_golden_outputs.py @@ -113,10 +113,10 @@ smriti/ │ │ ├── formatters.py Continuation-oriented markdown renderers │ │ │ (multi-branch, active claims, divergence) │ │ └── skill_pack/ Agent skill pack source and renderer -│ │ ├── template.md Single source of truth (v2.0, 15 sections) +│ │ ├── template.md Single source of truth (v2.1, 15 sections) │ │ ├── renderer.py Pure-function render + versioned install │ │ └── targets.py Target configs (claude-code, codex) -│ └── tests/ CLI + MCP tests (122 tests) +│ └── tests/ CLI + MCP tests (129 tests) │ ├── test_branch_close.py │ ├── test_init.py │ ├── test_mcp_server.py @@ -164,11 +164,11 @@ make migration Create a new migration (usage: make migration msg="...") --- -## Test counts (as of V2 worktree binding) +## Test counts (as of V3 dirty paths) | Suite | Count | Location | |---|---|---| -| Backend integration | 122 | `backend/tests/integration/` | -| Backend unit | 125 | `backend/tests/unit/` | -| CLI + MCP | 122 | `cli/tests/` | -| **Total** | **369** | | +| Backend integration | 124 | `backend/tests/integration/` | +| Backend unit | 129 | `backend/tests/unit/` | +| CLI + MCP | 129 | `cli/tests/` | +| **Total** | **382** | | diff --git a/backend/app/api/routes/chat.py b/backend/app/api/routes/chat.py index ad85c44..5dbd0ba 100644 --- a/backend/app/api/routes/chat.py +++ b/backend/app/api/routes/chat.py @@ -372,6 +372,7 @@ class ActiveWorktreeSummary(BaseModel): path: str branch: str dirty_files: int + dirty_paths: list[str] = Field(default_factory=list) ahead: int behind: int last_commit_sha: str diff --git a/backend/app/api/routes/worktrees.py b/backend/app/api/routes/worktrees.py index d1dda67..886844e 100644 --- a/backend/app/api/routes/worktrees.py +++ b/backend/app/api/routes/worktrees.py @@ -183,6 +183,7 @@ class WorkTreeResponse(BaseModel): class WorkTreeProbe(BaseModel): dirty_files: int + dirty_paths: list[str] = Field(default_factory=list) ahead: int behind: int last_commit_sha: str | None = None diff --git a/backend/app/services/worktree_probe.py b/backend/app/services/worktree_probe.py index 0c18d3b..5c70b01 100644 --- a/backend/app/services/worktree_probe.py +++ b/backend/app/services/worktree_probe.py @@ -33,6 +33,19 @@ def _run_git(path: str, args: list[str]) -> subprocess.CompletedProcess[str]: ) +def _parse_dirty_paths(porcelain_output: str, limit: int = 3) -> list[str]: + """Parse `git status --porcelain` output into the first dirty paths.""" + paths: list[str] = [] + for line in porcelain_output.splitlines(): + if not line.strip(): + continue + if len(line) >= 3: + paths.append(line[3:].strip()) + if len(paths) >= limit: + break + return paths + + def _probe_worktree(worktree_id: str, path: str, branch: str) -> dict[str, Any] | None: """Return git drift information for a worktree, or None on any error. @@ -61,6 +74,7 @@ def _probe_worktree_uncached( _log_probe_failure(worktree_id, "status", status) return None dirty_files = len([line for line in status.stdout.splitlines() if line.strip()]) + dirty_paths = _parse_dirty_paths(status.stdout) counts = _run_git(path, ["rev-list", "--left-right", "--count", "HEAD...origin/main"]) if counts.returncode != 0: @@ -94,6 +108,7 @@ def _probe_worktree_uncached( "path": path, "branch": branch, "dirty_files": dirty_files, + "dirty_paths": dirty_paths, "ahead": ahead, "behind": behind, "last_commit_sha": last_parts[0], diff --git a/backend/tests/integration/test_api_v4_chat.py b/backend/tests/integration/test_api_v4_chat.py index 4b2f7f9..5c4410e 100644 --- a/backend/tests/integration/test_api_v4_chat.py +++ b/backend/tests/integration/test_api_v4_chat.py @@ -57,6 +57,7 @@ def test_state_active_claims_include_bound_worktree_info(client, db_session, mon "path": path, "branch": branch, "dirty_files": 3, + "dirty_paths": ["cli/main.py", "backend/app/api/routes/chat.py"], "ahead": 1, "behind": 0, "last_commit_sha": "def5678", @@ -75,6 +76,7 @@ def test_state_active_claims_include_bound_worktree_info(client, db_session, mon "path": "/tmp/state-worktree", "branch": "smriti/codex-local/abc12345", "dirty_files": 3, + "dirty_paths": ["cli/main.py", "backend/app/api/routes/chat.py"], "ahead": 1, "behind": 0, "last_commit_sha": "def5678", diff --git a/backend/tests/integration/test_worktrees.py b/backend/tests/integration/test_worktrees.py index 3066b6a..13b7fc2 100644 --- a/backend/tests/integration/test_worktrees.py +++ b/backend/tests/integration/test_worktrees.py @@ -188,6 +188,7 @@ def test_list_includes_probe_data_for_active_worktrees(client, tmp_path, monkeyp "path": path, "branch": branch, "dirty_files": 3, + "dirty_paths": ["cli/main.py", "backend/app/api/routes/worktrees.py"], "ahead": 1, "behind": 0, "last_commit_sha": "abc1234", @@ -203,6 +204,7 @@ def test_list_includes_probe_data_for_active_worktrees(client, tmp_path, monkeyp assert data[0]["id"] == created["id"] assert data[0]["probe"] == { "dirty_files": 3, + "dirty_paths": ["cli/main.py", "backend/app/api/routes/worktrees.py"], "ahead": 1, "behind": 0, "last_commit_sha": "abc1234", diff --git a/backend/tests/unit/test_worktree_probe.py b/backend/tests/unit/test_worktree_probe.py index 587ad09..1d3309b 100644 --- a/backend/tests/unit/test_worktree_probe.py +++ b/backend/tests/unit/test_worktree_probe.py @@ -38,6 +38,7 @@ def test_probe_worktree_success(monkeypatch): "path": "/tmp/wt", "branch": "feature/wt", "dirty_files": 2, + "dirty_paths": ["file.py", "new.py"], "ahead": 2, "behind": 1, "last_commit_sha": "abc1234", @@ -108,3 +109,54 @@ def test_probe_worktree_malformed_output_returns_none(monkeypatch): monkeypatch.setattr(worktree_probe, "_run_git", fake_run_git) assert worktree_probe._probe_worktree("wt-bad", "/tmp/wt", "branch") is None + + +def test_parse_dirty_paths_basic(): + output = "\n".join( + [ + " M cli/main.py", + "?? backend/app/main.py", + "A docs/notes.md", + " D old.py", + "MM changed.py", + ] + ) + + assert worktree_probe._parse_dirty_paths(output) == [ + "cli/main.py", + "backend/app/main.py", + "docs/notes.md", + ] + + +def test_parse_dirty_paths_handles_renamed(): + output = "R old/path.py -> new/path.py\n" + + assert worktree_probe._parse_dirty_paths(output) == [ + "old/path.py -> new/path.py", + ] + + +def test_parse_dirty_paths_empty(): + assert worktree_probe._parse_dirty_paths("") == [] + + +def test_parse_dirty_paths_respects_limit(): + output = "\n".join( + [ + " M one.py", + " M two.py", + " M three.py", + " M four.py", + " M five.py", + " M six.py", + ] + ) + + assert worktree_probe._parse_dirty_paths(output, limit=5) == [ + "one.py", + "two.py", + "three.py", + "four.py", + "five.py", + ] diff --git a/cli/smriti_cli/formatters.py b/cli/smriti_cli/formatters.py index a5f0d62..e1d04e7 100644 --- a/cli/smriti_cli/formatters.py +++ b/cli/smriti_cli/formatters.py @@ -52,6 +52,27 @@ def format_worktree_ahead(worktree: dict) -> str: return "0" +def _truncate_dirty_path(path: str, limit: int = 40) -> str: + """Truncate long dirty paths so active-work lines stay readable.""" + if len(path) <= limit: + return path + return path[: limit - 1] + "…" + + +def _format_dirty_summary(dirty_files: int, dirty_paths: list[str] | None) -> str: + """Render dirty count plus the first dirty paths for active work claims.""" + if dirty_files <= 0: + return "0 dirty" + paths = dirty_paths or [] + if not paths: + return f"{dirty_files} dirty" + + shown = [_truncate_dirty_path(path) for path in paths[:3]] + if dirty_files > len(shown): + shown.append(f"+{dirty_files - len(shown)} more") + return f"{dirty_files} dirty ({', '.join(shown)})" + + def _relative_time(iso_ts: str) -> str: """Format an ISO-8601 UTC timestamp as a relative-time string.""" try: @@ -148,7 +169,7 @@ def _artifact_section( return "" if compact: # Labels only — no content. Explicit recovery instruction. - lines = [f"## Attached artifacts (compact — content omitted)"] + lines = ["## Attached artifacts (compact — content omitted)"] for art in artifacts: label = art.get("label") or "Untitled" lines.append(f"- {label}") @@ -291,6 +312,10 @@ def _format_active_claims_section(active_claims: list[dict]) -> str: if worktree: path = _pretty_path(worktree.get("path")) or worktree.get("path") or "?" dirty = worktree.get("dirty_files", 0) + dirty_summary = _format_dirty_summary( + int(dirty or 0), + worktree.get("dirty_paths") or [], + ) ahead = worktree.get("ahead", 0) behind = worktree.get("behind", 0) last_sha = worktree.get("last_commit_sha") or "?" @@ -298,7 +323,7 @@ def _format_active_claims_section(active_claims: list[dict]) -> str: lines.append(f" · worktree: {path}") lines.append( f" · branch: {worktree.get('branch') or '?'} · " - f"{dirty} dirty · ahead {ahead} · behind {behind} · " + f"{dirty_summary} · ahead {ahead} · behind {behind} · " f"last commit `{last_sha}` {last_rel}" ) elif worktree_id: diff --git a/cli/smriti_cli/skill_pack/template.md b/cli/smriti_cli/skill_pack/template.md index 4e3a042..330d21a 100644 --- a/cli/smriti_cli/skill_pack/template.md +++ b/cli/smriti_cli/skill_pack/template.md @@ -1,5 +1,5 @@ --- -smriti_skill_pack_version: 2.0 +smriti_skill_pack_version: 2.1 title: Smriti — how to use it well target: {{display_name}} --- @@ -368,6 +368,49 @@ Say out loud: **"Opening a worktree for this session."** before the first worktree open. **"Binding my claim to worktree ."** when claiming. **"Closing the worktree."** when done. +The state brief now shows you which files other agents are actively +editing, not just how many. Each active claim's worktree drift line +includes the first 3 dirty paths inline: + +``` + · branch: ... · 3 dirty (cli/main.py, backend/app/main.py, +1 more) · ... +``` + +This is your file-level coordination signal. Before editing a file in +your worktree, scan the active-claim drift lines for that path. If +another agent already has it dirty, hold off or pick a different file. +The signal is best-effort — it shows the first 3 dirty paths only and +the cache is 60 seconds — but it catches the common case where two +agents drift into the same file by accident. + +If your shell tool resets the working directory between commands (some +agent harnesses do this — every Bash call starts in the original cwd +even after `cd `), use absolute paths to the worktree +throughout. Example: + +``` +WT=/Users/.../.smriti/worktrees// +cat $WT/some/file.py +edit $WT/some/file.py +``` + +The skill pack used to say "use the worktree path as your cwd" — that +works for persistent shells, but absolute-path discipline works +everywhere. Capture the worktree path from `smriti worktree open` once +and reuse it. + +The default branch name when you `smriti worktree open` is +`smriti//`. That's fine for the system but ugly for +PR titles. Pass `--branch ` to use a custom branch name instead: + +``` +smriti worktree open --agent --branch v3-feature-name +``` + +Use this when the worktree maps cleanly to a single feature/PR. Stick +with the default when the worktree is short-lived or when several +related branches will live in it. + ### 3.7 Check freshness before checkpointing If you have been working for more than a few minutes, check whether @@ -926,7 +969,7 @@ tell you. Do not guess. --- -*Smriti skill pack version {{primary_mode}}-2.0 — this file is +*Smriti skill pack version {{primary_mode}}-2.1 — this file is authoritative for agent behaviour on this project. If you catch it contradicting itself or your observed behaviour of the tools, tell the human; the skill pack is versioned and meant to be updated.* diff --git a/cli/tests/test_skill_pack.py b/cli/tests/test_skill_pack.py index 38ba16b..1dddfa2 100644 --- a/cli/tests/test_skill_pack.py +++ b/cli/tests/test_skill_pack.py @@ -21,7 +21,6 @@ from pathlib import Path import pytest from smriti_cli.skill_pack import ( - InstallResult, get_target, get_version, install, @@ -44,7 +43,7 @@ def test_load_template_nonempty(): def test_get_version_parses_frontmatter(): version = get_version() - assert version == "2.0" + assert version == "2.1" def test_get_version_raises_when_frontmatter_missing(): @@ -164,6 +163,10 @@ _REQUIRED_PHRASES = [ "worktree_id", "cross-agent commit pollution", "Open a worktree per agent", + # Section 3.6.1 — V3 additions + "dirty paths", + "absolute paths", + "custom branch name", # Section 3.7 — freshness check "check freshness", "since", diff --git a/cli/tests/test_state_multi_branch.py b/cli/tests/test_state_multi_branch.py index 5717b61..d15540b 100644 --- a/cli/tests/test_state_multi_branch.py +++ b/cli/tests/test_state_multi_branch.py @@ -17,8 +17,6 @@ from __future__ import annotations from datetime import datetime, timezone -import pytest - from smriti_cli import mcp_server from smriti_cli.formatters import ( _format_active_branches_section, @@ -690,6 +688,51 @@ def test_claims_section_with_worktree_info(): assert "last commit `def5678` 5 minutes ago" in out +def test_render_dirty_paths_inline(): + from smriti_cli.formatters import _format_active_claims_section + + claims = [_worktree_claim(dirty_files=3, dirty_paths=[ + "cli/main.py", + "backend/app/api/routes/chat.py", + "backend/app/services/worktree_probe.py", + ])] + + out = _format_active_claims_section(claims) + + assert ( + "3 dirty (cli/main.py, backend/app/api/routes/chat.py, " + "backend/app/services/worktree_probe.py)" + ) in out + + +def test_render_dirty_paths_with_overflow(): + from smriti_cli.formatters import _format_active_claims_section + + claims = [_worktree_claim(dirty_files=5, dirty_paths=[ + "cli/main.py", + "backend/app/api/routes/chat.py", + "backend/app/services/worktree_probe.py", + ])] + + out = _format_active_claims_section(claims) + + assert ( + "5 dirty (cli/main.py, backend/app/api/routes/chat.py, " + "backend/app/services/worktree_probe.py, +2 more)" + ) in out + + +def test_render_dirty_paths_zero(): + from smriti_cli.formatters import _format_active_claims_section + + claims = [_worktree_claim(dirty_files=0, dirty_paths=[])] + + out = _format_active_claims_section(claims) + + assert "0 dirty · ahead" in out + assert "0 dirty (" not in out + + def test_claims_section_with_worktree_probe_failure(): """A bound claim with failed probing still renders a useful hint.""" from smriti_cli.formatters import _format_active_claims_section @@ -709,3 +752,26 @@ def test_claims_section_with_worktree_probe_failure(): out = _format_active_claims_section(claims) assert "probe failed or worktree closed" in out + + +def _worktree_claim(dirty_files: int, dirty_paths: list[str]) -> dict: + return { + "agent": "codex-local", + "branch_name": "v3-dirty-paths-and-skill-pack", + "scope": "Implement V3", + "worktree_id": "worktree-uuid", + "worktree": { + "id": "worktree-uuid", + "path": "/Users/example/.smriti/worktrees/smriti-dev/codex-local-abc12345", + "branch": "v3-dirty-paths-and-skill-pack", + "dirty_files": dirty_files, + "dirty_paths": dirty_paths, + "ahead": 1, + "behind": 0, + "last_commit_sha": "def5678", + "last_commit_relative": "5 minutes ago", + }, + "intent_type": "implement", + "base_commit_hash": "abc1234", + "claimed_at": datetime.now(timezone.utc).isoformat(), + }