-
Notifications
You must be signed in to change notification settings - Fork 5
docs(agents): introduce ARCHITECTURE.md + lessons + bash/powershell rules; misc lint and CI fixes #256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
docs(agents): introduce ARCHITECTURE.md + lessons + bash/powershell rules; misc lint and CI fixes #256
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
6f8a90d
fix(sh): install_uv - prefer UV_SYSTEM_CERTS / UV_NATIVE_TLS env vars
szymonos 590d1ed
chore(ci): switch repo_checks runner to ubuntu-slim
szymonos 97f93b4
chore(lint): centralize prek run via PREK_RUN macro; extract shellche…
szymonos 4b99da7
docs(agents): slim AGENTS.md to compound-knowledge index; add ARCHITE…
szymonos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ../AGENTS.md |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| --- | ||
| globs: ["*.sh", "*.bash", "*.bats", "*.zsh"] | ||
| --- | ||
|
|
||
| # Bash style | ||
|
|
||
| - Shebang: `#!/usr/bin/env bash` | ||
| - Indentation: **2 spaces**; line length: **120 chars max** | ||
| - Error handling: `set -euo pipefail` for executable scripts. For files dot-sourced into the user's shell (`.assets/config/bash_cfg/*.sh`, anything installed to `/etc/profile.d/`), drop `-u` - `nounset` breaks shell-init files that source optional vars from the environment. | ||
| - Command substitution: `$(...)`, never backticks | ||
| - Functions: `snake_case`, private: `_prefixed`; prefer `local` for function-scoped variables | ||
| - Variables: `snake_case` locals, `UPPERCASE` constants/env | ||
| - Color output: `\e[31;1m` red/error, `\e[32m` green, `\e[92m` bright green, `\e[96m` cyan/info | ||
|
|
||
| ## Cross-platform regex | ||
|
|
||
| BSD `grep`/`sed` (macOS) and GNU `grep`/`sed` (Linux) disagree on edge cases. Anything sourced from `.assets/config/bash_cfg/` may run on a macOS host during contributor dev loops. | ||
|
|
||
| - Never use empty-alternative regex: `(|foo|bar)` - BSD silently fails to match the empty alternative. Use `(foo|bar)?` instead. | ||
| - Prefer BSD-compatible sed: `sed -En` (not `-rn`), no `\s`/`\w`/`\d` shorthand, no `-P` (perl-regex). | ||
| - `grep -E` (ERE) is portable; `grep -P` (PCRE) is GNU-only. | ||
|
|
||
| See `design/lessons.md` for the BSD-grep incident (commit `78b177d`). | ||
|
|
||
| ## POSIX guard for `/etc/profile.d/` | ||
|
|
||
| Any file installed into `/etc/profile.d/` is sourced by dash (Debian/Ubuntu `/bin/sh`) during login. dash chokes on bash syntax (`function` keyword, arrays, `[[ ]]`). Start the file with: | ||
|
|
||
| ```bash | ||
| [ -n "${BASH_VERSION:-}${ZSH_VERSION:-}" ] || return 0 | ||
| ``` | ||
|
|
||
| Verify with `dash -c '. <file>'`. See `design/lessons.md` (commit `58649ce`). | ||
|
|
||
| ## zsh compatibility | ||
|
|
||
| Files under `.assets/config/bash_cfg/` are sourced from both bash and zsh user shells. The `check-zsh-compat` pre-commit hook parses each one under zsh. Common pitfalls: | ||
|
|
||
| - `[[ $arr[@] ]]` - arrays are 1-indexed in zsh, 0-indexed in bash. | ||
| - Word splitting on unquoted `$var` - zsh doesn't split by default; explicit `${(s: :)var}` or quote consistently. | ||
| - `local` outside a function - bash errors loudly, zsh warns silently. | ||
|
|
||
| ## Common patterns | ||
|
|
||
| ```bash | ||
| # Distro detection | ||
| SYS_ID="$(sed -En '/^ID.*(alpine|arch|fedora|debian|ubuntu|opensuse).*/{s//\1/;p;q}' /etc/os-release)" | ||
|
|
||
| # Root check | ||
| if [ $EUID -ne 0 ]; then | ||
| printf '\e[31;1mRun the script as root.\e[0m\n' >&2 | ||
| exit 1 | ||
| fi | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| --- | ||
| globs: ["*.ps1", "*.psm1", "*.psd1"] | ||
| --- | ||
|
|
||
| # PowerShell style | ||
|
|
||
| ## Formatting | ||
|
|
||
| - Indentation: **4 spaces** | ||
| - Brace style: **OTBS** - opening `{` on same line as statement, closing `}` on its own line, block body always on separate lines | ||
| - For conditional/loop statements with multiple conditions, all conditions and the opening `{` must be on the same line | ||
| - Use ternary `? :` for simple true/false; `switch` with `continue` for range/complex conditions; `if/elseif/else` for distinct paths | ||
|
|
||
| ## Naming | ||
|
|
||
| - Functions: `Verb-Noun` PascalCase (approved verbs only - `Get-Verb` for the list) | ||
| - Parameters: `PascalCase`; local variables: `camelCase` | ||
| - Script-scoped variables: `$script:varName` for state shared across function boundaries (`wsl_setup.ps1` uses `$Script:rel_*` to cache release versions across distro loops) | ||
| - Public functions require comment-based help: `.SYNOPSIS`, `.PARAMETER`, `.EXAMPLE` | ||
|
|
||
| ## Parameters | ||
|
|
||
| - Use `[Parameter(Mandatory, Position = 0)]` for required positional parameters | ||
| - Use `[Alias('s')]` for short parameter aliases (single letter) | ||
| - Use `[ValidateSet(...)]` for enums, `[ValidateNotNullOrEmpty()]` for required strings, `[ValidateScript({ ... })]` for complex validation | ||
| - Switches are never mandatory | ||
| - Use parameter splatting for >3 parameters; cast switches to bool when passing: `-WebDownload ([bool]$WebDownload)` | ||
| - Use `[Parameter(ParameterSetName = 'X')]` with `[CmdletBinding(DefaultParameterSetName = 'Y')]` for mutually exclusive parameter groups | ||
|
|
||
| ## Error handling | ||
|
|
||
| - Set `$ErrorActionPreference = 'Stop'` in `begin` block | ||
| - Use specific exception types in catch blocks before generic catch-all | ||
| - Check `$LASTEXITCODE` immediately after external command invocations (`wsl.exe`, `git`, native binaries) | ||
| - **Guard native binaries that may not exist on the host.** Cross-platform scripts (Windows host + Linux/WSL guest) cannot assume `ssh`, `git`, `gh`, etc. are on PATH. Use `Get-Command <name> -ErrorAction SilentlyContinue` before invoking, and provide a fallback. See `design/lessons.md` (commit `dfb5943`). | ||
|
|
||
| ## Strings | ||
|
|
||
| - Prefer `-f` format operator for simple interpolations: `'{0}: {1}' -f $name, $value` | ||
| - Use `[string]::Join()` for complex multi-part strings (output formatting, shell commands) | ||
| - Never use `+` operator for string concatenation | ||
| - Normalize line endings with `.Replace("\`r\`n", "\`n")` for cross-platform (Windows/Linux) consistency - `wsl.exe` output crosses this boundary | ||
| - ANSI colors via escape sequences: `` `e[32m `` (green), `` `e[31;1m `` (bold red), `` `e[0m `` (reset) | ||
|
|
||
| ## Collections | ||
|
|
||
| - Use `[System.Collections.Generic.List[T]]` for ordered collections that grow | ||
| - Use `[System.Collections.Generic.SortedSet[T]]` for unique items with automatic sorting | ||
| - Use `[System.Collections.Generic.HashSet[T]]` for deduplication and O(1) lookups | ||
| - Never use non-generic `System.Collections` classes | ||
| - Pipe `.Add()` calls to `| Out-Null` to suppress return value | ||
| - Use `.ForEach({ ... })` method for pipeline expressions; `foreach` keyword for iteration with `break`/`continue` | ||
|
|
||
| ## Output and return | ||
|
|
||
| - Use `Write-Host` for colored/formatted console output (bypasses pipeline) | ||
| - Return `$null` explicitly when conditional logic skips operations | ||
| - Use `Write-Output -NoEnumerate` when returning empty arrays to prevent pipeline unwrapping `@()` to `$null` | ||
|
|
||
| ## WSL-specific (wsl/*.ps1) | ||
|
|
||
| - Use direct `wsl.exe @args` with argument splatting for distribution commands | ||
| - Build wsl arguments in `[System.Collections.Generic.List[string]]`, modify in-place for subsequent calls | ||
| - Always set `$psi.UseShellExecute = $false` and `$psi.WorkingDirectory = $PWD.Path` for `Process.Start` | ||
| - Normalize captured `wsl.exe` output with `.Replace("\`r\`n", "\`n")` - the boundary between Windows and Linux line endings is the most common source of bugs in this layer | ||
|
|
||
| ## Module imports | ||
|
|
||
| - Use `Push-Location "$PSScriptRoot/.."` in `begin` block to set directory context | ||
| - Import with `Import-Module (Resolve-Path './modules/<name>') -Force` or `Convert-Path` | ||
| - Re-export public functions from the module manifest (`<module>.psd1` → `FunctionsToExport`) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| --- | ||
| repo: szymonos/linux-setup-scripts | ||
| --- | ||
|
|
||
| # Review brief - linux-setup-scripts | ||
|
|
||
| Curated context for a heterogeneous-model reviewer (`/second-opinion`). | ||
| You are reviewing a code diff. Read this brief first, then the diff, then any files needed for context. | ||
|
|
||
| ## Project | ||
|
|
||
| Automation scripts for provisioning Linux systems, primarily **WSL** (Windows Subsystem for Linux). | ||
|
|
||
| - **Languages:** Bash 5+ (`.sh`), PowerShell 7.4+ (`.ps1`), Python 3 (pre-commit hooks under `tests/hooks/`). | ||
| - **Supported distros:** Fedora/RHEL, Debian/Ubuntu, Arch, OpenSUSE, Alpine. | ||
| - **Two execution contexts:** Windows host (`wsl/*.ps1`) drives WSL guests via `wsl.exe`; guest scripts (`.assets/scripts/`, `.assets/provision/`) run inside the distro. | ||
|
|
||
| See `ARCHITECTURE.md` for the full host/guest split, distro detection pattern, pre-commit hook inventory, test layout, and recipes. | ||
|
|
||
| ## Focus areas (ordered by importance) | ||
|
|
||
| 1. **Correctness** - logic errors, missed edge cases, off-by-one, race conditions, exit-code handling. Distro-detection branches that silently skip a supported distro. | ||
| 2. **Cross-platform shell portability** - the `.assets/config/bash_cfg/*` files are sourced from both bash and zsh user shells. Anything installed to `/etc/profile.d/` is also sourced by dash. Watch for: | ||
| - `function` keyword, bash arrays, `[[ ]]`, `${var,,}` in files that might run under dash. | ||
| - Empty-alternative regex `(|foo|bar)` (BSD grep silently fails to match the empty alternative) - use `(foo|bar)?`. | ||
| - GNU sed/grep extensions: `-P`, `-r` (use `-E`), `\s`, `\w` - macOS dev loops use BSD tools. | ||
| - See `design/lessons.md` for the four documented postmortems (POSIX guard, SSH probe, safe.directory, BSD regex). | ||
| 3. **Cross-host PowerShell guards** - `wsl/*.ps1` runs on Windows hosts where guest binaries (`ssh`, `gh`, etc.) may not exist. Calling them under `$ErrorActionPreference = 'Stop'` without a `Get-Command <name> -ErrorAction SilentlyContinue` guard aborts orchestration. See `design/lessons.md` (commit `dfb5943`). | ||
| 4. **Idempotency** - provisioning scripts are re-run during upgrade. Writes to `~/.gitconfig`, `/etc/profile.d/`, and similar must guard with `grep -qFx`, atomic append, or equivalent. Don't append the same line twice. | ||
| 5. **Error handling** - | ||
| - Bash: missing `set -euo pipefail` on executable scripts (note: files sourced into the user's shell or installed to `/etc/profile.d/` should drop `-u`). | ||
| - PowerShell: missing `$ErrorActionPreference = 'Stop'` in `begin` block; unchecked `$LASTEXITCODE` after `wsl.exe` or native binary invocations. | ||
| 6. **WSL boundary** - `wsl.exe` output crosses Windows/Linux line-ending boundaries. Strings captured from inside WSL should be normalized with `.Replace("\`r\`n", "\`n")`. | ||
|
|
||
| ## Known patterns - do NOT flag | ||
|
|
||
| These are deliberate. Flagging them is noise - they're documented project decisions: | ||
|
|
||
| - **`prek` instead of `pre-commit`** - `prek` is the pre-commit wrapper used in this project. | ||
| - **`--no-verify` in skill instructions** - per-commit hooks are skipped during branch consolidation and validated once via `make lint-diff` after all commits. | ||
| - **`--force-with-lease` in skill instructions** - deliberate; the skill rewrites branch history during consolidation. | ||
| - **`set -eo pipefail` without `-u` in files sourced into user shells** (under `.assets/config/bash_cfg/`, `/etc/profile.d/`) - intentional. `nounset` breaks shell-init files that source optional env vars. | ||
| - **POSIX guard `[ -n "${BASH_VERSION:-}${ZSH_VERSION:-}" ] || return 0` at the top of files installed to `/etc/profile.d/`** - intentional; prevents dash from choking on bash syntax. | ||
| - **BSD-compatible regex** (`sed -En`, no `\s`/`\w`/`-P`) in shell files - intentional for macOS dev-loop compatibility. | ||
| - **`Write-Host` in PowerShell** - intentional; bypasses the pipeline for colored console output. | ||
| - **Symlinked `CLAUDE.md` → `AGENTS.md`** - single source of truth, two lookup paths. | ||
| - **`REVIEW-BRIEF.md` with `repo:` frontmatter tag** - portability mechanism; not stale metadata. | ||
| - **`ubuntu-slim` as a GitHub Actions runner** - valid GitHub-hosted runner, used intentionally. | ||
| - **OpenWolf files under `.wolf/` and `.claude/rules/openwolf.md`** - gitignored personal context management system, not stale dead code. | ||
| - **`disable-model-invocation: true` in some skill frontmatter** - controls whether a skill can be invoked programmatically; deliberate per-skill setting. | ||
|
|
||
| ## Output format | ||
|
|
||
| Produce a single markdown response with this structure: | ||
|
|
||
| ```text | ||
| ## Findings | ||
|
|
||
| ### F-001 - <severity> - <file>:<line> | ||
| <one-paragraph description; reference the constraint being violated; be specific> | ||
|
|
||
| **Suggestion:** <concrete fix direction, NOT a patch> | ||
|
|
||
| ### F-002 - <severity> - <file>:<line> | ||
| ... | ||
| ``` | ||
|
|
||
| Severities: | ||
|
|
||
| - **`bug`** - correctness or security defect; the code is wrong. | ||
| - **`warning`** - likely issue, needs judgment; the code might be wrong under conditions you can't fully verify. | ||
| - **`nit`** - style or clarity; the code works but could be clearer. | ||
|
|
||
| If zero findings, output exactly: `No findings.` | ||
|
|
||
| ## Bias-control rules | ||
|
|
||
| - Speculate carefully. If you suspect a bug but can't verify the call site, mark `warning` not `bug`. | ||
| - Don't pad with `nit` findings to look productive. Five `nit` items on a 200-line diff is fine; thirty is noise. | ||
| - If several findings share the same root cause, consolidate into one finding with multiple `<file>:<line>` references. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.