Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions .catalyst-code/skills/add-core-background-task/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
---
name: add-core-background-task
description: Add a periodic background task to the Rust core (heartbeat, poll, presence publisher) that runs independent of turns
version: 1
---

## When to use

You need something in `catalyst-code`'s Rust core that runs CONTINUOUSLY, on a
timer, independent of turns — not triggered by a tool call or a user message.
Examples already in the codebase:

- **`umans_conc` poll** (`main.rs`) — every 5s hits `/v1/usage`, emits
`umans_conc { used, limit, provider }` so the footer shows live concurrency.
- **Presence heartbeat** (`main.rs` + `presence` module) — every 8s publishes
this session's `WorkState` to a per-pid file so peers can detect concurrent
activity, and refreshes a cached peer snapshot.

If the thing must happen AT a specific point in the turn loop (before/after a
model request, on each tool result), that's a **plugin hook** or a `run_turn`
injection — NOT a background task. Background tasks are for ambient polling/
broadcasting that has no per-turn trigger.

## Where things live

- **Spawn site** — `core/src/main.rs::async fn main()`, AFTER the
`Arc<State>` is constructed and BEFORE `let stdin = …; while let … lines.next_line()`.
The existing `umans_conc` poll block is the canonical template — copy its shape.
- **The task** — a `tokio::spawn(async move { loop { …; tokio::time::sleep(interval).await; } })`.
Capture `state.clone()` (an `Arc<State>`) + any read-once values (pid, a
workspace clone) into the move. Inside, snapshot State fields via their
`tokio::sync::Mutex`/`RwLock` (`.lock().await` / `.read().await`), hold the
guard only as long as needed (`drop(guard)`), then do the I/O.
- **State field for output** — if a consumer needs the polled value on the hot
path (e.g. a per-tool-result check), cache it in a NEW `Mutex<T>` field on
`State` that the task refreshes each tick; consumers read the cache (cheap,
no I/O) instead of re-polling. Add the field to the `State` struct + initialize
it in the `Arc::new(State { … })` literal. (See presence: `peers: Mutex<Vec<_>>`
refreshed by the heartbeat, read by `maybe_concurrency_note` in `run_turn`.)

## Steps

1. **Decide cadence + whether a hot-path cache is needed.** If a turn-loop
consumer needs the value, add a `State` field for the cached snapshot
(struct decl + init literal) and have the task refresh it each tick. If the
task only EMITS an event (no turn-loop consumer), no State field is needed.
2. **Capture read-once inputs before the move** — `std::process::id()`, a
`workspace.clone()` read from `state.cfg` (cfg is moved into the `RwLock` at
State construction), `home_dir()`, a `started_at` timestamp. These are
constant for the process lifetime, so reading them once avoids per-tick
lock churn.
3. **Do the first write/emit IMMEDIATELY (before the loop)** so a consumer
checking right after startup sees the value — don't wait one full interval.
4. **Spawn the task** mirroring the `umans_conc` block: clone `state`, move it
in, `loop { …work…; tokio::time::sleep(interval).await; }`. Keep work cheap;
a background tick should never block the turn loop (it can't — it's a
separate task — but a slow tick delays the next tick).
5. **Crash-safety for any FILE the task writes** — use atomic temp+fsync+rename
(same pattern as `session`/`memory` persistence). If the task writes a
per-process file, also provide a `clear_<thing>()` called on clean shutdown
(the stdin-EOF path at the tail of `main`, after awaiting the running turn)
so the file disappears instantly; tolerate `kill -9` via a stale-reaper on
read (mtime threshold) since the shutdown path doesn't run on SIGKILL.

## Gotchas

