chore(agents): scripts/wt and workflow-doc follow-ups from #139 review

gabogg commented

2026-09-26 16:28:28 +00:00

Owner

Follow-ups from the second-pass review of #139 (review comment). All are P3. Each item names its fix, so the design is settled.

Standards

S2-1 scripts/wt local_files(): build the ignore regex from SHARED instead of hard-coding ^(\.venv|\.env|scripts/tailwindcss)$, so adding a shared symlink is a one-place edit.
S2-2 scripts/wt cmd_pr: give unpushed() a ref argument and reuse it, replacing the inline rev-list --count "$branch" --not --remotes=….
S2-3 (done in #141) scripts/wt cmd_rm: canonicalise the resolved directory with realpath before the "$WT_DIR"/* prefix check, so . and .. are rejected (today wt rm .. -f reaches git worktree remove --force on the main checkout).
S2-4 scripts/wt cmd_pr, where a local branch has no worktree: verify fast-forward (merge-base --is-ancestor) before git worktree add, so a divergence leaves no orphan worktree.
S2-5 scripts/wt remove(): message should read "removed ; deleted merged branch ".

S2-6 docs/agents/forgejo-cli.md: drop "Installed at ~/.local/bin/tea on the dev workstation" (it caches the environment).

S2-7 docs/standards/git-and-workflow.md: "Merge commit (no squash or rebase)" → "Merge commit only".

Spec

C2-1 (done in #141) Pruning from inside a worktree deletes the session's own cwd. scripts/wt should refuse to remove the worktree containing $PWD (in both rm and prune, with a "run from the main checkout" message), and forgejo-cli.md §Merging should say "from the main checkout".

C2-2 scripts/wt prune: when a worktree's branch has no upstream (pushed without -u), print kept <dir>: no upstream instead of skipping it silently.

Verification

Exercise each wt change on local throwaway branches. Pre-commit (ruff, pytest) and scripts/check_docs.py must be green.

Maintainer triage — 2026-09-27

For C2-3, prevent test and documentation runs from creating runtime databases/logs in worktrees: use isolated temporary storage and avoid runtime initialization during documentation inspection where possible. Keep ordinary pruning conservative for remaining local databases; do not automatically delete them based on size or make force removal the routine workflow. Verify no data/hikcentral.db or logs/lifecycle.log artifacts are left by the affected test/doc commands. Retain all outstanding listed chores; #141 already completed the checked items.

Follow-ups from the second-pass review of #139 ([review comment](https://git.gaboggamer.online/gabogg/hikcentral/pulls/139#issuecomment-2543)). All are P3. Each item names its fix, so the design is settled. ## Standards - [ ] **S2-1** `scripts/wt` `local_files()`: build the ignore regex from `SHARED` instead of hard-coding `^(\.venv|\.env|scripts/tailwindcss)$`, so adding a shared symlink is a one-place edit. - [ ] **S2-2** `scripts/wt` `cmd_pr`: give `unpushed()` a ref argument and reuse it, replacing the inline `rev-list --count "$branch" --not --remotes=…`. - [x] **S2-3** *(done in #141)* `scripts/wt` `cmd_rm`: canonicalise the resolved directory with `realpath` before the `"$WT_DIR"/*` prefix check, so `.` and `..` are rejected (today `wt rm .. -f` reaches `git worktree remove --force` on the main checkout). - [ ] **S2-4** `scripts/wt` `cmd_pr`, where a local branch has no worktree: verify fast-forward (`merge-base --is-ancestor`) *before* `git worktree add`, so a divergence leaves no orphan worktree. - [ ] **S2-5** `scripts/wt` `remove()`: message should read "removed <dir>; deleted merged branch <b>". - [ ] **S2-6** `docs/agents/forgejo-cli.md`: drop "Installed at `~/.local/bin/tea` on the dev workstation" (it caches the environment). - [ ] **S2-7** `docs/standards/git-and-workflow.md`: "Merge commit (no squash or rebase)" → "Merge commit only". ## Spec - [x] **C2-1** *(done in #141)* Pruning from inside a worktree deletes the session's own cwd. `scripts/wt` should refuse to remove the worktree containing `$PWD` (in both `rm` and `prune`, with a "run from the main checkout" message), and `forgejo-cli.md` §Merging should say "from the main checkout". - [ ] **C2-2** `scripts/wt prune`: when a worktree's branch has no upstream (pushed without `-u`), print `kept <dir>: no upstream` instead of skipping it silently. ## Verification Exercise each `wt` change on local throwaway branches. Pre-commit (ruff, pytest) and `scripts/check_docs.py` must be green. ## Maintainer triage — 2026-09-27 For C2-3, prevent test and documentation runs from creating runtime databases/logs in worktrees: use isolated temporary storage and avoid runtime initialization during documentation inspection where possible. Keep ordinary pruning conservative for remaining local databases; do not automatically delete them based on size or make force removal the routine workflow. Verify no data/hikcentral.db or logs/lifecycle.log artifacts are left by the affected test/doc commands. Retain all outstanding listed chores; #141 already completed the checked items.

gabogg added the

ready-for-agent

label

2026-09-26 16:28:28 +00:00

gabogg commented

2026-09-26 16:29:14 +00:00

Author

Owner

C2-3 (found while merging #139): `prune` keeps almost every worktree. Open design point

After #139 merged, scripts/wt prune from the main checkout kept .worktrees/docs-agent-workspaces ("holds ignored local files"). Only two files blocked it, both produced by ordinary test and doc runs inside the worktree:

data/hikcentral.db: 4 KB, empty schema, created at app import (scripts/check_docs.py / pytest)
logs/lifecycle.log

So any worktree that has run the suite needs wt rm -f after merge, which weakens the S1 guard: once -f becomes routine, it stops protecting anything.

A decision is needed before implementation. Options:

Treat logs/ as expendable in local_files(), and only protect data/ files larger than an empty-schema DB (or any non-empty table). This is precise, but needs a sqlite check in bash.
Keep test and doc runs out of data/. Point the app at a temp DB under test (env var / conftest) so data/ only appears when someone deliberately runs the app in the worktree. This fixes the cause, but touches app config.
Keep refusing, but show sizes in the "kept" message, so a 4 KB empty DB is obviously safe to -f. The cheapest option; the guard still asks a human or agent to judge.

Relabelled needs-triage until this is settled; the other items in this issue are ready as written.

## C2-3 (found while merging #139): `prune` keeps almost every worktree. **Open design point** After #139 merged, `scripts/wt prune` from the main checkout kept `.worktrees/docs-agent-workspaces` ("holds ignored local files"). Only two files blocked it, both produced by ordinary test and doc runs inside the worktree: - `data/hikcentral.db`: 4 KB, empty schema, created at app import (`scripts/check_docs.py` / pytest) - `logs/lifecycle.log` So any worktree that has run the suite needs `wt rm -f` after merge, which weakens the S1 guard: once `-f` becomes routine, it stops protecting anything. A decision is needed before implementation. Options: 1. **Treat `logs/` as expendable** in `local_files()`, and only protect `data/` files larger than an empty-schema DB (or any non-empty table). This is precise, but needs a sqlite check in bash. 2. **Keep test and doc runs out of `data/`.** Point the app at a temp DB under test (env var / conftest) so `data/` only appears when someone deliberately runs the app in the worktree. This fixes the cause, but touches app config. 3. **Keep refusing, but show sizes** in the "kept" message, so a 4 KB empty DB is obviously safe to `-f`. The cheapest option; the guard still asks a human or agent to judge. Relabelled `needs-triage` until this is settled; the other items in this issue are ready as written.

gabogg added

needs-triage

and removed

ready-for-agent

labels

2026-09-26 16:29:16 +00:00

gabogg referenced this issue from a commit

2026-09-26 16:35:28 +00:00

fix(wt): interim safety guards from #140 review follow-ups

gabogg referenced this issue

2026-09-26 16:35:43 +00:00

fix(wt): interim safety guards from #140 review follow-ups #141

gabogg referenced this issue

2026-09-26 20:41:48 +00:00

fix(wt): interim safety guards from #140 review follow-ups #141

gabogg referenced this issue from a commit

2026-09-26 20:45:06 +00:00

fix(wt): address code-review findings for PR #141

gabogg referenced this issue

2026-09-26 20:45:21 +00:00

fix(wt): interim safety guards from #140 review follow-ups #141

gabogg referenced this issue from a commit

2026-09-26 20:47:55 +00:00

fix(wt): interim safety guards from #140 review follow-ups (#141)

gabogg commented

2026-09-26 20:48:17 +00:00

Author

Owner

#141 merged (02f0971): S2-3 (wt rm path canonicalisation) and C2-1 (cwd guard in rm/prune, docs say "from the main checkout") are done and ticked. C2-3 got an interim mitigation only: refusals list each blocking file with its size (quoted names, symlinks, capped at 20), and prune reports only merged candidates. The permanent policy is still open, so this issue stays needs-triage.

#141 merged (02f0971): **S2-3** (`wt rm` path canonicalisation) and **C2-1** (cwd guard in `rm`/`prune`, docs say "from the main checkout") are done and ticked. **C2-3** got an interim mitigation only: refusals list each blocking file with its size (quoted names, symlinks, capped at 20), and `prune` reports only merged candidates. The permanent policy is still open, so this issue stays `needs-triage`.

gabogg referenced this issue

2026-09-26 21:23:50 +00:00

chore(agents): one worktree per agent, and a Forgejo CLI instead of curl/Python snippets #118

gabogg commented

2026-09-27 10:06:27 +00:00

Author

Owner

Added from the #154 second-pass review (P3): in docs/agents/forgejo-cli.md, change "your worktree's scratchpad subfolder" to "your scratchpad subfolder". The rule in workspaces.md#scratch-files also covers agents without a worktree (reviewers, orchestrators), who name the subfolder after their task.

Added from the #154 second-pass review (P3): in `docs/agents/forgejo-cli.md`, change "your worktree's scratchpad subfolder" to "your scratchpad subfolder". The rule in `workspaces.md#scratch-files` also covers agents without a worktree (reviewers, orchestrators), who name the subfolder after their task.

gabogg referenced this issue

2026-09-27 10:06:28 +00:00

docs(agents): per-worktree scratch subfolders for parallel agents #154

gabogg added

ready-for-agent

low-priority

and removed

needs-triage

labels

2026-09-27 16:29:20 +00:00