jlevy / jlevy/tbd

Suggestion/RFC: auto-installed, idempotent CI-watch reminder on PR creation (cross-agent)

Open
#180 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
79
Forks
9
Avg merge
1d 14h
Merged PRs (30d)
42

Description

**This is a suggestion / RFC, not a prescription.** It proposes an approach and a hook-install design for maintainers and tbd agents to assess and decide. Companion to #179 (the problem + our first tentative attempt); related to #29 (stale hook-script cleanup).

## What this issue adds over #179

#179 reports the scenario (agents file a PR, never watch CI) and shares a tentative standalone hook. This issue proposes (a) the approach we'd lean toward, and (b) — more importantly — how tbd could **auto-install** such a hook **idempotently** when similar hooks may already exist. The idempotency design generalizes to *all* tbd-managed hooks, not just this one.

## Suggested approach (assess and decide)

Prefer **extending the existing `tbd-closing-reminder.sh`** to also match `gh pr create` (in addition to `git push`), rather than shipping a second hook.

Why we lean this way:
- It's **auto-installed across all surfaces already** — `tbd setup --surfaces all` writes `tbd-closing-reminder.sh` into `.claude/`, `.codex/`, etc. No new install path, no new idempotency surface.
- **Single source of truth**: `tbd closing` stays the canonical checklist; the hook just triggers it at one more moment.
- Cross-agent only works upstream anyway: `.codex/` is commonly gitignored / regenerated by `tbd setup`, so a repo-local hook can't durably reach Codex.

Trade-off worth your call: folding into one hook keeps it simple but couples two triggers; a separate hook is cleaner conceptually but adds an install/idempotency surface (below). We don't have a strong opinion — flagging the tradeoff.

## Should tbd auto-install it?

If folded into the existing reminder: **yes, by construction** — nothing new to opt into. If you'd rather gate it, the `--no-gh-cli` flag is a precedent for an opt-out (e.g. `--no-ci-reminder`). Suggestion: on by default, opt-out available.

## Idempotency design (the part we'd most value your judgment on)

`tbd setup --auto` re-runs often (refresh/upgrade), users and other tools also edit `settings.json` / `.codex/hooks.json`, and stale tbd hooks can linger (#29). A robust installer should:

1. **Own its entries by a stable key.** JSON hook entries have no comment field, so key on a *resolved command identity* — e.g. the script basename within a tbd-owned location (`.../tbd-closing-reminder.sh`). Everything keyed this way is "tbd-managed"; everything else is "user/third-party, do not touch."
2. **Upsert, never duplicate.** For each tbd hook `(event, matcher, command-identity)`: update in place if present, append if absent. Re-running setup N times yields exactly one entry. Preserve unknown entries verbatim and in order.
3. **Clean up / migrate stale tbd hooks** (directly addresses #29): remove entries that live in a tbd-owned location but are no longer in the current install set (renamed/removed across versions). This is safe precisely because of the ownership key in (1).
4. **Detect colliding non-tbd hooks.** If a repo already hand-rolled a similar hook (e.g. our `pr-ci-watch-reminder.sh` from #179), don't silently double-fire: warn, and offer `--prune`/`--force` so the stopgap can be retired. Surfacing the collision is enough; auto-removing a user's file would be too aggressive.
5. **Dry-run diff.** `tbd setup --dry-run` should print the exact hook-config delta and **not mutate** (cf. #126, where `--dry-run` mutated). This lets users see idempotent re-runs are no-ops.
6. **One canonical spec → all surfaces.** Derive every surface's config (`.claude/settings.json`, `.codex/hooks.json`, future) from a single hook spec via the same merge, so surfaces can't drift.

A sketch of the merge contract (illustrative):

```
for surface in installed_surfaces:
config = read(surface) # may contain user + tbd + third-party entries
for hook in TBD_HOOKS: # canonical, versioned set
upsert(config, key=hook.identity, value=hook.render(surface))
prune(config, owned_by_tbd and not in TBD_HOOKS) # closes #29
warn_on_collisions(config, TBD_HOOKS) # non-tbd dup commands
write_if_changed(surface, config) # no-op when identical -> idempotent
```

## Open questions for maintainers

- Ownership marker: command-path/basename heuristic vs an explicit sentinel (e.g. a `tbd`-prefixed script dir) — which do you prefer for reliable, non-destructive ownership detection?
- Fold-into-existing-hook vs separate hook (the trade-off above).
- How aggressive should stale-hook pruning be by default vs behind a flag?
- Should the `agents-md` / `portable` surfaces (no hook runtime) get a one-line "monitor CI after a PR" instruction instead?

We're happy to PR a draft (extended `tbd-closing-reminder.sh` + an idempotent merge for the surface writers) if a direction appeals — but deferring the call to you.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.