Skip to content

Commit fe783b7

Browse files
authored
feat(copilot): canonical copilot-instructions.md + sync script (#115)
## Summary Ships the canonical \`.github/copilot-instructions.md\` that GitHub Copilot reads for code review, Chat, and inline generation. Applies to macbook-dev-setup's own reviews immediately; sibling repos (mojwang.tech, ihw) sync via the new script and ship separate follow-up PRs. ## Why canonical + sync (not symlink) GitHub reads \`.github/copilot-instructions.md\` from each repo's own blob server-side — symlinks outside the repo don't resolve. Pattern mirrors existing \`sync-agentic.sh\` (canonical in macbook-dev-setup, discovery from \`\$SECOND_BRAIN_HOME/repos/personal\`) but copies instead of symlinking. ## Canonical scope — universal discipline only After a mid-draft design review, the file deliberately stays out of project-specific voice. Each repo owns its own tone via its own \`CLAUDE.md\`. This canonical covers: - **Review focus:** silent cached failure modes, cache-key consistency, doc/code drift, defense-in-depth on user-facing paths, duplication and magic numbers, security + payment-hook adjacency. - **Universal discipline:** Conventional Commits, <200 LOC/commit, no \`Co-Authored-By Claude\` tags, PR title/body conventions. - **Explicit skip list:** AI Gateway migration (Tier 2.1 deferred), generic "consider adding tests," style-only nits, taste renames. - **Explicitly out of scope:** project voice — reviewed against each repo's own stated conventions. ## Sync script \`scripts/sync-copilot-instructions.sh\`: - \`set -euo pipefail\`, shellcheck clean - Auto-discovers sibling repos under \`\$SECOND_BRAIN_HOME/repos/personal\` that have a \`.github/\` directory; skips macbook-dev-setup itself - \`--repo <path>\` arg to target a specific repo - Idempotent (unchanged-file detection via \`cmp\`) - Copies, not symlinks — target repos commit their own file ## Pair with a branch ruleset for full auto-assignment Per the current GitHub docs, auto-requesting Copilot review on every PR requires a branch ruleset (Settings → Rules → Rulesets → New branch ruleset → \"Automatically request Copilot code review\"). That's a GitHub UI step I can't do via PR — enable it alongside merging this. Together: ruleset = auto-assigned reviewer, this file = house rules loaded into the review. ## Follow-up PRs (not in this one) - mojwang.tech — run \`sync-copilot-instructions.sh\`, commit the resulting \`.github/copilot-instructions.md\` - ihw — same Dry-run verified: the script finds both sibling repos and copies cleanly.
1 parent 4ac6296 commit fe783b7

2 files changed

Lines changed: 179 additions & 0 deletions

File tree

.github/copilot-instructions.md

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Copilot instructions
2+
3+
Canonical `.github/copilot-instructions.md` — applies to Copilot code review, Chat, and inline generation in this repo. Synced to sibling repos via `scripts/sync-copilot-instructions.sh`. Single source of truth; edit here, re-sync downstream.
4+
5+
## Project context
6+
7+
For repo-specific context (stack, conventions, architectural decisions, voice), read the repo's own `CLAUDE.md`, `README.md`, or equivalent. Review guidance below applies universally — review each PR against the *repo's* stated conventions, not patterns from other sibling repos.
8+
9+
## Canonical source
10+
11+
This file is synced from `macbook-dev-setup/.github/copilot-instructions.md` via `macbook-dev-setup/scripts/sync-copilot-instructions.sh`. Edit the canonical; re-sync downstream. Don't edit the copy in a sibling repo without also editing the canonical, or the next sync overwrites the local change.
12+
13+
## Review focus
14+
15+
When reviewing a PR, prioritize these concerns in roughly this order.
16+
17+
### Silent or cached failure modes
18+
- A `catch` that returns an empty / default value inside anything wrapped in `unstable_cache`, React `cache()`, or similar memoization. Once cached, the empty state persists until explicit invalidation. Flag it — suggest a cache-boundary split (throw on error from the cached layer; handle fallback uncached).
19+
- Swallowed errors without logging, or logging without enough context to distinguish sources.
20+
- `try/catch` around code that probably shouldn't fail — often papers over a real bug.
21+
22+
### Cache correctness
23+
- Two cache layers keyed on different normalizations of the same input (e.g. React cache on raw string, `unstable_cache` on sliced string). Both layers must key on the same normalized value.
24+
- Heavy fields (embeddings, large blobs) in a cached projection when consumers don't read them. Flag as cache-payload bloat; suggest explicit column selection.
25+
- Tags referenced in `revalidateTag(...)` with no queries that actually set the tag — dead code.
26+
27+
### Docs ↔ code drift
28+
- Comments / docstrings / PR descriptions that misdescribe actual behavior. Call out specific divergences: "doc says X, code does Y — pick one."
29+
- Stale references to functions, files, or line numbers that have moved. Cite the current location.
30+
- Claims of "deduped," "normalized," "validated," etc. that aren't actually enforced in code.
31+
32+
### Defense-in-depth on user-facing paths
33+
- `useSearchParams().get()` already returns decoded values; calling `decodeURIComponent` again is redundant AND throws `URIError` on malformed percent-encoding.
34+
- Bash scripts without `set -euo pipefail` or equivalent.
35+
- Bash flag parsing that references `$2` without guarding against end-of-args (crashes with `set -u`).
36+
- Shell command construction where user input flows into `eval`, `path.resolve`, or subshell args without validation.
37+
38+
### Duplication and magic numbers
39+
- Literal numbers that appear in multiple files (`3600`, `2000`, etc.) — usually belong in a named constant.
40+
- Patterns copy-pasted across files with identical bodies — usually belong in a shared helper.
41+
- Config scattered across files when one module could own it (e.g., cache timings).
42+
- **Hotfix discipline:** when a PR fixes a bug in file A, check whether the same pattern exists in neighbor files B and C. Flag the scope.
43+
44+
### Security + safety
45+
- Secrets or API keys in client-bundled code, committed `.env` files, or agent frontmatter.
46+
- `Bash` commands that touch paid services (Stripe, x402, Browserbase, parallel.ai, tempo) without the `CLAUDE_ALLOW_PAYMENTS=1` escape valve — the payment-safety hook exists for a reason.
47+
- Admin DB creds (`DATABASE_URL_ADMIN`) in `.env.local` — they belong in `.env.sync-vault.local`.
48+
49+
## Universal discipline (applies to every repo this file is synced to)
50+
51+
- **Commit format:** Conventional Commits — `type(scope): description`. Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert.
52+
- **Small commits:** aim for <200 LOC per diff. Larger is sometimes right, but call out the reason in the commit body.
53+
- **No `Co-Authored-By` Claude tags** in commits or PRs. No "Generated with Claude Code" footers.
54+
- **PR titles** are the TL;DR; detail goes in the body.
55+
- **PR bodies** include a Test plan with specific, runnable checks — not "verified locally" without saying what was verified.
56+
57+
## Project-specific voice and style
58+
59+
This file does not prescribe voice. Each repo owns its own tone:
60+
- Writing / comments / PR bodies should match the repo's existing CLAUDE.md or equivalent style guide.
61+
- If a repo's style is silent on a point, follow the surrounding code's idiom, not a preference from another repo.
62+
- Don't suggest style edits purely on voice grounds when the surrounding code is consistent with itself.
63+
64+
## Skip (don't suggest these)
65+
66+
- **AI Gateway migration** (Tier 2.1). Direct `openai` imports in `src/db/queries/related.ts`, `scripts/sync-vault.ts` are known and deferred — stop flagging them as a recommendation.
67+
- **Generic "consider adding tests"** without naming the specific behavior gap the new test would close.
68+
- **Style-only nits** in files the PR didn't change.
69+
- **Renaming for taste** unless it prevents a future conflict or bug.
70+
- **Introducing new runtime dependencies** to replace a small amount of custom code.
71+
- **Vercel-specific "migrate to X" advice** unless X is a current, non-deprecated product.
72+
73+
## What good feedback looks like
74+
75+
Concrete, cites file + line, suggests a specific fix, explains the failure mode it prevents. Example: *"`getNoteBySlugCached` uses `.select()` (all columns) including the 1536-dim embedding vector — ~12KB per cached row. Consider selecting only the 8 fields `/mind/[slug]/page.tsx` reads, because the embedding is never consumed by callers."*
76+
77+
Not: *"Consider narrowing the column selection."*
78+
79+
## When in doubt
80+
81+
If you can't decide whether to flag something, err toward naming the specific concrete risk and letting the author judge. Vague concerns ("this could be clearer") create review noise; specific ones ("line 47 swallows a network error that was previously surfaced") earn their place.
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
#!/usr/bin/env bash
2+
# sync-copilot-instructions.sh — Copy the canonical copilot-instructions.md
3+
# to a target repo's .github/ directory.
4+
#
5+
# Unlike sync-agentic.sh (which symlinks), this script COPIES because
6+
# GitHub reads .github/copilot-instructions.md server-side from each
7+
# repo's own blob — symlinks outside the repo don't resolve. Each target
8+
# repo commits its own copy.
9+
#
10+
# Usage:
11+
# sync-copilot-instructions.sh # auto-discover siblings
12+
# sync-copilot-instructions.sh /path/to/repo # single target
13+
#
14+
# Auto-discovery walks $SECOND_BRAIN_HOME/repos/personal/*/ and copies
15+
# the canonical into any repo that has a .github/ directory. Skips
16+
# macbook-dev-setup itself (it IS the canonical).
17+
18+
set -euo pipefail
19+
20+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
21+
MACBOOK_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
22+
CANONICAL="$MACBOOK_ROOT/.github/copilot-instructions.md"
23+
24+
# Colors
25+
GREEN='\033[0;32m'
26+
YELLOW='\033[0;33m'
27+
RED='\033[0;31m'
28+
NC='\033[0m'
29+
30+
if [[ ! -f "$CANONICAL" ]]; then
31+
echo -e "${RED}Canonical not found: $CANONICAL${NC}" >&2
32+
exit 1
33+
fi
34+
35+
# Allow override of the sibling-discovery root. Defaults match the same
36+
# env var sync-agentic.sh uses, so both scripts share one config point.
37+
# shellcheck disable=SC1091
38+
source "$MACBOOK_ROOT/.personal/config.sh" 2>/dev/null || true
39+
ROOT="${SECOND_BRAIN_HOME:-$HOME/ai/workspace/claude}/repos/personal"
40+
41+
copy_to() {
42+
local target_repo="$1"
43+
local target_dir="$target_repo/.github"
44+
local target_file="$target_dir/copilot-instructions.md"
45+
46+
if [[ ! -d "$target_repo" ]]; then
47+
echo -e "${RED}$target_repo — not a directory${NC}" >&2
48+
return 1
49+
fi
50+
51+
if [[ "$(cd "$target_repo" && pwd)" = "$MACBOOK_ROOT" ]]; then
52+
# Canonical lives here; nothing to copy.
53+
return 0
54+
fi
55+
56+
mkdir -p "$target_dir"
57+
58+
if [[ -f "$target_file" ]] && cmp -s "$CANONICAL" "$target_file"; then
59+
echo -e "${GREEN}${NC} $target_repo (unchanged)"
60+
return 0
61+
fi
62+
63+
cp "$CANONICAL" "$target_file"
64+
echo -e "${GREEN}${NC} $target_repo (copied)"
65+
}
66+
67+
if [[ $# -gt 0 ]]; then
68+
# Explicit target(s): copy to each and exit.
69+
rc=0
70+
for target in "$@"; do
71+
copy_to "$target" || rc=$?
72+
done
73+
exit "$rc"
74+
fi
75+
76+
# Auto-discover: any repo with .github/ directory under personal/
77+
if [[ ! -d "$ROOT" ]]; then
78+
echo -e "${YELLOW}Discovery root not found: $ROOT${NC}" >&2
79+
echo "Set SECOND_BRAIN_HOME or pass an explicit target." >&2
80+
exit 1
81+
fi
82+
83+
echo "Syncing copilot-instructions.md from: $CANONICAL"
84+
echo "Discovery root: $ROOT"
85+
echo
86+
87+
rc=0
88+
for repo in "$ROOT"/*/; do
89+
repo="${repo%/}"
90+
if [[ -d "$repo/.github" ]]; then
91+
copy_to "$repo" || rc=$?
92+
fi
93+
done
94+
95+
if [[ $rc -ne 0 ]]; then
96+
echo -e "${RED}Some copies failed.${NC}" >&2
97+
fi
98+
exit "$rc"

0 commit comments

Comments
 (0)