From 3cb467e6aa4356f9912d661ef12d581d61b65cb6 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Fri, 22 May 2026 16:59:56 -0500 Subject: feat: split team publishing rules into an installable overlay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global commits.md carried DeepSat-specific publishing steps — Linear ticket-state moves, the Slack notification protocol with its channel ID and engineer names, the deepsat.ghe.com host, the team merge norm. Those are symlinked into every project on the machine, so they sat as dead weight in personal repos and risked misfiring where there's no Linear ticket to move or Slack mpdm to ping. I split them out. commits.md keeps the universal skeleton (identity, attribution, commit format, the review-and-publish gate, verification) and replaces the team steps with seams: "run the project's publishing overlay here if it defines one," the same pattern startup.org uses for startup-extras. A project with no overlay runs the complete flow, just without ticket and chat integration. The DeepSat specifics move to teams/deepsat/claude/rules/publishing.md. That file is not a global rule — install-team.sh copies it into one project's .claude/rules/ (make install-team TEAM=deepsat PROJECT=...), keyed on the PROJECT argument, so only the named project gets it. Location decides distribution: claude-rules/ is the global-symlink set, teams/ is targeted-copy, so the overlay reaches DeepSat and nowhere else. The startup freshness check (sync-language-bundle.sh) now covers team overlays alongside language bundles: a process_bundle function handles both, with a team syncing only its own rule (no generic rules, hooks, or settings — those belong to a language bundle). A drifted overlay rule auto-fixes from canonical at the project's next startup, the same mechanism language bundles already ride. Tested: 3 new bats cases (team overlay clean / drifted-and-fixed / does-not-pull-generic-rules) on top of the 11 existing; install-team + sync verified end-to-end against a temp project. make test green, shellcheck clean. --- Makefile | 21 ++++++ claude-rules/commits.md | 61 ++++----------- scripts/install-team.sh | 49 ++++++++++++ scripts/sync-language-bundle.sh | 123 ++++++++++++++++++------------- scripts/tests/sync-language-bundle.bats | 39 ++++++++++ teams/deepsat/claude/rules/publishing.md | 67 +++++++++++++++++ 6 files changed, 265 insertions(+), 95 deletions(-) create mode 100755 scripts/install-team.sh create mode 100644 teams/deepsat/claude/rules/publishing.md diff --git a/Makefile b/Makefile index 3508510..44431dc 100644 --- a/Makefile +++ b/Makefile @@ -16,6 +16,7 @@ OPTIN_HOOKS := hooks/destructive-bash-confirm.py DEFAULT_HOOKS := $(filter-out $(OPTIN_HOOKS),$(HOOKS)) CLAUDE_CONFIG := $(wildcard .claude/*.json) $(wildcard .claude/.*.json) LANGUAGES := $(notdir $(wildcard languages/*)) +TEAMS := $(notdir $(wildcard teams/*)) PDFTOOLS_VENV ?= $(HOME)/.local/venvs/pdftools # LANG is also the standard POSIX locale env var. Make inherits env vars into @@ -54,6 +55,17 @@ define pick_lang_shell test -n "$$L" || { echo "No language selected." >&2; exit 1; }; \ fi endef +define pick_team_shell + T="$(TEAM)"; \ + if [ -z "$$T" ]; then \ + if ! command -v fzf >/dev/null 2>&1; then \ + echo "ERROR: TEAM= not set and fzf is not installed" >&2; \ + exit 1; \ + fi; \ + T=$$(printf '%s\n' $(TEAMS) | fzf --prompt="Team> " 2>/dev/null); \ + test -n "$$T" || { echo "No team selected." >&2; exit 1; }; \ + fi +endef # Cross-platform package install helper (brew/apt/pacman) define install_pkg @@ -383,6 +395,15 @@ install-elisp: ## Install Elisp bundle ([PROJECT=] [FORCE=1]) install-python: ## Install Python bundle ([PROJECT=] [FORCE=1]) @$(MAKE) install-lang LANG=python PROJECT="$(PROJECT)" FORCE="$(FORCE)" +list-teams: ## List available team overlays + @echo "Available team overlays (teams/):" + @for team in $(TEAMS); do echo " - $$team"; done + +install-team: ## Install a team publishing overlay into one project ([TEAM=] [PROJECT=]) + @$(pick_team_shell); \ + $(pick_project_shell); \ + bash scripts/install-team.sh "$$T" "$$P" + ##@ MCP servers (user scope) install-mcp: ## Decrypt mcp/secrets.env.gpg and register MCP servers at user scope (idempotent) diff --git a/claude-rules/commits.md b/claude-rules/commits.md index aeb25cb..a467d93 100644 --- a/claude-rules/commits.md +++ b/claude-rules/commits.md @@ -187,26 +187,21 @@ Edge case: when one of these files *is* the change (a commit in the rulesets rep Different artifact types carry different content. Don't duplicate. -**Linear ticket bodies:** two sections, in order. +**PR descriptions:** four sections, in order. 1. **Problem** — what's wrong, with enough detail that a teammate can recognize the same failure mode in their own work. -2. **Fix** — what changed (or what's proposed). - -The causal "why" and the test verification belong in the PR, not the -ticket. Linear's GitHub integration auto-cross-links once the PR body -includes the `Linear:` line, so the ticket reader reaches the PR -without needing a body-level link. - -**PR descriptions:** four sections, in order. Same first two as the -ticket, plus: - +2. **Fix** — what changed. 3. **Why this fixes it** — causal link, one or two sentences. 4. **How it was tested** — skip for proposals, specs, or discussions; required for shipped fixes. The PR is the technical artifact. It carries the detail. +If the project's publishing overlay defines a ticket system, see it for +ticket-body conventions (a ticket body is typically just the Problem and +Fix, with the causal why and test verification left to the PR). + **PR review comments** are conversational and don't follow this structure — they follow the Voice and Focus rules above. @@ -318,7 +313,7 @@ Either way the draft runs through `/voice personal` first. The subflows below de **For PR descriptions:** -1. Write the title as line 1 and the body below it to `/tmp/pr-.md`. **Title format:** ` ()` — the ticket ID goes at the end in parentheses (e.g. `refactor: remove dead if-count-is-not-None check in admin (SE-289)`). If there is no ticket, omit the parenthetical. The body must also include a `Linear: []()` line so Linear's GitHub integration auto-cross-links the PR to the ticket. If there is no ticket, state that explicitly ("Linear: n/a") so reviewers know it was considered. +1. Write the title as line 1 and the body below it to `/tmp/pr-.md`. **Title format:** the conventional-commit subject (`refactor: remove dead if-count-is-not-None check in admin`). If the project defines a publishing overlay with a ticket system, follow it for the ticket suffix in the title and the cross-link line in the body (see the overlay). 2. Run `/voice personal` on the file. The PR title stays imperative per Conventional Commits — `/voice personal` rewrites the body, not the title. 3. Print the final draft inline in the terminal. Title on line 1, blank line, then body — exactly as it'll be posted. State that the skill ran. Surface any pattern #39 (public-artifact scope) warnings. 4. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default. @@ -327,16 +322,15 @@ Either way the draft runs through `/voice personal` first. The subflows below de - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-.md`. After the editor closes, re-read the file, re-print inline, ask again. 5. Split the file on the first blank line and pass the title and body to `gh pr create --title "..." --body "$(tail -n +3 )"` (or a heredoc) so formatting is preserved. Add `--reviewer ` in the same call when you already know who should review. 6. Request reviewers on the new PR if you didn't pass `--reviewer` at create time. Use `gh pr edit --add-reviewer `. If the repo has a `CODEOWNERS` file, GitHub auto-suggests based on touched paths. Still issue the explicit request so the reviewer gets notified. Pick reviewers per the team's convention for the area touched (often documented in the per-repo `CLAUDE.md`). For follow-up PRs, consider tagging the parent PR's author if their context would help. PRs without a human reviewer request stall — "checks passed" is not a substitute for review. -7. After `gh pr create` returns a URL, post a comment on the linked Linear ticket with the PR URL (use the Linear MCP `save_comment` tool, or open the ticket manually if MCP is unavailable). This closes the ticket→PR direction of the cross-link. -8. Move the Linear ticket to the "Dev Review" status (use `save_issue` with the Dev Review state ID, or the Linear UI). The ticket should not remain "In Progress" once a PR is open against it. +7. **Project publishing overlay (if present).** If the project defines a publishing overlay — a `publishing-.md` rule loaded from its `.claude/rules/` — run its post-create steps now: ticket cross-linking, ticket-state moves, and any other tracker integration it specifies. A project with no overlay skips this; the PR is already open and reviewers are requested, which is the complete universal flow. **For PR review comments and replies (review verdicts, threaded discussion, follow-up notes on someone else's PR or your own):** Pick the shape first. Most reviews are Shape 1. -- **Shape 1 — Single review** (verdict + summary body + 0+ inline pins). The default for any post that carries a verdict (`APPROVE`, `REQUEST_CHANGES`, `COMMENT`), even when the verdict has no line-specific findings. One `gh api` call posts the summary, every inline pin, and the verdict together. Slack notification fires once for `APPROVE` or `REQUEST_CHANGES`. -- **Shape 2 — Issue-thread comment** (no verdict). General PR discussion, not a review. No inline pins. No Slack notification. -- **Shape 3 — Reply on an existing inline thread**. Responding to a specific prior reviewer comment. Threads under that comment. No Slack notification. +- **Shape 1 — Single review** (verdict + summary body + 0+ inline pins). The default for any post that carries a verdict (`APPROVE`, `REQUEST_CHANGES`, `COMMENT`), even when the verdict has no line-specific findings. One `gh api` call posts the summary, every inline pin, and the verdict together. review notification fires once for `APPROVE` or `REQUEST_CHANGES`. +- **Shape 2 — Issue-thread comment** (no verdict). General PR discussion, not a review. No inline pins. No review notification. +- **Shape 3 — Reply on an existing inline thread**. Responding to a specific prior reviewer comment. Threads under that comment. No review notification. **Inline threshold for Shape 1.** Any finding that names a `path:line` belongs as an inline comment pinned to that line. Cross-cutting observations (verdict rationale, "third PR with the same pattern", overall test-coverage gaps that don't pin to one place) stay in the summary body. There's no "fold one inline into the summary" exception — a single line-specific finding still goes inline. @@ -383,34 +377,11 @@ Pick the shape first. Most reviews are Shape 1. -F "comments[][body]=" ``` - `event` is one of `APPROVE`, `REQUEST_CHANGES`, `COMMENT`. The `comments[]` array can be empty for verdicts with zero line-specific findings — the call still uses the same endpoint. Pass `--hostname` for non-`github.com` hosts (e.g. `deepsat.ghe.com`). + `event` is one of `APPROVE`, `REQUEST_CHANGES`, `COMMENT`. The `comments[]` array can be empty for verdicts with zero line-specific findings — the call still uses the same endpoint. Pass `--hostname` for non-`github.com` hosts (a project's publishing overlay names its host when it's a GitHub Enterprise instance). 7. Verify the review landed. `gh api repos///pulls//reviews --hostname ...` returns the latest review with bundled inlines. Confirm `state` matches the verdict and the inline count matches what was posted. -8. **Notify the PR author on Slack** (`APPROVE` or `REQUEST_CHANGES` verdicts only). After the review posts, send a one-line Slack message to the PR-review group DM `C0B1B0NH2N5` (the mpdm with Craig, Eric, Vrezh, Kostya — *not* `C0AM2MWHCJU`, which is a separate 3-person Craig/Vrezh/Kostya mpdm and is *not* an "#ai" channel despite older notes calling it that) via the `slack-deepsat` MCP. No `/voice personal` pass — the message is short and templated. Two cases: - - Approved: `<@author-slack-id> Approved PR #N.\n` - - Changes requested: `<@author-slack-id> Changes Requested PR #N\n` - Replace `#N` with the PR number and `` with the GHE URL of the PR. Always lead with an `<@author-slack-id>` mention so the engineer who owns the merge decision gets pinged — the message is useless without it. Resolve the Slack user ID via the slack-deepsat `users_search` tool using the PR author's name or email. The URL goes on its own line, immediately after the message. If the MCP doesn't have a post-message tool registered (for example, in a project where the Slack MCP is read-only), surface that to the user and skip the post — don't fall back to a different channel or asking the user to paste it. - - **MCP payload format.** The `mcp__slack-deepsat__conversations_add_message` tool's `payload` parameter is the **raw Slack mrkdwn message body**, not a Slack API JSON envelope. Pass the message body string directly: - - ``` - payload: <@U09AUV4087N> Approved PR #171. - https://deepsat.ghe.com/deepsat/orchestration_dashboard_mvp/pull/171 - ``` - - Wrapping the body in `{"text": "..."}` posts the literal JSON object as the message — the `<@>` mention strips to plain text, escaped `\n` renders as the letter `n`, and the `text:` key prefix appears in the post. The engineer doesn't get pinged. Use real newlines in the string (a literal newline character), not the escape sequence. The MCP has no delete method, so a garbled post can only be removed manually from Slack — surface garble to the user immediately so they can clean it up before reposting. - - Slack mrkdwn that the `payload` string supports: `<@USER_ID>` for user mentions, `<#CHANNEL_ID>` for channel refs, real newlines for line breaks, `*bold*`, `_italic_`, ``\`code\``` and ``` ``` ``` blocks, and `` for labeled links. - - **Thread under the engineer's review-request post.** Before posting, scan the channel's recent history (`mcp__slack-deepsat__conversations_history`) for the engineer's original post containing the PR URL — typically a "New PR" message or just the PR link. Post the verdict notification with `thread_ts=` so it lands in the reply thread off that original. This keeps each PR's review state contained in one thread instead of fragmenting across top-level posts in a mixed feed. The same threading rule applies to both `APPROVE` and `REQUEST_CHANGES` verdicts. - - **No review-request post found?** Don't auto-pick a fallback. Ask the user which they prefer: - - Post the verdict at the top level of the channel. - - DM the engineer directly. - - Don't post at all. - - Skip Slack for `event=COMMENT` reviews (informal feedback, not a verdict) and for Shapes 2 and 3 below. Approve and request-changes are the only verdicts that warrant the notification. +8. **Project review-notification overlay (if present).** If the project defines a publishing overlay with a review-notification step (e.g. a Slack ping to the PR author), run it now — but only for `APPROVE` and `REQUEST_CHANGES` verdicts. The overlay owns the channel, the message format, the author-mention lookup, and the threading. A project with no overlay skips notification entirely. `COMMENT` verdicts and Shapes 2-3 below never notify, overlay or not. **Shape 2: Issue-thread comment (no verdict)** @@ -421,7 +392,7 @@ Use when the post is informal discussion that shouldn't appear as a review verdi 3. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5. 4. Post: `gh pr comment --body-file /tmp/pr--comment.md`. 5. Verify: `gh api repos///issues//comments`. -6. No Slack notification. +6. No review notification. **Shape 3: Reply on an existing inline thread** @@ -433,9 +404,9 @@ Use when responding to a specific prior reviewer comment. 4. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5. 5. Post: `gh api repos///pulls//comments -F in_reply_to= -F body="$(cat /tmp/pr--reply-.md)"`. 6. Verify in the same `comments` list. -7. No Slack notification. +7. No review notification. -**Approve does not authorize a merge.** The team's practice is approve-then-author-merges, not approve-and-merge. The Slack notification in Step 8 hands the merge decision to the PR author. Anything in `## Merge Strategy` below applies only to merges *you* are about to perform on your own branches — and even then, the merge needs its own explicit user confirmation per the rules there. +**Approve does not authorize a merge.** Reviewing a PR never authorizes merging it. Anything in `## Merge Strategy` below applies only to merges *you* are about to perform on your own branches — and even then, the merge needs its own explicit user confirmation per the rules there. A project's publishing overlay may add a team merge practice (e.g. approve-then-author-merges, where the review notification hands the merge decision to the PR author); that's an overlay concern, not a global one. **Exception:** trivial one-liners the user dictated verbatim in the conversation (e.g. "commit this as `chore: bump version`", "reply just diff --git a/scripts/install-team.sh b/scripts/install-team.sh new file mode 100755 index 0000000..0e6e129 --- /dev/null +++ b/scripts/install-team.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +# Install a team publishing overlay into a single target project. +# Usage: install-team.sh +# +# Copies the team overlay's rule file(s) into the project's .claude/rules/. +# A team overlay carries only its own rules — no generic rules, hooks, +# githooks, or settings (those belong to the global install or a language +# bundle). Re-runnable; the authoritative source overwrites. +# +# The overlay is NOT a global rule: it lands in one project's .claude/rules/ +# and loads only there. The companion sync-language-bundle.sh keeps it fresh +# at that project's startup. + +set -euo pipefail + +TEAM="${1:-}" +PROJECT="${2:-}" + +if [ -z "$TEAM" ] || [ -z "$PROJECT" ]; then + echo "Usage: $0 " >&2 + exit 1 +fi + +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +SRC="$REPO_ROOT/teams/$TEAM" + +if [ ! -d "$SRC/claude/rules" ]; then + echo "ERROR: no team overlay rules for '$TEAM' (expected $SRC/claude/rules/)" >&2 + exit 1 +fi + +if [ ! -d "$PROJECT" ]; then + echo "ERROR: project path does not exist: $PROJECT" >&2 + exit 1 +fi +PROJECT="$(cd "$PROJECT" && pwd)" + +echo "Installing team overlay '$TEAM' into $PROJECT" + +mkdir -p "$PROJECT/.claude/rules" +count=0 +for f in "$SRC/claude/rules"/*.md; do + [ -f "$f" ] || continue + cp "$f" "$PROJECT/.claude/rules/$(basename "$f")" + count=$((count + 1)) +done + +echo " [ok] .claude/rules/ — $count overlay rule(s) from teams/$TEAM/" +echo "Loads only in this project. Startup keeps it synced via sync-language-bundle.sh." diff --git a/scripts/sync-language-bundle.sh b/scripts/sync-language-bundle.sh index 8858f74..b2db8cb 100755 --- a/scripts/sync-language-bundle.sh +++ b/scripts/sync-language-bundle.sh @@ -1,19 +1,26 @@ #!/usr/bin/env bash -# Per-project language-bundle freshness check, designed to run at startup. +# Per-project bundle freshness check, designed to run at startup. # -# Detects which language bundle(s) a project has by fingerprint (no marker -# file), then reconciles them against the canonical rulesets source: -# - AUTO-FIX (rulesets-owned, safe to overwrite): .claude/rules/*.md, -# .claude/hooks/*, githooks/* -# - SURFACE (project may customize — reported, never written): -# .claude/settings.json +# Covers two kinds of bundle copied into a project's .claude/: +# - LANGUAGE bundles (languages//) — generic rules + language rules + +# hooks + githooks, with settings.json surfaced (not auto-fixed). +# - TEAM overlays (teams//) — only the overlay's own rule file(s); +# no generic rules, hooks, githooks, or settings. +# +# Detection is by fingerprint (no marker file): a project "has" a bundle iff +# one of that bundle's own rule files is present in the project's +# .claude/rules/. For each detected bundle it reconciles against the canonical +# rulesets source: +# - AUTO-FIX (rulesets-owned, safe to overwrite): .claude/rules/*.md, and +# for language bundles also .claude/hooks/* and githooks/* +# - SURFACE (project may customize — reported, never written): settings.json # CLAUDE.md is intentionally not tracked: it is seed-only in install-lang # and project-owned afterward (diff-lang skips it for the same reason). # # Usage: sync-language-bundle.sh [project-path] (default: $PWD) # # Exit: 0 no bundle, or clean / rules+hooks auto-fixed (resolved) -# 3 manual action recommended (settings.json / CLAUDE.md drift) +# 3 manual action recommended (settings.json drift) # 1 usage / path error # # Quiet when there is nothing to report. Resolves the canonical source @@ -25,6 +32,7 @@ PROJECT="${1:-$PWD}" REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" LANG_ROOT="$REPO_ROOT/languages" +TEAM_ROOT="$REPO_ROOT/teams" GENERIC_RULES="$REPO_ROOT/claude-rules" if [ ! -d "$PROJECT" ]; then @@ -33,8 +41,6 @@ if [ ! -d "$PROJECT" ]; then fi PROJECT="$(cd "$PROJECT" && pwd)" -[ -d "$LANG_ROOT" ] || exit 0 # no bundles available — nothing to do - OUT="" # buffered report, printed once at the end FIXED=0 MANUAL=0 @@ -78,64 +84,81 @@ surface() { MANUAL=$((MANUAL + 1)) } -for src_lang in "$LANG_ROOT"/*/; do - [ -d "$src_lang" ] || continue - src_lang="${src_lang%/}" - lang="$(basename "$src_lang")" - lang_rules="$src_lang/claude/rules" - [ -d "$lang_rules" ] || continue - - # Fingerprint: project has this bundle iff any of the language's own - # rule files is present in .claude/rules/. - detected=0 - for rf in "$lang_rules"/*.md; do +# process_bundle KIND SRC_DIR — reconcile one bundle (KIND is language|team). +# A team overlay owns only its own rule files; a language bundle also owns the +# shared generic rules, its hooks/githooks, and surfaces settings.json. +process_bundle() { + local kind="$1" src="${2%/}" + local name rules rf f rel manual_before + name="$(basename "$src")" + rules="$src/claude/rules" + [ -d "$rules" ] || return 0 + + # Fingerprint: project has this bundle iff one of its own rule files is + # present in .claude/rules/. + local detected=0 + for rf in "$rules"/*.md; do [ -f "$rf" ] || continue if [ -f "$PROJECT/.claude/rules/$(basename "$rf")" ]; then detected=1 break fi done - [ "$detected" -eq 1 ] || continue + [ "$detected" -eq 1 ] || return 0 - CUR_HEADER="language bundle '$lang' — $PROJECT:" + CUR_HEADER="$kind bundle '$name' — $PROJECT:" HEADER_DONE=0 manual_before=$MANUAL - # AUTO-FIX: generic rules (shared) + language rules - for f in "$GENERIC_RULES"/*.md; do - [ -f "$f" ] || continue - fix "$f" "$PROJECT/.claude/rules/$(basename "$f")" - done - for f in "$lang_rules"/*.md; do + # AUTO-FIX: language bundles carry the shared generic rules; team overlays + # carry only their own rule(s). + if [ "$kind" = language ]; then + for f in "$GENERIC_RULES"/*.md; do + [ -f "$f" ] || continue + fix "$f" "$PROJECT/.claude/rules/$(basename "$f")" + done + fi + for f in "$rules"/*.md; do [ -f "$f" ] || continue fix "$f" "$PROJECT/.claude/rules/$(basename "$f")" done - # AUTO-FIX: language hooks (executable) - if [ -d "$src_lang/claude/hooks" ]; then - while IFS= read -r f; do - rel="${f#"$src_lang"/claude/}" - fix "$f" "$PROJECT/.claude/$rel" exec - done < <(find "$src_lang/claude/hooks" -type f) - fi - - # AUTO-FIX: githooks (executable) - if [ -d "$src_lang/githooks" ]; then - while IFS= read -r f; do - rel="${f#"$src_lang"/githooks/}" - fix "$f" "$PROJECT/githooks/$rel" exec - done < <(find "$src_lang/githooks" -type f) + # Language bundles also own hooks, githooks, and settings; teams don't. + if [ "$kind" = language ]; then + if [ -d "$src/claude/hooks" ]; then + while IFS= read -r f; do + rel="${f#"$src"/claude/}" + fix "$f" "$PROJECT/.claude/$rel" exec + done < <(find "$src/claude/hooks" -type f) + fi + if [ -d "$src/githooks" ]; then + while IFS= read -r f; do + rel="${f#"$src"/githooks/}" + fix "$f" "$PROJECT/githooks/$rel" exec + done < <(find "$src/githooks" -type f) + fi + surface "$src/claude/settings.json" "$PROJECT/.claude/settings.json" fi - # SURFACE-ONLY: settings.json may carry project customization, so report - # rather than overwrite. (CLAUDE.md is intentionally untracked here — it's - # seed-only in install-lang and project-owned after, same as diff-lang skips it.) - surface "$src_lang/claude/settings.json" "$PROJECT/.claude/settings.json" - if [ "$MANUAL" -gt "$manual_before" ]; then - OUT+=" → reconcile: make install-$lang PROJECT=."$'\n' + if [ "$kind" = team ]; then + OUT+=" → reconcile: make install-team TEAM=$name PROJECT=."$'\n' + else + OUT+=" → reconcile: make install-$name PROJECT=."$'\n' + fi fi -done +} + +if [ -d "$LANG_ROOT" ]; then + for d in "$LANG_ROOT"/*/; do + [ -d "$d" ] && process_bundle language "$d" + done +fi +if [ -d "$TEAM_ROOT" ]; then + for d in "$TEAM_ROOT"/*/; do + [ -d "$d" ] && process_bundle team "$d" + done +fi [ -n "$OUT" ] && printf '%s' "$OUT" [ "$MANUAL" -gt 0 ] && exit 3 diff --git a/scripts/tests/sync-language-bundle.bats b/scripts/tests/sync-language-bundle.bats index 9fd1108..e641646 100644 --- a/scripts/tests/sync-language-bundle.bats +++ b/scripts/tests/sync-language-bundle.bats @@ -44,6 +44,14 @@ matches_canonical() { # project-relative-path canonical-abs-path diff -q "$PROJ/$1" "$2" >/dev/null 2>&1 } +# Mirror install-team.sh: copy only the team overlay's own rule files into a +# synthetic project (no generic rules, hooks, githooks, or settings). +install_team_overlay() { + local team="$1" proj="$2" + mkdir -p "$proj/.claude/rules" + cp "$REAL_REPO/teams/$team/claude/rules/"*.md "$proj/.claude/rules/" +} + # --- Normal: no bundle / clean --- @test "sync: project with no bundle is a quiet no-op (exit 0)" { @@ -151,3 +159,34 @@ matches_canonical() { # project-relative-path canonical-abs-path [ "$status" -eq 1 ] [[ "$output" == *"does not exist"* ]] } + +# --- Team overlays --- + +@test "sync: clean team overlay is a quiet no-op (exit 0)" { + install_team_overlay deepsat "$PROJ" + run bash "$SCRIPT" "$PROJ" + [ "$status" -eq 0 ] + [ -z "$output" ] +} + +@test "sync: drifted team-overlay rule is auto-fixed and restored" { + install_team_overlay deepsat "$PROJ" + echo "junk drift" >> "$PROJ/.claude/rules/publishing.md" + run bash "$SCRIPT" "$PROJ" + [ "$status" -eq 0 ] + [[ "$output" == *"team bundle 'deepsat'"* ]] + [[ "$output" == *"publishing.md"* ]] + matches_canonical ".claude/rules/publishing.md" "$REAL_REPO/teams/deepsat/claude/rules/publishing.md" +} + +@test "sync: team overlay does NOT pull in generic claude-rules" { + install_team_overlay deepsat "$PROJ" # only publishing.md present, no commits.md + echo "junk drift" >> "$PROJ/.claude/rules/publishing.md" + run bash "$SCRIPT" "$PROJ" + [ "$status" -eq 0 ] + # A team overlay owns only its own rule; the sync must not copy generic + # rules (commits.md, testing.md, ...) into the project the way a language + # bundle does. + [ ! -f "$PROJ/.claude/rules/commits.md" ] + [ ! -f "$PROJ/.claude/rules/testing.md" ] +} diff --git a/teams/deepsat/claude/rules/publishing.md b/teams/deepsat/claude/rules/publishing.md new file mode 100644 index 0000000..c919d3c --- /dev/null +++ b/teams/deepsat/claude/rules/publishing.md @@ -0,0 +1,67 @@ +# DeepSat Publishing Overlay + +Applies to: `**/*` — but only present in the DeepSat work project. This file is **not** a global rule. It is copied into a single project's `.claude/rules/` via `make install-team TEAM=deepsat PROJECT=` and loads only there. Its canonical source lives in `teams/deepsat/claude/rules/publishing.md` in the rulesets repo. + +This overlay extends the publish flow in the global `commits.md` with DeepSat's ticket and chat integration. The global flow defines the universal skeleton (identity, attribution, commit format, the review-and-publish gate, verification); the seams in that flow say "if the project defines a publishing overlay, run its steps here." This is that overlay. A project without it commits, opens PRs, and reviews exactly as the global flow describes — just without the Linear and Slack steps below. + +## Ticket system (Linear) + +DeepSat tracks work in Linear. Tickets carry IDs like `SE-289`. + +**PR title.** Append the ticket ID in parentheses to the conventional-commit subject: `refactor: remove dead if-count check in admin (SE-289)`. If there is no ticket, omit the parenthetical. + +**PR body cross-link.** Include a `Linear: []()` line so Linear's GitHub integration auto-cross-links the PR to the ticket. If there is no ticket, state it explicitly (`Linear: n/a`) so reviewers know it was considered. + +**Linear ticket bodies** (when you write or update a ticket) carry two sections, in order: + +1. **Problem** — what's wrong, with enough detail that a teammate can recognize the same failure mode in their own work. +2. **Fix** — what changed (or what's proposed). + +The causal "why" and the test verification belong in the PR, not the ticket. Linear's GitHub integration auto-cross-links once the PR body includes the `Linear:` line, so the ticket reader reaches the PR without a body-level link. + +## After `gh pr create` (ticket reconciliation) + +Run these where the global PR-description subflow says to run the project's post-create overlay steps: + +1. Post a comment on the linked Linear ticket with the PR URL (Linear MCP `save_comment`, or open the ticket manually if MCP is unavailable). This closes the ticket→PR direction of the cross-link. +2. Move the Linear ticket to the **Dev Review** status (`save_issue` with the Dev Review state ID, or the Linear UI). The ticket should not remain "In Progress" once a PR is open against it. + +## GitHub Enterprise host + +DeepSat's GitHub is `deepsat.ghe.com`, not `github.com`. Pass `--hostname deepsat.ghe.com` to `gh api` calls (the `gh pr` porcelain picks the host up from the remote, but raw `gh api` needs it explicit). + +## Slack review notification + +Run this where the global PR-review Shape 1 flow says to run the project's review-notification overlay step. + +**Notify the PR author on Slack** (`APPROVE` or `REQUEST_CHANGES` verdicts only). After the review posts, send a one-line Slack message to the PR-review group DM `C0B1B0NH2N5` (the mpdm with Craig, Eric, Vrezh, Kostya — *not* `C0AM2MWHCJU`, which is a separate 3-person Craig/Vrezh/Kostya mpdm and is *not* an "#ai" channel despite older notes calling it that) via the `slack-deepsat` MCP. No `/voice personal` pass — the message is short and templated. Two cases: + +- Approved: `<@author-slack-id> Approved PR #N.\n` +- Changes requested: `<@author-slack-id> Changes Requested PR #N\n` + +Replace `#N` with the PR number and `` with the GHE URL of the PR. Always lead with an `<@author-slack-id>` mention so the engineer who owns the merge decision gets pinged — the message is useless without it. Resolve the Slack user ID via the slack-deepsat `users_search` tool using the PR author's name or email. The URL goes on its own line, immediately after the message. If the MCP doesn't have a post-message tool registered (for example, where the Slack MCP is read-only), surface that to the user and skip the post — don't fall back to a different channel or asking the user to paste it. + +**MCP payload format.** The `mcp__slack-deepsat__conversations_add_message` tool's `payload` parameter is the **raw Slack mrkdwn message body**, not a Slack API JSON envelope. Pass the message body string directly: + +``` +payload: <@U09AUV4087N> Approved PR #171. +https://deepsat.ghe.com/deepsat/orchestration_dashboard_mvp/pull/171 +``` + +Wrapping the body in `{"text": "..."}` posts the literal JSON object as the message — the `<@>` mention strips to plain text, escaped `\n` renders as the letter `n`, and the `text:` key prefix appears in the post. The engineer doesn't get pinged. Use real newlines in the string (a literal newline character), not the escape sequence. The MCP has no delete method, so a garbled post can only be removed manually from Slack — surface garble to the user immediately so they can clean it up before reposting. + +Slack mrkdwn that the `payload` string supports: `<@USER_ID>` for user mentions, `<#CHANNEL_ID>` for channel refs, real newlines for line breaks, `*bold*`, `_italic_`, `` `code` `` and ``` ``` ``` blocks, and `` for labeled links. + +**Thread under the engineer's review-request post.** Before posting, scan the channel's recent history (`mcp__slack-deepsat__conversations_history`) for the engineer's original post containing the PR URL — typically a "New PR" message or just the PR link. Post the verdict notification with `thread_ts=` so it lands in the reply thread off that original. This keeps each PR's review state contained in one thread instead of fragmenting across top-level posts in a mixed feed. The same threading rule applies to both `APPROVE` and `REQUEST_CHANGES` verdicts. + +**No review-request post found?** Don't auto-pick a fallback. Ask the user which they prefer: + +- Post the verdict at the top level of the channel. +- DM the engineer directly. +- Don't post at all. + +Skip Slack for `event=COMMENT` reviews (informal feedback, not a verdict) and for issue-thread comments and inline replies. Approve and request-changes are the only verdicts that warrant the notification. + +## Merge practice + +The team's practice is **approve-then-author-merges**, not approve-and-merge. The Slack notification above hands the merge decision to the PR author. Reviewing a PR never authorizes merging it; the global `Merge Strategy` rules apply only to merges you perform on your own branches, and each of those needs its own explicit user confirmation. -- cgit v1.2.3