|
29 | 29 |
|
30 | 30 | - **REQUIRED for staging**: surgical `git add <specific-file> [<file>…]` with explicit paths. Never `-A` / `.`. |
31 | 31 | - **If you need a quick WIP save**: commit on a new branch from inside a worktree, not a stash. |
| 32 | +- **NEVER revert files you didn't touch.** If `git status` shows files you didn't modify, those belong to another session, an upstream pull, or a hook side-effect — leave them alone. Specifically: do not run `git checkout -- <unrelated-path>` to "clean up" the diff before committing, and do not include unrelated paths in `git add`. Stage only the explicit files you edited. |
32 | 33 |
|
33 | 34 | The umbrella rule: never run a git command that mutates state belonging to a path other than the file you just edited. |
34 | 35 |
|
35 | 36 | ## 📚 SHARED STANDARDS |
36 | 37 |
|
37 | 38 | - Commits: [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) `<type>(<scope>): <description>` — NO AI attribution |
| 39 | +- **Open PRs:** when adding commits to an OPEN PR, ALWAYS update the PR title and description to match the new scope. A title like `chore: foo` after you've added security-fix and docs commits to it is now a lie. Use `gh pr edit <num> --title "..." --body "..."` (or `--body-file`) and rewrite the body so it reflects every commit on the branch, grouped by theme. The reviewer should be able to read the PR description and know what's in it without scrolling commits. |
38 | 40 | - Scripts: Prefer `pnpm run foo --flag` over `foo:bar` scripts |
39 | 41 | - Dependencies: After `package.json` edits, run `pnpm install` |
40 | 42 | - Backward Compatibility: 🚨 FORBIDDEN to maintain — actively remove when encountered |
41 | 43 | - 🚨 **NEVER use `npx`, `pnpm dlx`, or `yarn dlx`** — use `pnpm exec <package>` or `pnpm run <script>`. Add tools as pinned devDependencies first. |
42 | 44 | - **minimumReleaseAge**: NEVER add packages to `minimumReleaseAgeExclude` in CI. Locally, ASK before adding — the age threshold is a security control. |
| 45 | +- 🚨 **NEVER mention private repos or internal project names** in commits, PR titles/descriptions/comments, issues, release notes, or any public-surface text. Internal codenames, unreleased product names, internal tooling repo names not on the public org page, customer names, partner names — none belong in public surfaces. **Omit the reference entirely.** Don't substitute a placeholder ("an internal tool", "a downstream consumer", etc.) — the placeholder itself is a tell that something is being elided. Rewrite the sentence to not need the reference at all. |
| 46 | +- 🚨 **NEVER trigger Publish / Release / Provenance / Build-Release workflows** — no `gh workflow run`, `gh workflow dispatch`, or `gh api .../dispatches`. Workflow dispatches are irrevocable: Publish workflows push npm versions (unpublishable after 24h), Build/Release workflows pin GitHub releases by SHA, container workflows push immutable tags. Even build workflows with a `dry_run` input still treat the dispatch itself as the prod trigger. The user runs workflow_dispatch jobs manually after CI passes on the release commit + tag — Claude **never** dispatches them. If the user asks for a publish, tell them to run the command in their own terminal (or the GitHub Actions UI). |
43 | 47 | - File existence: ALWAYS `existsSync` from `node:fs`. NEVER `fs.access`, `fs.stat`-for-existence, or an async `fileExists` wrapper. Import form: `import { existsSync, promises as fs } from 'node:fs'`. |
44 | 48 | - Null-prototype objects: ALWAYS use `{ __proto__: null, ...rest }` for config, return, and internal-state objects. Prevents prototype pollution and accidental inheritance. See `src/socket-sdk-class.ts` and `src/file-upload.ts` for examples. |
45 | 49 | - Linear references: NEVER reference Linear issues (e.g. `SOC-123`, `ENG-456`, Linear URLs) in code, code comments, or PR titles/descriptions/review comments. Keep the codebase and PR history tool-agnostic — tracking lives in Linear. |
46 | 50 |
|
| 51 | +### Sorting |
| 52 | + |
| 53 | +Sort lists alphanumerically (literal byte order, ASCII before letters). Apply this to: |
| 54 | + |
| 55 | +- **Config lists** — `permissions.allow` / `permissions.deny` in `.claude/settings.json`, `external-tools.json` checksum keys, allowlists in workflow YAML. |
| 56 | +- **Object key entries** — sort keys in plain JSON config + return-shape literals + internal-state objects. (Exception: `__proto__: null` always comes first, ahead of any data keys.) |
| 57 | +- **Import specifiers** — sort named imports inside a single statement: `import { encrypt, randomDataKey, wrapKey } from './crypto.mts'`. Imports that say `import type` follow the same rule. Statement _order_ is the project's existing convention (`node:` → external → local → types) — that's separate from specifier order _within_ a statement. |
| 58 | +- **Method / function source placement** — within a module, sort top-level functions alphabetically. Convention: private functions (lowercase / un-exported) sort first, exported functions second. The first-line `export` keyword is the divider. |
| 59 | +- **Array literals** — when the array is a config list, allowlist, or set-like collection. Position-bearing arrays (e.g. argv, anything where index matters semantically) keep their meaningful order. |
| 60 | + |
| 61 | +When in doubt, sort. The cost of a sorted list that didn't need to be is approximately zero; the cost of an unsorted list that did need to be is a merge conflict. |
| 62 | + |
| 63 | +### Paths: One Path, One Reference |
| 64 | + |
| 65 | +**If a path appears in two places, that's a bug.** Every artifact (build output, cache directory, generated file, config location) lives at exactly one canonical location, and that location is defined in exactly one place — typically a `paths.mts` (or equivalent path helper) module. Everything else — other scripts, READMEs, Dockerfiles, workflows, tests — derives from that source. No hand-assembled `path.join(...)` strings outside the module that owns them. |
| 66 | + |
| 67 | +- **Within a package**: every script imports its own path module. No script computes paths from raw segments. |
| 68 | +- **Across packages**: when package B consumes package A's artifact, B imports A's path module (or a typed helper exported from it) — never reconstructs the path from string segments. The classic failure: A adds a new path segment (e.g. inserts a `wasm/` directory), B's hand-built copy of the path drifts, builds break. |
| 69 | +- **Doc strings**: README "Output:" lines and `@fileoverview` comments describe the path; they don't _encode_ it for tools to parse. The doc is for humans only — and even there, it must match what the path module actually produces, verified by running the function. |
| 70 | +- **Workflows / Dockerfiles**: GitHub Actions YAML and Dockerfiles can't `import` TS, so they're allowed to reference the path string directly — but they MUST add a comment pointing at the canonical path module so the next person editing knows where the source of truth lives, and any path string must match the module byte-for-byte. If you find yourself writing the same path twice in one workflow, hoist it to a step output or a job-level env var; reference that everywhere downstream. |
| 71 | +- **Comments that re-state the path**: forbidden. A comment like `// Path mirrors getBuildPaths(): build/<mode>/<arch>/out/Final/...` is duplication wearing a comment costume. The import statement is the comment. |
| 72 | + |
| 73 | +When you spot duplication, the answer is never "update both" — the answer is "delete one and import the other." Fix the architecture, not the symptom. |
| 74 | + |
| 75 | +### Inclusive Language |
| 76 | + |
| 77 | +Use precise, neutral terms over historical metaphors that imply hierarchy or exclusion. The substitutes are not euphemisms — they're more _accurate_ (a list of allowed values genuinely is an "allowlist"; "whitelist" is a metaphor that hides what the list does). |
| 78 | + |
| 79 | +| Replace | With | |
| 80 | +| -------------------------------- | --------------------------------------------------- | |
| 81 | +| `whitelist` / `whitelisted` | `allowlist` / `allowed` / `allowlisted` | |
| 82 | +| `blacklist` / `blacklisted` | `denylist` / `denied` / `blocklisted` / `blocked` | |
| 83 | +| `master` (branch, process, copy) | `main` (branch); `primary` / `controller` (process) | |
| 84 | +| `slave` | `replica`, `worker`, `secondary`, `follower` | |
| 85 | +| `grandfathered` | `legacy`, `pre-existing`, `exempted` | |
| 86 | +| `sanity check` | `quick check`, `confidence check`, `smoke test` | |
| 87 | +| `dummy` (placeholder) | `placeholder`, `stub` | |
| 88 | + |
| 89 | +Apply across **code** (identifiers, comments, string literals), **docs** (READMEs, CLAUDE.md, markdown), **config files** (YAML, JSON), **commit messages**, **PR titles/descriptions**, and **CI logs** you control. |
| 90 | + |
| 91 | +Two exceptions where the legacy term must remain (because changing it breaks something external): |
| 92 | + |
| 93 | +- **Third-party APIs / upstream code**: when interfacing with an external API field literally named `whitelist`, keep the field name; rename your local variable. E.g. `const allowedDomains = response.whitelist`. |
| 94 | +- **Vendored upstream sources**: don't rewrite vendored code (`vendor/**`, `upstream/**`, `**/fixtures/**`). Patch around it if needed. |
| 95 | + |
| 96 | +When you encounter a legacy term during unrelated work, fix it inline — don't defer. |
| 97 | + |
47 | 98 | ### Promise.race in loops |
48 | 99 |
|
49 | 100 | **NEVER re-race the same pool of promises across loop iterations.** Each call to `Promise.race([A, B, ...])` attaches fresh `.then` handlers to every arm; a promise that survives N iterations accumulates N handler sets. See [nodejs/node#17469](https://github.com/nodejs/node/issues/17469) and `@watchable/unpromise`. |
@@ -73,6 +124,27 @@ Emojis allowed sparingly: 📦 💡 🚀 🎉. Prefer text-based symbols for ter |
73 | 124 |
|
74 | 125 | --- |
75 | 126 |
|
| 127 | +### 1 path, 1 reference |
| 128 | + |
| 129 | +**A path is _constructed_ exactly once. Everywhere else _references_ the constructed value.** |
| 130 | + |
| 131 | +Referencing a single computed path many times is fine — that's the whole point of computing it once. What's banned is _re-constructing_ the same path in multiple places, because that's where drift is born. |
| 132 | + |
| 133 | +- **Within a package**: every script imports its own `scripts/paths.mts` (or `lib/paths.mts`). No `path.join('build', mode, ...)` outside that module. |
| 134 | +- **Across packages**: when package B consumes package A's output, B imports A's `paths.mts` via the workspace `exports` field. Never `path.join(PKG, '..', '<sibling>', 'build', ...)`. |
| 135 | +- **Workflows, Dockerfiles, shell scripts**: they can't `import` TS, so they construct the string once and reference it everywhere downstream. Workflows: a "Compute paths" step exposes `steps.paths.outputs.final_dir`; later steps read `${{ steps.paths.outputs.final_dir }}`. Dockerfiles/shell: assign once to a variable / `ENV`, reference by name thereafter. Each canonical construction carries a comment naming the source-of-truth `paths.mts`. **Re-building** the same path in a second step is the violation, not referring to the constructed value many times. |
| 136 | +- **Comments**: may describe path _structure_ with placeholders ("`<mode>/<arch>`") but should not encode a complete literal path string. The import statement IS the comment. |
| 137 | + |
| 138 | +Code execution takes priority over docs: violations in `.mts`/`.cts`, Makefiles, Dockerfiles, workflow YAML, and shell scripts are blocking. README and doc-comment violations are advisory unless they contain a fully-qualified path with no parametric placeholders. |
| 139 | + |
| 140 | +**Three-level enforcement:** |
| 141 | + |
| 142 | +- **Hook** — `.claude/hooks/path-guard/` blocks `Edit`/`Write` calls that would introduce a violation in a `.mts`/`.cts` file at edit time. |
| 143 | +- **Gate** — `scripts/check-paths.mts` runs in `pnpm check`. Fails the build on any violation that isn't allowlisted in `.github/paths-allowlist.yml`. |
| 144 | +- **Skill** — `/path-guard` audits the repo and fixes findings; `/path-guard check` reports only; `/path-guard install` drops the gate + hook + rule into a fresh repo. |
| 145 | + |
| 146 | +The mantra is intentionally short so it sticks: **1 path, 1 reference**. When in doubt, find the canonical owner and import from it. |
| 147 | + |
76 | 148 | ## 🏗️ SDK-SPECIFIC |
77 | 149 |
|
78 | 150 | ### Architecture |
@@ -173,6 +245,7 @@ Configs live in `.config/`: |
173 | 245 | - Type properties: required first, then optional; alphabetical within groups |
174 | 246 | - Class members: 1) private properties, 2) private methods, 3) public methods (alphabetical) |
175 | 247 | - Object properties & destructuring: alphabetical (except semantic ordering) |
| 248 | +- `Set` constructor arguments: `new Set([...])` literals are alphanumeric (runtime is order-insensitive) |
176 | 249 |
|
177 | 250 | ### Testing |
178 | 251 |
|
|
0 commit comments