fix: resolve the issue store path independently of the working directory

ISSUE_ROOT was the relative `tmp/issues`, so "the store" was whatever
directory the shell happened to be standing in. It is the --out default
in all eight scripts of both layers, which made one `cd` — and a `cd`
outlives the command that ran it — enough for readers to report an empty
store on a full one and for writers to quietly build a second store
beside the first. `issue_index.py` run from inside tmp/issues left
tmp/issues/tmp/issues/ behind and exited 0.

The anchor is issue.py's own __file__, not cwd. A script's location is a
fact about the installation; cwd is a fact about the last `cd`, and the
scripts are invoked by path from wherever the agent happens to be. From
there `store_root()` walks up to the nearest repo marker — `.git`
(exists(), not isdir(): a worktree's .git is a file) or AGENTS.md for a
copy taken out of git — and joins tmp/issues. Markers rather than a
fixed number of `..` hops, because the layout is not a promise. cwd is
tried only if the scripts are not inside a repository at all.

The function lives in the domain layer and skills/sync imports it, so
both layers agree by construction — the direction the layering rule
allows. skills/issue stays stdlib-only.

An explicit --out still wins and is used exactly as typed: a relative
--out stays relative to cwd, because that is what the operator asked
for. No new environment surface.

Two consequences the issue also asked for:

- Missing is no longer reported as empty. `store_error()` returns one
  message for a path that is not there and another for a store with no
  issues in it.
- Nothing conjures a store as a side effect of a write. save() and
  issue_index.build() require it instead of os.makedirs'ing it; only
  issue_new.py and pull.py create one, and both say so on stderr.

Establishes tests/ — plain stdlib unittest, no pytest, no dependencies.
The store tests build a throwaway repo in a TemporaryDirectory (a .git
marker, a copy of both script layers, fixture issues) and run the real
scripts inside it as subprocesses from five different working
directories; tmp/issues/ is never touched. Against the pre-fix scripts
15 of the 21 fail, reproducing the report exactly — five stray stores,
including tmp/issues/tmp/issues.

    python3 -m unittest discover -s tests -v

Closes claude-skills/tea#15

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
naudachu
2026-08-10 15:40:07 +05:00
parent d8bd927f1d
commit 6d01ead245
13 changed files with 651 additions and 19 deletions
+18
View File
@@ -47,6 +47,24 @@ tmp/issues/wire-sqlc.comments.md comment thread (written by /tea:sync only)
tmp/issues/tree-<id>.md saved graph (issue_tree.py --write)
```
## Where the store is
`<repo root>/tmp/issues`**not** `tmp/issues` relative to wherever you are
standing. The scripts resolve it by walking up from their own file to the
nearest `.git` or `AGENTS.md`, so they all see one store no matter which
directory you run them from, and a `cd` earlier in the session changes nothing.
`--out` overrides that and is taken **literally**: an absolute path is used as
given, a relative one stays relative to the current directory. Nothing rewrites
what you typed.
Two things follow, and both are deliberate:
- A store that is not there reports `does not exist`; a store with no issues in
it reports `is empty`. They are different problems.
- No script conjures a store as a side effect of writing. Only `issue_new.py`
creates one — the first issue in a fresh checkout — and it says so on stderr.
## Reading: grep, don't parse
Metadata is one field per line with inline lists precisely so plain `grep`
+111 -2
View File
@@ -48,7 +48,67 @@ without a parser:
import os
import re
ISSUE_ROOT = os.path.join("tmp", "issues")
# --------------------------------------------------------------------------
# where the store lives
# --------------------------------------------------------------------------
# `<repo root>/tmp/issues`, absolute, resolved once at import.
#
# It used to be the relative `tmp/issues`, which made "the store" whatever
# directory the shell happened to be standing in. One `cd` — and a `cd` outlives
# the command that ran it — was enough for readers to report an empty store on a
# full one and for writers to quietly build a second store beside the first.
#
# The anchor is THIS FILE, not the working directory. A script's own location is
# a fact about the installation; cwd is a fact about the last `cd`. Walking up
# from __file__ therefore hands every script in both layers the same answer no
# matter where it is invoked from — including from inside tmp/issues itself.
#
# An explicit --out still wins over all of this, and is used exactly as typed: a
# relative --out stays relative to cwd, because that is what the operator asked
# for. There is no environment override; the store is where the repo is.
STORE_PARTS = ("tmp", "issues")
# `.git` is a directory in a normal clone and a FILE in a worktree — hence
# exists(), not isdir(). AGENTS.md is the fallback for a plugin copied out of
# git; the agents-sync hook only ever puts one at a repository root.
REPO_MARKERS = (".git", "AGENTS.md")
_HERE = os.path.dirname(os.path.abspath(__file__))
def repo_root(start):
"""Nearest ancestor of `start` (inclusive) carrying a repo marker, or None.
Markers, not a fixed number of `..` hops: how deep this file sits below the
root is an implementation detail of the repo layout, and the layout is not
a promise."""
d = os.path.abspath(start)
while True:
if any(os.path.exists(os.path.join(d, m)) for m in REPO_MARKERS):
return d
parent = os.path.dirname(d)
if parent == d:
return None
d = parent
def store_root(start=None):
"""Absolute path of the issue store.
`start` overrides the anchor and exists so the resolution can be exercised
against a scratch tree. When these scripts are not inside a repository at
all, cwd gets a turn; failing that the historical cwd-relative location
stands, made absolute so an error message can name the directory it really
looked in."""
for anchor in ([start] if start is not None else [_HERE, os.getcwd()]):
root = repo_root(anchor)
if root:
return os.path.join(root, *STORE_PARTS)
return os.path.abspath(os.path.join(*STORE_PARTS))
ISSUE_ROOT = store_root()
# Domain-owned metadata, in render order. Foreign keys render after these,
# sorted, so the sync layer can add fields without touching this list.
@@ -356,6 +416,55 @@ def validate(issue, known_ids=None):
# store
# --------------------------------------------------------------------------
class StoreMissing(Exception):
"""The store directory is not there.
Deliberately a different answer from "the store is empty". One is a path
that does not exist, the other is a repository with no issues filed yet, and
conflating the two is exactly what made a missed directory look like an
empty backlog."""
def __init__(self, root):
self.root = root
Exception.__init__(self, "store %s does not exist" % root)
def store_exists(root):
return os.path.isdir(root)
def require_store(root):
"""Assert the store is there before reading or writing it."""
if not os.path.isdir(root):
raise StoreMissing(root)
return root
def create_store(root):
"""Create the store; True when it actually made the directory.
Only the commands that legitimately bootstrap a store call this — issue_new
and pull — and both announce it. Nothing creates a store as a side effect of
a write any more: a missing directory is something to report, not something
to conjure."""
if os.path.isdir(root):
return False
os.makedirs(root)
return True
def store_error(root):
"""Why `root` cannot be read as a store, or None when it holds issues.
The two messages are distinct on purpose — see StoreMissing."""
if not os.path.isdir(root):
return ("store %s does not exist — nothing was created; pass --out to "
"point elsewhere" % root)
if not all_ids(root):
return "store %s exists but is empty" % root
return None
def path_of(root, id):
return os.path.join(root, "%s.md" % id)
@@ -377,7 +486,7 @@ def load_all(root):
def save(root, issue):
os.makedirs(root, exist_ok=True)
require_store(root)
p = path_of(root, issue.id)
with open(p, "w") as f:
f.write(issue.to_text())
+6 -3
View File
@@ -26,16 +26,19 @@ def main():
ap.add_argument("ids", nargs="*", help="ids to check (default: all)")
ap.add_argument("--quiet", action="store_true", help="exit code only")
ap.add_argument("--strict", action="store_true", help="treat warnings as errors")
ap.add_argument("--out", default=issue.ISSUE_ROOT, help="store root (default: tmp/issues)")
ap.add_argument("--out", default=issue.ISSUE_ROOT,
help="store root (default: <repo>/tmp/issues)")
args = ap.parse_args()
problem = issue.store_error(args.out)
if problem:
sys.exit("issue_check.py: %s" % problem)
issues = issue.load_all(args.out)
ids = args.ids or sorted(issues)
for i in ids:
if i not in issues:
sys.exit("issue_check.py: no issue %r in %s" % (i, args.out))
if not ids:
sys.exit("issue_check.py: store %s is empty" % args.out)
known = set(issues)
bad = 0
+17 -4
View File
@@ -7,8 +7,12 @@ the index acknowledges that a tracker exists: `local` means the issue has never
left this machine, `gitea` means the sync layer has pushed or pulled it. Both
are ordinary issues here.
The store is <repo root>/tmp/issues unless --out says otherwise; an existing
store with nothing in it gets an "_empty_" table, a store that is not there is
an error rather than a directory to create.
Usage:
issue_index.py [--out tmp/issues]
issue_index.py [--out DIR]
"""
import argparse
import os
@@ -27,6 +31,9 @@ def cell(v):
def build(root):
# An index of a store that is not there is not an empty index, it is a bad
# path. Raising beats writing INDEX.md into a directory nobody asked for.
issue.require_store(root)
issues = issue.load_all(root)
rows = []
for i in sorted(issues):
@@ -71,7 +78,6 @@ def build(root):
out.append("")
path = os.path.join(root, "INDEX.md")
os.makedirs(root, exist_ok=True)
with open(path, "w") as f:
f.write("\n".join(out))
return path, len(rows)
@@ -79,9 +85,16 @@ def build(root):
def main():
ap = argparse.ArgumentParser(description="Rebuild the local issue index (offline)")
ap.add_argument("--out", default=issue.ISSUE_ROOT, help="store root (default: tmp/issues)")
ap.add_argument("--out", default=issue.ISSUE_ROOT,
help="store root (default: <repo>/tmp/issues)")
args = ap.parse_args()
path, n = build(args.out)
# An existing store with nothing in it is a legitimate thing to index — it
# gets an "_empty_" table. A store that is not there is not.
try:
path, n = build(args.out)
except issue.StoreMissing as e:
sys.exit("issue_index.py: %s — nothing was created; create an issue with "
"issue_new.py, or pass --out" % e)
print("%s%d issue(s)" % (path, n))
+7 -1
View File
@@ -151,7 +151,8 @@ def main():
ap.add_argument("--assignee", action="append", default=[], help="assignee; repeat")
ap.add_argument("--depends", action="append", default=[],
help="id this issue depends on; repeat")
ap.add_argument("--out", default=issue.ISSUE_ROOT, help="store root (default: tmp/issues)")
ap.add_argument("--out", default=issue.ISSUE_ROOT,
help="store root (default: <repo>/tmp/issues)")
args = ap.parse_args()
labels = ["type/%s" % args.type]
@@ -177,6 +178,11 @@ def main():
labels=labels, assignees=args.assignee, milestone=args.milestone,
depends=args.depends)
# The first issue in a fresh checkout has to create the store, but it says
# so — and it says where, because the path is absolute.
if issue.create_store(args.out):
sys.stderr.write("created store %s\n" % os.path.abspath(args.out))
path = issue.save(args.out, iss)
issue_index.build(args.out)
print("%s [type/%s] %s" % (path, args.type, args.title))
+6 -3
View File
@@ -66,12 +66,15 @@ def main():
ap.add_argument("--depth", type=int, default=6, help="max depth (default: 6)")
ap.add_argument("--write", action="store_true",
help="also write tmp/issues/tree-<slug>.md")
ap.add_argument("--out", default=issue.ISSUE_ROOT, help="store root (default: tmp/issues)")
ap.add_argument("--out", default=issue.ISSUE_ROOT,
help="store root (default: <repo>/tmp/issues)")
args = ap.parse_args()
problem = issue.store_error(args.out)
if problem:
sys.exit("issue_tree.py: %s" % problem)
issues = issue.load_all(args.out)
if not issues:
sys.exit("issue_tree.py: store %s is empty" % args.out)
edges = issue.graph(issues)
roots = args.ids
+6
View File
@@ -48,6 +48,12 @@ Key forms for `<key>`: `42`, `#42`, `owner/repo#42`, or a full issue URL. Repo
defaults to the current directory's git remote; add `--repo owner/repo` outside
one.
`--out` defaults to `issue.ISSUE_ROOT` on every one of them — the domain layer's
`<repo root>/tmp/issues`, resolved from the scripts' own location rather than
cwd. Both layers therefore address the same store by construction, from any
directory. Pass `--out` to override; a relative one stays relative to cwd. Only
`pull.py` will create a missing store, and it says so on stderr.
## Identity mapping
The local id is a slug; Gitea's is a number. The pair is recorded in the issue
+4 -1
View File
@@ -43,10 +43,13 @@ def main():
ap.add_argument("--edit", type=int, metavar="COMMENT_ID",
help="PATCH an existing comment instead of posting a new one")
ap.add_argument("--repo", help="owner/repo (default: auto-detect from CWD git remote)")
ap.add_argument("--out", default=issue.ISSUE_ROOT, help="store root (default: tmp/issues)")
ap.add_argument("--out", default=issue.ISSUE_ROOT,
help="store root (default: <repo>/tmp/issues)")
args = ap.parse_args()
root = args.out
if not issue.store_exists(root):
_gitea.die("store %s does not exist — nothing was created" % root)
if not os.path.isfile(issue.path_of(root, args.id)):
_gitea.die("no issue %r in %s" % (args.id, root))
iss = issue.load(root, args.id)
+7 -1
View File
@@ -108,7 +108,8 @@ def main():
ap.add_argument("--cached", action="store_true",
help="skip issues already on disk instead of refetching")
ap.add_argument("--repo", help="owner/repo (default: auto-detect from CWD git remote)")
ap.add_argument("--out", default=issue.ISSUE_ROOT, help="store root (default: tmp/issues)")
ap.add_argument("--out", default=issue.ISSUE_ROOT,
help="store root (default: <repo>/tmp/issues)")
args = ap.parse_args()
filtered = bool(args.milestone or args.label or args.query)
@@ -118,6 +119,11 @@ def main():
_gitea.die("nothing to pull: pass issue keys, or --milestone / --label / -q")
root = args.out
# A first pull into a fresh checkout has to create the store; it says so,
# and the path is absolute, so it cannot be a stray cwd.
if issue.create_store(root):
sys.stderr.write("created store %s\n" % os.path.abspath(root))
login = _gitea.require_login()
# ---- which repo ------------------------------------------------------
+5 -3
View File
@@ -90,13 +90,15 @@ def main():
ap.add_argument("--dry-run", action="store_true", help="validate only, no network")
ap.add_argument("--force", action="store_true", help="push despite format violations")
ap.add_argument("--repo", help="owner/repo (default: auto-detect from CWD git remote)")
ap.add_argument("--out", default=issue.ISSUE_ROOT, help="store root (default: tmp/issues)")
ap.add_argument("--out", default=issue.ISSUE_ROOT,
help="store root (default: <repo>/tmp/issues)")
args = ap.parse_args()
root = args.out
problem = issue.store_error(root)
if problem:
_gitea.die("%s — create an issue with issue_new.py first" % problem)
issues = issue.load_all(root)
if not issues:
_gitea.die("store %s is empty — create an issue with issue_new.py first" % root)
chosen = select(issues, args.ids, args.update)
+2 -1
View File
@@ -39,7 +39,8 @@ def main():
ap.add_argument("--milestone", help="milestone id or title")
ap.add_argument("--limit", type=int, default=30)
ap.add_argument("--repo", help="owner/repo (default: auto-detect from CWD git remote)")
ap.add_argument("--out", default=issue.ISSUE_ROOT, help="store root (default: tmp/issues)")
ap.add_argument("--out", default=issue.ISSUE_ROOT,
help="store root (default: <repo>/tmp/issues)")
args = ap.parse_args()
login = _gitea.require_login()