- **`tokio::time::sleep`, not `std::thread::sleep`.** The task runs on the
async runtime; `std::thread::sleep` blocks a worker thread.
- **Don't hold a `MutexGuard` across `.await`.** Snapshot the value (`.clone()`)
and `drop(guard)` before any `.await`/I/O, or you'll serialize the task behind
the turn loop (and vice versa). The `umans_conc` block does this correctly.
- **`name` vs `&name`** — the dispatch variables in `run_turn` are owned
`String`s; a helper taking `&str` needs `&name` (deref coercion), not `name`.
- **The debug binary `core/target/debug/core` can vanish between
`cargo build` and a smoke run** — a concurrent `cargo test`/`cargo clippy`
(e.g. this session's own harness) reorganizes incremental artifacts and the
main bin can be absent while `deps/` remains. Re-run `cargo build` immediately
before any smoke test that execs the binary.
- **Config root is `~/.config/catalyst-code/`** (post-rename; older notes may
say `umans-harness`). Per-workspace scoping reuses `crate::memory::project_hash`
(already `pub`, fnv1a 16-hex of canonicalized cwd) — no need to promote or
duplicate it.

## Smoke-testing

A background task that writes a file is best verified by a real launch: start
the binary with a FIFO stdin, send `{"type":"init"}`, sleep one+ interval, then
check the file exists with the expected content; then close stdin (EOF, NOT
`kill` — `kill` skips the shutdown-cleanup path) and confirm the file is gone.
Use `find ~/.config/catalyst-code/<subsys>/ -name "<pid>.<ext>"` to locate the
per-process file (don't assume the workspace-hash subdir — list and match by pid).
19 changes: 13 additions & 6 deletions .catalyst-code/skills/git-commit-all/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,23 @@ Do NOT use for:
## Steps

1. **Show what will be committed**: `git status --short` and `git diff --stat` so the user can confirm.
2. **Stage everything**: `git add --all` (stages modified, new, and deleted files).
3. **Build a commit message** from the diff summary:
- Read the file list from `git diff --cached --stat`
2. **Verify it compiles** (don't land broken code): run the project's type checker / build for each *changed* component before committing — e.g. `cd core && cargo check` (main binary; avoid `cargo check --tests` if a pre-existing test-binary breakage unrelated to your change is known), `cd tui && go build ./... && go vet ./...`, `cd web && npx tsc --noEmit`. Run the changed ones in parallel; only block the commit on errors in files YOU touched (isolate concurrent-user edits — see the concurrent-user-edits-isolate-errors gotcha). Skip components you didn't touch.

**Prefer the EXACT CI gates, not just "it compiles"** — especially when the diff touches formatting or adds/changes a CI gate. A plain `cargo check` / `go build` won't catch fmt drift that CI fails on. The authoritative gates (mirror `.github/workflows/ci.yml`):
- core: `cargo fmt --all -- --check` · `cargo clippy --all-targets` (treat warnings as errors under `-D warnings` if CI sets it) · `cargo test --locked`
- tui: `gofmt -l .` (must be EMPTY) · `go vet ./...` · `go build ./...` · `go test -race ./...` (the `-race` matters — catches data-race fixes a plain `go test` won't)
- When a commit ADDS a new CI gate (e.g. a gofmt step), run that gate yourself before committing — the gate's first run should already pass on your commit.
3. **Stage everything**: `git add --all` (stages modified, new, and deleted files).
4. **Build a commit message** from the diff:
- For a SMALL change, `git diff --cached --stat` (file list + churn) is enough.
- For a LARGE or multi-feature diff (hundreds of lines, many files), read the FULL `git diff` (not just `--stat`) — `--stat` won't reveal the distinct features interleaved across shared files like `main.rs`; the actual hunks let you enumerate each feature accurately in the body.
- Group by directory/module (e.g. "core: …", "tui: …", "web: …")
- Include the primary change type (e.g. "refactor", "fix", "add feature X")
- If a single logical change spans files, use one sentence; if multiple, use bullet points
- Keep the subject line ≤72 chars; body wraps at 72
4. **Commit**: `git commit -m "<message>"`
5. **Show the commit**: `git log -1 --oneline`
6. **Push (only if the user asked to push)**:
5. **Commit**: `git commit -m "<message>"`
6. **Show the commit**: `git log -1 --oneline`
7. **Push (only if the user asked to push)**:
- Current branch: `git rev-parse --abbrev-ref HEAD` (if it prints `HEAD`, you're in detached HEAD — abort and tell the user to checkout a branch first).
- If the branch has an upstream (`git rev-parse --abbrev-ref @{u}` succeeds): `git push`.
- If it has NO upstream: `git push -u origin <branch>` (sets upstream on first push).
Expand Down
135 changes: 135 additions & 0 deletions .catalyst-code/skills/production-readiness-review/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
---
name: production-readiness-review
description: Audit a repo for production readiness before going public — fan out code reviewers + a secrets/PII scan + a build/test gate, then synthesize a go/no-go verdict
---

# Production-Readiness Review (Pre-Public Launch)

Use when the user is about to make a repo **public** (or cut a release) and asks
"is this ready?" / "production readiness" / "can we go public?". Distinct from a
general code-quality review (`parallel-codebase-review`) — this adds the three
things that specifically matter for *going public*: a **secrets/PII audit**, a
**build + test + lint gate**, and an explicit **go/no-go verdict**.

## When to use
- "Is the repo ready to go public / open-source?"
- Pre-release gate before flipping a repo from private to public.
- "Production readiness review" of a whole codebase.

## When NOT to use
- Targeted review of one file/feature → read it directly.
- General bug hunt with no public-launch angle → use `parallel-codebase-review`.
- A single component → one `reviewer` suffices, no fan-out needed.

## Workflow

### 1. Fan out 3–4 parallel subagents (respect the user's subagent cap)
Each is a fresh context — be self-contained: name the exact files, the focus
areas, the output contract (`file:line` evidence + severity + concrete fix), and
any "ignore this in-progress feature" caveat the user gave.

| Reviewer | Scope |
|----------|-------|
| `reviewer` (code) | one per major language component (Rust core, Go TUI, …). Adversarial: panics on adversarial input, resource leaks, error swallowing, secret-in-code, security (path confinement, command injection, sandbox). |
| `reviewer` (secrets/PII) | **repo-wide** — hardcoded secrets/tokens/private keys, personal paths/emails/IPs, committed binaries/artifacts, local-only files tracked, LICENSE/README/CI sanity, author fields in manifests. This is the highest-stakes pass. |
| `worker` (build gate) | actually RUN the lint/build/test suite. Report per-step PASS/FAIL + warnings, and split known/in-progress issues from real regressions. |

Tell each reviewer what is **expected-incomplete** (e.g. "presence feature is
mid-implementation — don't flag its incompleteness") so it doesn't waste budget
on known work, and what's a **known pre-existing issue** vs a new regression.

### 2. Re-verify the secrets/PII audit YOURSELF — don't trust a truncated summary
A subagent's detailed body sometimes gets truncated to a one-line verdict in the
orchestrator's parallel-task result. For the highest-stakes claim ("secrets are
clean"), **re-run the scan yourself** with `rg --hidden` (rg skips dotfiles by
default — always pass `--hidden`, exclude `.git/`, `target/`, `node_modules`).
Write a small script (don't inline long `rg` chains) and check:

- Real-looking secrets: `sk-[A-Za-z0-9]{16,}`, `AKIA[0-9A-Z]{16}`,
`gh[pousr]_…`, `xoxb-`, `Bearer <real-token>`, `BEGIN … PRIVATE KEY`.
Filter out obvious placeholders (`example`, `your_key`, `<key>`, `sk-xxxx`).
- Personal paths: `/home/<user>`, `/Users/<user>`, `/root`, `C:\Users\…`
(a container service user like `harness` is fine; a real username is PII).
- Emails; private keys; internal hostnames (`.local`, `.internal`, cloud-metadata
`169.254.169.254` — note: that IP appearing in code is often the SSRF *risk
being documented*, not a leak).
- Tracked local-only files: `git ls-files | rg '^scripts/|^tmp/|context\.md|plan\.md|\.env|\.log|\.pem|\.key'`.
- Committed large binaries: `git ls-files -z | while read f; do git cat-file -s "HEAD:$f"; done` — flag anything >1MB.
- `LICENSE` + `README` presence; author/owner fields in `Cargo.toml`/`go.mod`/`package.json` (PII leak); CI `secrets.` usage + `pull_request_target` + internal URLs.

A legitimate MIT/Apache LICENSE attributed to the user's own handle is fine (it's
their copyright, not a leak). The auto-provided `${{ secrets.GITHUB_TOKEN }}` is
standard, not a custom secret.

### 3. Verify surprising Critical/High findings before reporting them
Re-read the cited `file:line` yourself — converts "the reviewer said" into
"verified." Especially for security claims (e.g. "writes keys world-readable"):
confirm the code actually does what's claimed and that the doc comment (if any)
contradicts it. Line numbers drift; the code is the truth.

### 4. Synthesize — don't dump
Merge the reports, **dedupe across reviewers**, and rank by severity:
- **P0 blocker** — must fix or do NOT go public (a leaked secret, a crash on
normal input, broken build).
- **P1 should-fix** — small surgical fixes to land before public; not
architectural (a permission gap, a soft wedge, a real data race, fmt drift).
- **P2 nice-to-have** — hardening for a fast follow-up; doesn't block launch.

Lead with a one-line **go/no-go verdict**, then the P1 table (where / issue /
fix), then P2s, then a **"verified clean"** section so the user knows what WAS
checked (security boundaries, no-panic-on-bad-input, resource bounds, secrets
absent). End by offering to implement the P1 fixes.

## Gotchas
- **Stale "known issue" memories.** Before treating a compile/test failure as a
known pre-existing issue, actually run the gate — the tree may have moved on
and the issue resolved. Update the stale memory if so (don't leave a wrong
"tests are broken" note that scares off the next session).
- **Subagent body truncation.** If a parallel task returns only a one-line
summary instead of its detailed body, that detail is lost — re-verify the
critical claims yourself rather than reporting the bare verdict.
- **CI blind spots.** The build gate should run the SAME checks CI runs; if CI
omits something (e.g. a Go project whose CI runs `go vet`/`go build`/`go test`
but NOT `gofmt --check`), call that out as a finding — formatting drift slips
through silently. `cargo fmt --all` / `gofmt -w .` are one-command fixes.
- **The subagent hard cap.** If you need >8 parallel reviewers, batch them ≤8
(the `tasks` mode rejects > `parallel_max_tasks` instantly). See
`parallel-subagent-cap` memory.

## Applying the findings (fix-all)

When the user says "fix all issues" / "fix everything found" after a review,
this is the apply-and-verify counterpart. The risk is volume + correctness
across two codebases — structure it so each file tree has ONE writer and the
trickiest fix is yours.

1. **Delegate a cohesive same-tree cluster to ONE worker.** Group all fixes
that share files (e.g. every `tui/main.go` lifecycle fix: atomic process
ptr, signal-handler quit, startup watchdog, double-`Wait`) into a single
worker so it keeps coherence across its own edits. Give it EXACT fixes
(file:line + the change + the rationale) — workers execute well-specified
edits reliably; vague ones they botch. Tell it what's expected-incomplete
("presence feature is mid-impl — don't touch it") and that it's the SOLE
writer of its tree (so it may run the formatter itself, no race).
2. **Keep the security-sensitive / judgment-heavy fix for yourself.** SSRF
range-blocking, sandbox-bypass fixes, lifecycle redesigns — read the file,
design carefully, write tests. Don't hand these to a worker.
3. **Verify with the EXACT CI gates, not just "it compiles":**
- core: `cargo fmt --all -- --check` · `cargo clippy --all-targets` ·
`cargo test --locked`
- tui: `gofmt -l .` (empty) · `go vet ./...` · `go build ./...` ·
`go test -race ./...` (the `-race` matters — it catches the data-race
fixes a plain `go test` won't).
4. **Isolate failures to your own edits.** A test failing in a file you didn't
touch is almost certainly the user's concurrent work (or a pre-existing
issue) — confirm with `git status --short` which modified files are yours
vs. theirs before "fixing" it. (See `concurrent-user-edits-isolate-errors`.)
5. **Spot-check worker output — build-passing ≠ fixes-correct.** A worker
that claims "all 10 fixes done, tests green" may have no-op'd a tricky one.
`grep` for the specific fix markers (e.g. `atomic.Pointer`, `s.busy = false`
in the reset handler, `go fillMentionCache`, `modelIdx = -1`) and read the
one or two trickiest regions yourself. (The `grep` TOOL is flaky — use
`bash grep -n` instead.)
6. **Run the formatters yourself at the end** (`cargo fmt --all`, `gofmt -w .`)
and confirm `--check`/`-l` is clean — this is both a fix (P1 fmt drift) and
the CI gate.
Loading
Loading