diff --git a/CHANGELOG.md b/CHANGELOG.md index c7ad81a..3a98a70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.2.1] - 2026-03-17 + +### Removed + +- `--theme` flag and `theme` config key — Ghostty's AppleScript API does not support per-surface themes (error -10006). Set themes globally in `~/.config/ghostty/config` instead. +- `window-save-state` from `doctor` recommendations — Ghostty restores split layouts but not pane commands, creating stale splits that conflict with summon's layout management. + +### Fixed + +- `ENV_KEY_RE` extracted to shared constant in `validation.ts` (was duplicated in index.ts and launcher.ts) +- `summon config` now shows effective defaults when no machine config is set + +### Changed + +- `@internal` JSDoc tags added to all test-only exports for consistency +- `listStarshipPresets()` results are now cached (avoids repeated shell-outs) +- Branch protection on `main` hardened: `enforce_admins` and `dismiss_stale_reviews` enabled + +### Tests + +- 958 total tests (was 955 in v1.2.0) +- 8 weak `.toBeTruthy()` assertions replaced with `.toBeTypeOf('string')` +- 4 new tests covering tree layout + project CWD merge path +- 3 new tests for starship preset caching +- 6 new tests for `ENV_KEY_RE` validation + ## [1.2.0] - 2026-03-16 ### Added @@ -377,7 +403,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - CodeQL security scanning - Dependabot for npm and GitHub Actions -[Unreleased]: https://github.com/juan294/summon/compare/v1.2.0...develop +[Unreleased]: https://github.com/juan294/summon/compare/v1.2.1...develop +[1.2.1]: https://github.com/juan294/summon/compare/v1.2.0...v1.2.1 [1.2.0]: https://github.com/juan294/summon/compare/v1.1.0...v1.2.0 [1.1.0]: https://github.com/juan294/summon/compare/v1.0.0...v1.1.0 [1.0.0]: https://github.com/juan294/summon/compare/v0.8.0...v1.0.0 diff --git a/README.md b/README.md index 32f763b..4f7e70f 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,6 @@ Drop a `.summon` file in your project root to override machine-level config: layout=minimal editor=vim shell=npm run dev -theme=nord env.PORT=3000 ``` @@ -137,7 +136,6 @@ Config resolution order: **CLI flags > .summon > machine config > preset > defau | `--auto-resize` | Resize sidebar to match editor-size (default: on) | | `--no-auto-resize` | Disable auto-resize | | `--starship-preset ` | Starship prompt preset name (per-workspace) | -| `--theme ` | Ghostty theme for workspace | | `--font-size ` | Override font size for workspace panes | | `--env KEY=VALUE` | Set environment variable (repeatable) | | `--on-start ` | Run a command before workspace creation | @@ -161,7 +159,6 @@ Config resolution order: **CLI flags > .summon > machine config > preset > defau | `layout` | | Default layout preset | | `auto-resize` | `true` | Auto-resize sidebar to match editor-size | | `starship-preset` | | Starship prompt theme preset (per-workspace) | -| `theme` | | Ghostty theme for workspace | | `font-size` | | Font size for workspace panes (points) | | `on-start` | | Command to run before workspace creation | | `new-window` | `false` | Open workspace in a new Ghostty window | @@ -176,7 +173,6 @@ summon set editor vim # use vim as the editor summon set shell "npm run dev" # run a command in the shell pane summon set layout minimal # default to minimal preset summon set starship-preset tokyo-night # per-workspace Starship prompt theme -summon set theme nord # Ghostty theme for workspace summon set font-size 14 # font size for workspace panes summon set on-start "npm install" # run before workspace creation summon set new-window true # always open in a new window diff --git a/docs/agents/pre-launch-report.md b/docs/agents/pre-launch-report.md index 0de7b54..d23c7c0 100644 --- a/docs/agents/pre-launch-report.md +++ b/docs/agents/pre-launch-report.md @@ -1,98 +1,95 @@ # Pre-Launch Audit Report -> Generated on 2026-03-16 | Branch: `develop` | 6 parallel specialists | Pre-v1.2.0 +> Generated on 2026-03-17 | Branch: `develop` | 6 parallel specialists | Pre-v1.2.1 ## Verdict: CONDITIONAL -No blockers. 5 warnings across QA, Security, and UX. 27 recommendations. +One expected warning (unpushed commit awaiting release). No blockers. All systems green. -## Warnings +## Blockers (must fix before release) +None. +## Warnings | # | Issue | Severity | Found by | Risk | |---|-------|----------|----------|------| -| W1 | launcher.ts branch coverage at 79.59% (optsToConfigMap untested) | WARNING | qa-lead | Low | -| W2 | Polling-based test sync in setup.test.ts waitForHandler() | WARNING | qa-lead | Low | -| W3 | setTimeout(0) for keypress simulation in setup tests | WARNING | qa-lead | Low | -| W4 | `summon set env.INVALID` silently accepts invalid env var key names | WARNING | ux-reviewer | Medium | -| W5 | Layout name path traversal prevented by regex only | WARNING | security-reviewer | Low | +| W1 | 1 unpushed commit on `develop` (theme removal hotfix) | medium | devops | Must push before release PR | +| W2 | Duplicated env-var-name regex in index.ts:422 vs launcher.ts ENV_KEY_RE | low | architect, performance-eng | Logic divergence risk | +| W3 | Weak `.toBeTruthy()` assertions in setup.test.ts (8 uses) | low | qa-lead | Won't catch type regressions | +| W4 | launcher.ts branch coverage 91.91% — tree layout + project CWD merge path untested | low | qa-lead | Uncovered runtime path | +| W5 | `on-start` uses `execSync` (shell mode) — by-design but worth noting | low | security-reviewer | Same trust model as Makefile | +| W6 | Branch protection `enforce_admins: false` on main | low | devops | Admins can bypass checks | +| W7 | `--fix` and `--vim` missing from CLI_FLAGS array | low | ux-reviewer | Bash completion gap (zsh OK) | +| W8 | Unicode arrow `→` in index.ts output without fallback | low | ux-reviewer | Cosmetic; Ghostty renders fine | + +## Recommendations +| # | Suggestion | Found by | +|---|-----------|----------| +| R1 | Extract ENV_KEY_RE to shared constant in utils.ts or validation.ts | architect, performance-eng | +| R2 | Strengthen `.toBeTruthy()` to `.toBeTypeOf('string')` in setup.test.ts | qa-lead | +| R3 | Add test for tree layout + project CWD merge path (launcher.ts:441-443) | qa-lead | +| R4 | Consider `--yes`/`--no-confirm` flag for CI automation use cases | security-reviewer | +| R5 | Split EDITOR validation on first space to allow `code --wait` | security-reviewer | +| R6 | Document .summon trust model in dedicated README security section | security-reviewer | +| R7 | Mark all test-only exports with consistent `@internal` JSDoc tag | architect | +| R8 | Cache `listStarshipPresets()` result for repeated calls | performance-eng | +| R9 | Enable `enforce_admins` and `dismiss_stale_reviews` on main branch protection | devops | +| R10 | Add `--fix`/`--vim` to CLI_FLAGS for bash completion consistency | ux-reviewer | +| R11 | Show effective defaults in `summon config` when no machine config set | ux-reviewer | ## Detailed Findings ### 1. Quality Assurance (qa-lead) — GREEN -- **930 tests**, 100% pass rate, 12 test files -- **97.26% stmt / 90.29% branch / 98.13% func / 98.19% line** coverage -- All configured thresholds met (95/90/95/95) -- Typecheck: PASS | Lint: PASS - -| # | Finding | Severity | -|---|---------|----------| -| W1 | launcher.ts branch coverage 79.59% — optsToConfigMap 28 uncovered branches | WARNING | -| W2 | waitForHandler() polling without assertion on failure | WARNING | -| W3 | setTimeout(0) keypress simulation pattern | WARNING | -| R1 | Add direct unit tests for optsToConfigMap() | RECOMMENDATION | -| R2 | Add assertion in waitForHandler() if handler never captured | RECOMMENDATION | -| R3 | script.ts line 294 uncovered (3+ right-column editors edge case) | RECOMMENDATION | -| R4 | tree.ts line 124 uncovered (parser advance() defensive error) | RECOMMENDATION | +- **945 tests**, all passing, 0 skipped +- **Typecheck:** clean, **Lint:** clean +- **Coverage:** 98.98% statements, 94.12% branches, 98.60% functions, 99.22% lines +- Critical files all at 98-100% coverage +- Uncovered: setup.ts SIGINT handler (interactive-only), tree.ts defensive throw, launcher.ts tree+CWD merge branch +- 8 weak `.toBeTruthy()` assertions in setup.test.ts ### 2. Security (security-reviewer) — GREEN -- Zero vulnerable dependencies, zero runtime deps -- No hardcoded secrets -- Command injection guarded (execFileSync arrays, SAFE_COMMAND_RE) -- AppleScript injection prevented (escapeAppleScript on all user input) -- File permissions correct (0o600/0o700) - -| # | Finding | Severity | -|---|---------|----------| -| W5 | Layout name path traversal prevented by regex only (no secondary resolve+prefix check) | WARNING | -| R5 | export command writes .summon with 0o644 — appropriate but document distinction | RECOMMENDATION | -| R6 | Env var values not checked by confirmDangerousCommands (properly escaped) | RECOMMENDATION | - -### 3. Infrastructure (devops) — GREEN -- Build succeeds, CI green (4/4 completed runs) -- Node 18/20/22 matrix, CodeQL, dependency review -- Working tree clean, npm pack clean - -| # | Finding | Severity | -|---|---------|----------| -| R7 | Add `main` field to package.json for legacy tooling compat | RECOMMENDATION | -| R8 | Consider `publishConfig: {"access": "public"}` | RECOMMENDATION | +- `pnpm audit`: no known vulnerabilities, zero runtime deps +- No hardcoded secrets found +- osascript execution uses stdin (`execFileSync` with `input`), not shell args — safe +- AppleScript escaping (`escapeAppleScript`, `shellQuote`) correctly implemented +- `confirmDangerousCommands` catches shell metacharacters in `.summon` files +- `SAFE_COMMAND_RE`, `SAFE_SHELL_RE`, `ENV_KEY_RE` properly guard all injection vectors +- `layoutPath()` prevents path traversal with resolve+prefix check +- File permissions: config dir 0o700, files 0o600 — correct +- All dependency licenses permissive (MIT, Apache-2.0, ISC) + +### 3. Infrastructure (devops) — YELLOW +- Build succeeds (14ms, 75KB total output) +- CI: all recent runs green on develop +- CI config: Node [18, 20, 22] matrix, CodeQL, dependency review — solid +- Package metadata: correct (name, version, bin, files, engines, os, license) +- Env vars: full parity between documented (5) and used (5) +- **1 unpushed commit** (theme removal) — expected, will push with release +- Branch protection on main: required checks + 1 review, but `enforce_admins: false` ### 4. Architecture (architect) — GREEN -- Typecheck passes, no circular deps, no dead exports - -| # | Finding | Severity | -|---|---------|----------| -| R9 | Remove trivial resolveCommand wrapper in launcher.ts | RECOMMENDATION | -| R10 | Extract shared isGhosttyInstalled() helper | RECOMMENDATION | -| R11 | Update typescript-eslint 8.57.0 → 8.57.1 | RECOMMENDATION | -| R12 | Consider splitting setup.ts (~1700 lines) | RECOMMENDATION | +- Typecheck: clean +- Dependencies: all up to date +- No circular dependencies — clean DAG +- No dead code (test-only exports properly annotated with `@internal`) +- Module boundaries clean: layout planning → script generation → execution +- Dynamic imports for heavy modules (setup, completions, keybindings) +- 1 duplicated regex (ENV_KEY_RE) — minor DRY violation ### 5. Performance (performance-eng) — GREEN -- Total build: ~75 KB across 12 chunks -- Largest: index.js (35.37 KB) — under 50 KB threshold -- Code splitting effective, no hot-path bloat - -| # | Finding | Severity | -|---|---------|----------| -| R13 | Test-only exports inflate bundle marginally (~200 bytes) | RECOMMENDATION | -| R14 | setup.ts exports ~50 symbols, most test-only | RECOMMENDATION | -| R15 | doctor subcommand could be split out (<2 KB savings) | RECOMMENDATION | -| R16 | Monitor index.js size as features grow | RECOMMENDATION | +- Build: 14ms, 75KB total (12 chunks, code-split) +- Entry chunk: 35KB, setup lazy-loaded (22KB), completions lazy-loaded (6KB) +- All regexes compiled at module level (constants) +- String building uses array push + join — optimal +- Sync file I/O appropriate for one-shot CLI +- `resolveCommand` results cached in launcher +- Starship path cached, config dir ensured once +- No anti-patterns found ### 6. UX/Accessibility (ux-reviewer) — GREEN -- Help text comprehensive, error messages actionable -- NO_COLOR respected, non-TTY handled - -| # | Finding | Severity | -|---|---------|----------| -| W4 | `summon set env.*` accepts invalid env var key names | WARNING | -| R17 | freeze usage `` vs help `` inconsistent | RECOMMENDATION | -| R18 | btop preset description hard-codes "lazygit sidebar" | RECOMMENDATION | -| R19 | layoutNotFoundOrExit lacks `Error:` prefix | RECOMMENDATION | -| R20 | `summon doctor` exits 0 even when issues found | RECOMMENDATION | -| R21 | `summon export` drops env.* keys | RECOMMENDATION | -| R22 | Shell completions don't complete `doctor --fix` / `keybindings --vim` | RECOMMENDATION | -| R23 | `--fix`/`--vim` are global parse options but subcommand-specific | RECOMMENDATION | -| R24 | config unknown key message doesn't suggest removal | RECOMMENDATION | -| R25 | export header lacks timestamp | RECOMMENDATION | -| R26 | Ctrl+C exits with code 0 instead of 130 | RECOMMENDATION | -| R27 | Nested workspace warning "too scary" could be clearer | RECOMMENDATION | +- Help text: all 13 subcommands documented, consistent formatting, practical examples +- Error messages: consistent `Error: ` + actionable guidance pattern +- Exit codes: 0 (success), 1 (error), 2 (doctor issues), 130 (Ctrl+C) — all correct +- Setup wizard: handles no-Ghostty, Ctrl+C, invalid input, non-TTY, decline-and-retry +- NO_COLOR support via `useColor` flag, all ANSI output goes through `wrap()` +- True color detection with graceful fallback +- Screen reader compatible (no emoji-only messages) +- Tab completion: zsh fully working, bash minor gap for subcommand-specific flags diff --git a/docs/agents/remediation-report.md b/docs/agents/remediation-report.md index af5fe3a..2c49ae6 100644 --- a/docs/agents/remediation-report.md +++ b/docs/agents/remediation-report.md @@ -1,73 +1,49 @@ # Remediation Report -> Generated on 2026-03-16 | Branch: `develop` | 24 findings resolved +> Generated on 2026-03-17 | Branch: `develop` | 4 issues resolved > > Pre-launch report: `docs/agents/pre-launch-report.md` ## Summary -- Findings processed: 32 (5 warnings + 27 recommendations) -- Issues created: 7 (#175-#181) -- Issues resolved: 7 -- Tests added: ~25 -- Files modified: 14 -- CI status: PASSING +- Findings processed: 19 (8 warnings + 11 recommendations) +- Issues created: 4 +- Issues resolved: 4 +- Tests added: 13 (6 ENV_KEY_RE + 4 tree+CWD merge + 3 starship caching) +- Files modified: 12 +- CI status: IN PROGRESS (pending) ## Issues Resolved - | # | Issue | Domain | Severity | Tests Added | Status | |---|-------|--------|----------|-------------|--------| -| #175 | launcher/utils/setup refactors | architecture, qa | WARNING | 10 | Closed | -| #176 | setup.test.ts test infrastructure | qa | WARNING | 0 (infra fix) | Closed | -| #177 | index.ts UX fixes | ux | WARNING | 7 | Closed | -| #178 | completions.ts subcommand flags | ux | RECOMMENDATION | 4 | Closed | -| #179 | config.ts path traversal hardening | security | WARNING | 2 | Closed | -| #180 | test coverage gaps (script + tree) | qa | RECOMMENDATION | 2 | Closed | -| #181 | package.json metadata + deps | devops | RECOMMENDATION | 0 (config) | Closed | - -## Findings Addressed (24) - -| Finding | Work Unit | Fix | -|---------|-----------|-----| -| W1 | #175 | optsToConfigMap unit tests added | -| W2 | #176 | waitForHandler assertion on failure | -| W3 | #176 | setTimeout(0) patterns documented | -| W4 | #177 | env var key validation in `summon set` | -| W5 | #179 | layoutPath helper with resolve+prefix check | -| R1 | #175 | optsToConfigMap branch coverage | -| R2 | #176 | waitForHandler throw on timeout | -| R3 | #180 | 4-pane secondary editor test | -| R4 | #180 | parser truncated input test | -| R7 | #181 | package.json main field added | -| R8 | #181 | publishConfig access: public | -| R9 | #175 | Removed trivial resolveCommand wrapper | -| R10 | #175 | Extracted isGhosttyInstalled to utils.ts | -| R11 | #181 | typescript-eslint updated to 8.57.1 | -| R17 | #177 | freeze usage consistency | -| R18 | #177 | btop description fixed | -| R19 | #177 | layoutNotFoundOrExit Error: prefix | -| R20 | #177 | doctor exit code 2 on issues | -| R21 | #177 | export includes env.* keys | -| R22 | #178 | doctor --fix / keybindings --vim completions | -| R24 | #177 | config unknown key removal hint | -| R25 | #177 | export header timestamp | -| R26 | #175 | Ctrl+C exit code 130 | -| R27 | #175 | Nested warning "messy" wording | - -## Informational (no code change needed) -- R5: export 0o644 permissions appropriate for project files -- R6: env var values properly escaped, not exploitable - -## Deferred Items -- R12: Split setup.ts (~1700 lines) — major refactor, separate task -- R13/R14: Test-only exports (~200 bytes) — cosmetic -- R15: Doctor subcommand split — marginal savings (<2 KB) -- R16: Monitor index.js size — informational -- R23: --fix/--vim global parse scope — parseArgs architectural limitation +| #184 | Extract ENV_KEY_RE + show config defaults | architecture, ux | low | 6 | Closed | +| #185 | Strengthen test assertions + cover tree+CWD merge | qa | low | 4 | Closed | +| #186 | @internal JSDoc tags + cache listStarshipPresets | architecture, performance | low | 3 | Closed | +| #187 | Harden branch protection on main | devops | low | 0 (config) | Closed | + +## Findings Disposition +| Finding | Action | Details | +|---------|--------|---------| +| W1 | Resolved | Pushed with release | +| W2/R1 | Fixed (#184) | ENV_KEY_RE extracted to validation.ts | +| W3/R2 | Fixed (#185) | 8x .toBeTruthy() → .toBeTypeOf('string') | +| W4/R3 | Fixed (#185) | 4 tests for tree+CWD merge path | +| W5 | Skipped | By design (on-start uses shell intentionally) | +| W6/R9 | Fixed (#187) | enforce_admins + dismiss_stale_reviews enabled | +| W7/R10 | Skipped | False positive (adding to CLI_FLAGS breaks completions; subcommand-specific handling is correct) | +| W8 | Skipped | Cosmetic (Ghostty renders Unicode fine) | +| R4 | Skipped | Feature request (--yes flag for CI) — future release | +| R5 | Skipped | Security design decision (strict EDITOR validation) | +| R6 | Skipped | Already implemented (README Trust Model section exists) | +| R7 | Fixed (#186) | @internal tags added to GHOSTTY_PATHS, layoutPath | +| R8 | Fixed (#186) | listStarshipPresets() now cached | +| R11 | Fixed (#184) | `summon config` shows effective defaults | ## Final Verification -- [x] All 955 tests passing +- [x] All tests passing (958) - [x] Typecheck clean - [x] Lint clean -- [x] Build succeeds (75 KB total) -- [x] CI green -- [x] All worktrees removed -- [x] All issues closed +- [x] Build succeeds (75KB, 14ms) +- [ ] CI green (pending) +- [x] /simplify final pass complete + +## Remaining Items +None — all actionable findings resolved. Skipped items are by-design, false positives, or future feature requests. diff --git a/docs/agents/update-docs-report.md b/docs/agents/update-docs-report.md index 3ebc5f9..8f67a76 100644 --- a/docs/agents/update-docs-report.md +++ b/docs/agents/update-docs-report.md @@ -1,21 +1,37 @@ # Documentation Update Report -> Generated on 2026-03-16 | Branch: `develop` | Changes since v1.1.0 + +> Generated on 2026-03-17 | Branch: develop | Changes since v1.2.0 ## Summary -- 8 documents updated -- 2 diagrams refreshed (dependency graph + data flow) -- 3 version references corrected + +- 2 documents updated +- 2 diagrams refreshed +- 0 version references corrected (handled during /release) +- 0 inline doc blocks updated - 0 items flagged [NEEDS REVIEW] ## Changes by File -| File | Changes | -|------|---------| -| `CHANGELOG.md` | Added [Unreleased] section with all v1.2.0 features, fixes, refactors, infra, tests | -| `README.md` | Added freeze, keybindings, doctor --fix to commands table; added --theme flag; added theme config key; added theme to .summon example | -| `docs/user-manual.md` | Added doctor --fix, freeze, keybindings subcommand docs; added --theme flag; added theme to config reference; added per-pane cwd section | -| `docs/architecture.md` | Added keybindings.ts to module table + dependency graph; added freeze/keybindings to data flow; updated export annotations (extractPaneCwds, layoutPath, isGhosttyInstalled, checkAccessibility); added --theme to parseArgs | -| `CLAUDE.md` | Added keybindings.ts to project structure; updated utils.ts description | -| `SECURITY.md` | Updated supported versions: < 1.0 → No | -| `.github/ISSUE_TEMPLATE/bug_report.yml` | Version placeholder 1.0.0 → 1.1.0 | -| `docs/publishing.md` | Updated version strategy from pre-1.0 language to post-1.0 | +### docs/architecture.md + +1. **Module Map table (line 20)**: Updated `validation.ts` description to include `ENV_KEY_RE` and `parsePositiveFloat`, and corrected dependency from "none" to "utils". +2. **Dependency Graph annotation box (lines 104-107)**: Added `ENV_KEY_RE` to the validation.ts export list. +3. **Data Flow diagram (line 135)**: Removed `--theme` from the parseArgs flags list (flag was removed from the codebase). + +### MANUAL_TESTBED.md + +1. **Section AC (Ghostty Theme)**: Replaced full test section with a "REMOVED in v1.2.1" notice explaining why (Ghostty AppleScript API doesn't support per-surface themes) and workaround (global config). +2. **Section T (Doctor)**: Removed `window-save-state` from the recommended settings list (doctor no longer checks it). +3. **Section AD (Doctor --fix)**: Updated test instructions to use `notify-on-command-finish` instead of `window-save-state` for the missing-setting test scenarios. Updated expected assertion text. + +## Already Up to Date + +- README.md -- theme references already removed +- docs/user-manual.md -- theme and window-save-state already removed +- CHANGELOG.md -- [Unreleased] correctly documents theme removal +- CLAUDE.md -- accurate project structure +- CONTRIBUTING.md, SECURITY.md, publishing.md -- all current + +## Flagged for Review + +None. diff --git a/docs/architecture.md b/docs/architecture.md index 6c85383..a3a0ab3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -17,7 +17,7 @@ Technical reference for contributors. | `starship.ts` | Starship detection, preset listing, TOML config caching | yes | config, utils | | `tree.ts` | Tree data model, DSL parser, plan builder (pure — no side effects) | **pure** | layout | | `keybindings.ts` | Ghostty key table config generator (pure function) | **pure** | none | -| `validation.ts` | Input validation helpers (`parseIntInRange`, `validateIntFlag`, `validateFloatFlag`) | **pure** | none | +| `validation.ts` | Input validation helpers (`ENV_KEY_RE`, `parseIntInRange`, `parsePositiveFloat`, `validateIntFlag`, `validateFloatFlag`) | **pure** | utils | | `globals.d.ts` | Build-time constant declarations (`__VERSION__`) | — | — | | `*.test.ts` | Co-located unit tests (Vitest) | — | — | @@ -101,7 +101,8 @@ graph TD EDITOR_CATALOG, SIDEBAR_CATALOG"] completions -.- comp_fns["generateZshCompletion, generateBashCompletion"] - validation -.- val_fns["parseIntInRange, + validation -.- val_fns["ENV_KEY_RE, + parseIntInRange, parsePositiveFloat, validateIntFlag, validateFloatFlag"] @@ -132,7 +133,7 @@ flowchart TD --editor, --panes, --editor-size, --sidebar, --shell, --starship-preset, --auto-resize, --no-auto-resize, --dry-run, - --env, --font-size, --on-start, --theme, + --env, --font-size, --on-start, --new-window, --fullscreen, --maximize, --float"] parse --> helpcheck{"--help flag?"} diff --git a/docs/user-manual.md b/docs/user-manual.md index b1d204f..244cfe0 100644 --- a/docs/user-manual.md +++ b/docs/user-manual.md @@ -204,7 +204,6 @@ summon export .summon # write to file Check your Ghostty config for recommended settings and macOS Accessibility permission. Reports on: -- `window-save-state = always` — restore windows after restart - `notify-on-command-finish = unfocused` — notifications for long commands - `shell-integration = detect` — shell integration for status tracking - Accessibility permission — required for pane resizing via System Events @@ -282,7 +281,6 @@ Flags override both machine and per-project config for a single launch. | `--auto-resize` | Resize sidebar to match editor-size (default: on) | | `--no-auto-resize` | Disable auto-resize | | `--starship-preset ` | Starship prompt preset name (per-workspace) | -| `--theme ` | Ghostty theme for workspace | | `--font-size ` | Override font size for workspace panes | | `--env KEY=VALUE` | Set environment variable for all panes (repeatable) | | `--on-start ` | Run a command in the target directory before workspace creation | @@ -739,7 +737,6 @@ When summon launches, config values are resolved in this order (first wins): | `layout` | string | | Default layout preset (`minimal`, `full`, `pair`, `cli`, `btop`) or a custom layout name. | | `auto-resize` | boolean | `true` | Auto-resize sidebar to match editor-size. | | `starship-preset` | string | | Starship prompt theme preset. When set, each workspace launches with `STARSHIP_CONFIG` pointing to a cached preset TOML file at `~/.config/summon/starship/.toml`. Requires [Starship](https://starship.rs) installed. | -| `theme` | string | | Ghostty theme for workspace. | | `font-size` | number | | Font size for workspace panes (in points). | | `on-start` | string | | Command to run in the target directory before workspace creation. | | `new-window` | boolean | `false` | Open workspace in a new Ghostty window instead of reusing the front window. | diff --git a/package.json b/package.json index 6b6b1e5..c22a672 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "summon-ws", - "version": "1.2.0", + "version": "1.2.1", "description": "Launch configurable multi-pane Ghostty workspaces with one command", "type": "module", "packageManager": "pnpm@10.29.2", diff --git a/src/completions.ts b/src/completions.ts index 69240d7..c6da3e3 100644 --- a/src/completions.ts +++ b/src/completions.ts @@ -51,7 +51,6 @@ _summon() { '--starship-preset[Starship preset]:preset:->starship_preset' \\ '*--env[Set environment variable]:var:' \\ '--font-size[Font size in points]:size:' \\ - '--theme[Ghostty theme]:theme:' \\ '--on-start[Run command before workspace creation]:command:' \\ '--new-window[Open in new Ghostty window]' \\ '--fullscreen[Start in fullscreen mode]' \\ diff --git a/src/config.ts b/src/config.ts index f67d776..1430c81 100644 --- a/src/config.ts +++ b/src/config.ts @@ -115,7 +115,7 @@ export function listConfig(): Map { return readKV(CONFIG_FILE); } -export const VALID_KEYS = ["editor", "sidebar", "panes", "editor-size", "shell", "layout", "auto-resize", "starship-preset", "new-window", "fullscreen", "maximize", "float", "font-size", "theme", "on-start"]; +export const VALID_KEYS = ["editor", "sidebar", "panes", "editor-size", "shell", "layout", "auto-resize", "starship-preset", "new-window", "fullscreen", "maximize", "float", "font-size", "on-start"]; /** Config keys that accept only "true" or "false" values. */ export const BOOLEAN_KEYS = new Set(["auto-resize", "new-window", "fullscreen", "maximize", "float"]); @@ -124,7 +124,7 @@ export const CLI_FLAGS = [ "--help", "--version", "--layout", "--editor", "--panes", "--editor-size", "--sidebar", "--shell", "--auto-resize", "--no-auto-resize", "--starship-preset", "--dry-run", - "--env", "--new-window", "--fullscreen", "--maximize", "--float", "--font-size", "--theme", "--on-start", + "--env", "--new-window", "--fullscreen", "--maximize", "--float", "--font-size", "--on-start", "-h", "-v", "-l", "-e", "-p", "-s", "-n", ]; @@ -136,7 +136,7 @@ export function isValidLayoutName(name: string): boolean { return LAYOUT_NAME_RE.test(name) && !isPresetName(name); } -/** Defense-in-depth: verify resolved path stays within LAYOUTS_DIR. */ +/** @internal — exported for testing only */ export function layoutPath(name: string): string { const filePath = join(LAYOUTS_DIR, name); if (!resolve(filePath).startsWith(resolve(LAYOUTS_DIR))) { diff --git a/src/index.test.ts b/src/index.test.ts index 99275dc..728bc2e 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -825,12 +825,11 @@ describe("CLI integration", () => { const result = run("doctor", "--fix"); // Exit 0 if all issues fixed, or 2 if accessibility still missing expect(result.status === 0 || result.status === 2).toBe(true); - expect(result.stdout).toContain("Added 3 setting(s)"); + expect(result.stdout).toContain("Added 2 setting(s)"); // Verify the config was created const ghosttyConfig = join(TEMP_HOME, ".config", "ghostty", "config"); expect(existsSync(ghosttyConfig)).toBe(true); const content = readFileSync(ghosttyConfig, "utf-8"); - expect(content).toContain("window-save-state = always"); expect(content).toContain("notify-on-command-finish = unfocused"); expect(content).toContain("shell-integration = detect"); expect(content).toContain("# Added by summon doctor --fix"); @@ -841,20 +840,19 @@ describe("CLI integration", () => { mkdirSync(ghosttyDir, { recursive: true }); const ghosttyConfig = join(ghosttyDir, "config"); // Write config with one setting already present - writeFileSync(ghosttyConfig, "window-save-state = always\n"); + writeFileSync(ghosttyConfig, "notify-on-command-finish = unfocused\n"); const result = run("doctor", "--fix"); // Exit 0 if all issues fixed, or 2 if accessibility still missing expect(result.status === 0 || result.status === 2).toBe(true); - // Should only add the 2 missing settings - expect(result.stdout).toContain("Added 2 setting(s)"); + // Should only add the 1 missing setting + expect(result.stdout).toContain("Added 1 setting(s)"); expect(result.stdout).toContain("Backed up"); // Verify backup was created (filename includes timestamp) const backups = readdirSync(ghosttyDir).filter(f => f.startsWith("config.bak")); expect(backups.length).toBeGreaterThanOrEqual(1); - // Verify original setting is still present and new ones added + // Verify original setting is still present and new one added const content = readFileSync(ghosttyConfig, "utf-8"); - expect(content).toContain("window-save-state = always"); expect(content).toContain("notify-on-command-finish = unfocused"); expect(content).toContain("shell-integration = detect"); }); @@ -863,7 +861,7 @@ describe("CLI integration", () => { const ghosttyDir = join(TEMP_HOME, ".config", "ghostty"); mkdirSync(ghosttyDir, { recursive: true }); const ghosttyConfig = join(ghosttyDir, "config"); - writeFileSync(ghosttyConfig, "window-save-state = always\nnotify-on-command-finish = unfocused\nshell-integration = detect\n"); + writeFileSync(ghosttyConfig, "notify-on-command-finish = unfocused\nshell-integration = detect\n"); const result = run("doctor", "--fix"); // Exit 0 if accessibility granted, or 2 if not diff --git a/src/index.ts b/src/index.ts index 328b360..7b3e9b8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,8 +22,8 @@ import { } from "./config.js"; import { launch, resolveConfig, optsToConfigMap } from "./launcher.js"; import type { CLIOverrides } from "./launcher.js"; -import { PANES_MIN, EDITOR_SIZE_MIN, EDITOR_SIZE_MAX, isPresetName, getPresetNames } from "./layout.js"; -import { validateIntFlag, validateFloatFlag } from "./validation.js"; +import { PANES_MIN, PANES_DEFAULT, EDITOR_SIZE_MIN, EDITOR_SIZE_MAX, EDITOR_SIZE_DEFAULT, isPresetName, getPresetNames } from "./layout.js"; +import { validateIntFlag, validateFloatFlag, ENV_KEY_RE } from "./validation.js"; import { SAFE_COMMAND_RE, getErrorMessage, exitWithUsageHint, checkAccessibility, ACCESSIBILITY_SETTINGS_PATH, ACCESSIBILITY_ENABLE_HINT } from "./utils.js"; function validateLayoutNameOrExit(name: string): void { @@ -91,7 +91,6 @@ Options: --starship-preset Starship prompt preset name (per-workspace) --env Set environment variable (repeatable) --font-size Override font size for workspace panes - --theme Ghostty theme for workspace --on-start Run command before workspace creation --new-window Open workspace in a new Ghostty window --fullscreen Start workspace in fullscreen mode @@ -113,7 +112,6 @@ Config keys: maximize Start workspace maximized (default: false) float Float workspace window on top (default: false) font-size Font size in points for workspace panes - theme Ghostty theme for workspace on-start Command to run before workspace launches env. Environment variable passed to all panes @@ -257,7 +255,6 @@ const parseOpts = { "starship-preset": { type: "string" }, "env": { type: "string", multiple: true }, "font-size": { type: "string" }, - "theme": { type: "string" }, "on-start": { type: "string" }, "new-window": { type: "boolean" }, "fullscreen": { type: "boolean" }, @@ -358,7 +355,6 @@ function buildOverrides(): CLIOverrides { if (values["starship-preset"]) overrides["starship-preset"] = values["starship-preset"]; if (values.env) overrides.env = values.env; if (values["font-size"]) overrides["font-size"] = values["font-size"]; - if (values.theme) overrides.theme = values.theme; if (values["on-start"]) overrides["on-start"] = values["on-start"]; if (values["new-window"]) overrides["new-window"] = "true"; if (values["fullscreen"]) overrides["fullscreen"] = "true"; @@ -423,7 +419,7 @@ switch (subcommand) { } if (key.startsWith("env.")) { const envName = key.slice(4); - if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(envName)) { + if (!ENV_KEY_RE.test(envName)) { console.error(`Error: invalid environment variable name "${envName}".`); console.error("Environment variable names must start with a letter or underscore and contain only letters, digits, and underscores."); process.exit(1); @@ -471,7 +467,17 @@ switch (subcommand) { case "config": { const config = listConfig(); if (config.size === 0) { - console.log("No machine config set. Use: summon set "); + console.log("No machine config set. Effective defaults:"); + console.log(` panes → ${PANES_DEFAULT}`); + console.log(` editor-size → ${EDITOR_SIZE_DEFAULT}`); + console.log(` sidebar → lazygit`); + console.log(` shell → true`); + console.log(` auto-resize → true`); + console.log(` new-window → false`); + console.log(` fullscreen → false`); + console.log(` maximize → false`); + console.log(` float → false`); + console.log("\nCustomize with: summon set "); } else { console.log("Machine config:"); for (const [key, value] of config) { @@ -536,13 +542,6 @@ switch (subcommand) { : ""; const checks = [ - { - name: "Session Persistence", - key: "window-save-state", - recommended: "always", - reason: "Restore your workspace layout after Ghostty restarts", - regex: /^\s*window-save-state\s*=/m, - }, { name: "Command Notifications", key: "notify-on-command-finish", diff --git a/src/launcher.test.ts b/src/launcher.test.ts index 70f03ab..f10e7f6 100644 --- a/src/launcher.test.ts +++ b/src/launcher.test.ts @@ -2711,7 +2711,6 @@ describe("optsToConfigMap", () => { shell: "zsh", autoResize: true, fontSize: 14, - theme: "tokyo-night", newWindow: true, fullscreen: true, maximize: true, @@ -2724,7 +2723,6 @@ describe("optsToConfigMap", () => { expect(result.get("shell")).toBe("zsh"); expect(result.get("auto-resize")).toBe("true"); expect(result.get("font-size")).toBe("14"); - expect(result.get("theme")).toBe("tokyo-night"); expect(result.get("new-window")).toBe("true"); expect(result.get("fullscreen")).toBe("true"); expect(result.get("maximize")).toBe("true"); @@ -2736,11 +2734,6 @@ describe("optsToConfigMap", () => { expect(result.has("font-size")).toBe(false); }); - it("excludes null theme", () => { - const result = optsToConfigMap({ theme: null }); - expect(result.has("theme")).toBe(false); - }); - it("excludes boolean flags when false", () => { const result = optsToConfigMap({ newWindow: false, @@ -2759,8 +2752,106 @@ describe("optsToConfigMap", () => { expect(result.get("font-size")).toBe("18.5"); }); - it("includes theme when set to a string", () => { - const result = optsToConfigMap({ theme: "dracula" }); - expect(result.get("theme")).toBe("dracula"); +}); + +describe("tree layout + project CWD merge (#185)", () => { + it("merges per-pane cwds from project .summon file into tree layout", () => { + // Custom layout defines a tree with pane CWDs in the layout itself + mockIsCustomLayout.mockReturnValue(true); + mockReadCustomLayout.mockReturnValue( + new Map([ + ["tree", "editor | sidebar"], + ["pane.editor", "vim"], + ["pane.sidebar", "lazygit"], + ["pane.editor.cwd", "src"], + ]), + ); + // Project .summon overrides sidebar CWD + mockReadKVFile.mockReturnValue( + new Map([ + ["layout", "mywork"], + ["pane.sidebar.cwd", "tests"], + ]), + ); + vi.mocked(listConfig).mockReturnValue(new Map([["editor", "vim"]])); + + const result = resolveConfig("/tmp/workspace", {}); + + expect(result.treeLayout).toBeDefined(); + expect(result.treeLayout!.paneCwds).toBeDefined(); + // Layout-defined CWD preserved + expect(result.treeLayout!.paneCwds!.get("editor")).toBe("src"); + // Project CWD merged in + expect(result.treeLayout!.paneCwds!.get("sidebar")).toBe("tests"); + }); + + it("project pane CWD overrides layout-defined CWD for the same pane", () => { + mockIsCustomLayout.mockReturnValue(true); + mockReadCustomLayout.mockReturnValue( + new Map([ + ["tree", "editor | sidebar"], + ["pane.editor", "vim"], + ["pane.sidebar", "lazygit"], + ["pane.editor.cwd", "src"], + ]), + ); + // Project overrides editor CWD + mockReadKVFile.mockReturnValue( + new Map([ + ["layout", "mywork"], + ["pane.editor.cwd", "lib"], + ]), + ); + vi.mocked(listConfig).mockReturnValue(new Map([["editor", "vim"]])); + + const result = resolveConfig("/tmp/workspace", {}); + + expect(result.treeLayout).toBeDefined(); + expect(result.treeLayout!.paneCwds).toBeDefined(); + // Project CWD overrides the layout's CWD + expect(result.treeLayout!.paneCwds!.get("editor")).toBe("lib"); + }); + + it("returns no paneCwds when neither layout nor project defines them", () => { + mockIsCustomLayout.mockReturnValue(true); + mockReadCustomLayout.mockReturnValue( + new Map([ + ["tree", "editor | sidebar"], + ["pane.editor", "vim"], + ["pane.sidebar", "lazygit"], + ]), + ); + mockReadKVFile.mockReturnValue(new Map([["layout", "mywork"]])); + vi.mocked(listConfig).mockReturnValue(new Map([["editor", "vim"]])); + + const result = resolveConfig("/tmp/workspace", {}); + + expect(result.treeLayout).toBeDefined(); + expect(result.treeLayout!.paneCwds).toBeUndefined(); + }); + + it("adds project paneCwds to tree layout that has no layout-defined CWDs", () => { + mockIsCustomLayout.mockReturnValue(true); + mockReadCustomLayout.mockReturnValue( + new Map([ + ["tree", "editor | sidebar"], + ["pane.editor", "vim"], + ["pane.sidebar", "lazygit"], + ]), + ); + // Project defines CWDs even though layout has none + mockReadKVFile.mockReturnValue( + new Map([ + ["layout", "mywork"], + ["pane.editor.cwd", "app"], + ]), + ); + vi.mocked(listConfig).mockReturnValue(new Map([["editor", "vim"]])); + + const result = resolveConfig("/tmp/workspace", {}); + + expect(result.treeLayout).toBeDefined(); + expect(result.treeLayout!.paneCwds).toBeDefined(); + expect(result.treeLayout!.paneCwds!.get("editor")).toBe("app"); }); }); diff --git a/src/launcher.ts b/src/launcher.ts index f3ca339..2b25df6 100644 --- a/src/launcher.ts +++ b/src/launcher.ts @@ -18,7 +18,7 @@ import { generateAppleScript, generateTreeAppleScript } from "./script.js"; import { parseTreeDSL, extractPaneDefinitions, extractPaneCwds, resolveTreeCommands as resolveTreeCmds, buildTreePlan, findPaneByName } from "./tree.js"; import type { LayoutNode } from "./tree.js"; import { resolveCommand as resolveCommandPath, promptUser, getErrorMessage, SUMMON_WORKSPACE_ENV, isAccessibilityError, isGhosttyInstalled, ACCESSIBILITY_SETTINGS_PATH, ACCESSIBILITY_ENABLE_HINT } from "./utils.js"; -import { parseIntInRange, parsePositiveFloat } from "./validation.js"; +import { parseIntInRange, parsePositiveFloat, ENV_KEY_RE } from "./validation.js"; import { isStarshipInstalled, ensurePresetConfig, getPresetConfigPath } from "./starship.js"; const SAFE_SHELL_RE = /^\/[a-zA-Z0-9_/.-]+$/; @@ -54,7 +54,6 @@ export function optsToConfigMap(opts: Partial): Map, projectConfig: Map, @@ -321,7 +316,6 @@ function resolveLayoutBase( const fs = parsePositiveFloat(customData.get("font-size")!); if (fs.ok) base.fontSize = fs.value; } - if (customData.has("theme")) base.theme = customData.get("theme")!; if (customData.has("new-window")) base.newWindow = customData.get("new-window") === "true"; if (customData.has("fullscreen")) base.fullscreen = customData.get("fullscreen") === "true"; if (customData.has("maximize")) base.maximize = customData.get("maximize") === "true"; @@ -408,9 +402,6 @@ function layerConfigValues( } } - const theme = pick(cliOverrides.theme, "theme"); - if (theme !== undefined) result.theme = theme; - const newWindow = pick(cliOverrides["new-window"], "new-window"); const fullscreen = pick(cliOverrides.fullscreen, "fullscreen"); const maximize = pick(cliOverrides.maximize, "maximize"); @@ -482,19 +473,15 @@ function executeOnStart(onStart: string, targetDir: string): void { } } -/** Append shared optional dry-run header lines (starship preset, theme). */ +/** Append shared optional dry-run header lines (starship preset). */ function appendDryRunExtras( headerLines: string[], starshipPreset: string | undefined, dryRunStarshipPath: string | null, - theme: string | null | undefined, ): void { if (starshipPreset) { headerLines.push(`-- Starship preset: ${starshipPreset} (${dryRunStarshipPath})`); } - if (theme) { - headerLines.push(`-- Theme: ${theme}`); - } } async function launchTreeLayout( @@ -513,7 +500,6 @@ async function launchTreeLayout( autoResize: opts.autoResize, editorSize: opts.editorSize, fontSize: opts.fontSize, - theme: opts.theme, newWindow: opts.newWindow, fullscreen: opts.fullscreen, maximize: opts.maximize, @@ -531,7 +517,7 @@ async function launchTreeLayout( `-- Layout: tree layout, ${paneCount} ${paneCount === 1 ? "pane" : "panes"}`, `-- Target: ${targetDir}`, ]; - appendDryRunExtras(headerLines, starshipPreset, dryRunStarshipPath, opts.theme); + appendDryRunExtras(headerLines, starshipPreset, dryRunStarshipPath); console.log(`${headerLines.join("\n")}\n${script}`); return; } @@ -572,7 +558,7 @@ async function launchTraditionalLayout( `-- Layout: ${totalPanes} editor ${totalPanes === 1 ? "pane" : "panes"}, editor=${plan.editor}, sidebar=${plan.sidebarCommand}, shell=${plan.hasShell}`, `-- Target: ${targetDir}`, ]; - appendDryRunExtras(headerLines, starshipPreset, dryRunStarshipPath, opts.theme); + appendDryRunExtras(headerLines, starshipPreset, dryRunStarshipPath); console.log(`${headerLines.join("\n")}\n${script}`); return; } diff --git a/src/layout.test.ts b/src/layout.test.ts index fb36d73..03187e5 100644 --- a/src/layout.test.ts +++ b/src/layout.test.ts @@ -200,21 +200,6 @@ describe("validation constants", () => { expect(plan.float).toBe(false); }); - it("defaults theme to null", () => { - const plan = planLayout(); - expect(plan.theme).toBeNull(); - }); - - it("passes through theme value", () => { - const plan = planLayout({ theme: "nord" }); - expect(plan.theme).toBe("nord"); - }); - - it("passes through theme: null explicitly", () => { - const plan = planLayout({ theme: null }); - expect(plan.theme).toBeNull(); - }); - it("passes through newWindow=true", () => { const plan = planLayout({ newWindow: true }); expect(plan.newWindow).toBe(true); diff --git a/src/layout.ts b/src/layout.ts index 099b9a1..8ffc734 100644 --- a/src/layout.ts +++ b/src/layout.ts @@ -26,7 +26,6 @@ export interface LayoutOptions { fullscreen: boolean; maximize: boolean; float: boolean; - theme: string | null; } const DEFAULT_OPTIONS: LayoutOptions = { @@ -42,7 +41,6 @@ const DEFAULT_OPTIONS: LayoutOptions = { fullscreen: false, maximize: false, float: false, - theme: null, }; export interface LayoutPlan { @@ -61,7 +59,6 @@ export interface LayoutPlan { fullscreen: boolean; maximize: boolean; float: boolean; - theme: string | null; } function parseShell(value: string): { hasShell: boolean; shellCommand: string | null } { @@ -116,6 +113,5 @@ export function planLayout(partial?: Partial): LayoutPlan { fullscreen: opts.fullscreen, maximize: opts.maximize, float: opts.float, - theme: opts.theme ?? null, }; } diff --git a/src/script.test.ts b/src/script.test.ts index beae80c..439c486 100644 --- a/src/script.test.ts +++ b/src/script.test.ts @@ -884,25 +884,6 @@ describe("generateAppleScript", () => { }); }); - describe("theme flag", () => { - it("sets theme on surface config when theme specified", () => { - const plan = planLayout({ theme: "nord" }); - const script = generateAppleScript(plan, "/tmp/test"); - expect(script).toContain('set theme of cfg to "nord"'); - }); - - it("omits theme when theme is null (default)", () => { - const plan = planLayout(); - const script = generateAppleScript(plan, "/tmp/test"); - expect(script).not.toContain("set theme of cfg"); - }); - - it("escapes special characters in theme name", () => { - const plan = planLayout({ theme: 'my "custom" theme' }); - const script = generateAppleScript(plan, "/tmp/test"); - expect(script).toContain('set theme of cfg to "my \\"custom\\" theme"'); - }); - }); }); describe("generateTreeAppleScript", () => { @@ -918,7 +899,6 @@ describe("generateTreeAppleScript", () => { autoResize: false, editorSize: 75, fontSize: null, - theme: null, newWindow: false, fullscreen: false, maximize: false, @@ -1142,25 +1122,6 @@ describe("generateTreeAppleScript", () => { expect(script).toContain("set font size of cfg to 14"); }); - it("theme", () => { - const plan = makePlan( - { type: "pane", name: "editor", command: "vim" }, - { theme: "tokyo-night" }, - ); - const script = generateTreeAppleScript(plan, "/tmp/project"); - - expect(script).toContain('set theme of cfg to "tokyo-night"'); - }); - - it("omits theme when null", () => { - const plan = makePlan( - { type: "pane", name: "editor", command: "vim" }, - ); - const script = generateTreeAppleScript(plan, "/tmp/project"); - - expect(script).not.toContain("set theme of cfg"); - }); - it("always includes SUMMON_WORKSPACE=1 in surface config env vars", () => { const plan = makePlan( { type: "pane", name: "editor", command: "claude" }, diff --git a/src/script.ts b/src/script.ts index a12e93b..a40bac9 100644 --- a/src/script.ts +++ b/src/script.ts @@ -103,7 +103,6 @@ function emitSurfaceConfig( { add, blank }: ScriptBuilder, targetDir: string, fontSize: number | null, - theme: string | null, allEnvVars: string[], ): void { add(0, `tell application "${GHOSTTY_APP_NAME}"`); @@ -116,9 +115,6 @@ function emitSurfaceConfig( if (fontSize !== null) { add(1, `set font size of cfg to ${fontSize}`); } - if (theme !== null) { - add(1, `set theme of cfg to "${escapeAppleScript(theme)}"`); - } const escaped = allEnvVars.map(e => `"${escapeAppleScript(e)}"`).join(", "); add(1, `set environment variables of cfg to {${escaped}}`); blank(); @@ -391,7 +387,7 @@ export function generateAppleScript(plan: LayoutPlan, targetDir: string, loginSh const allEnvVars = buildEnvVarsList(starshipConfigPath, envVars); - emitSurfaceConfig(sb, targetDir, plan.fontSize, plan.theme, allEnvVars); + emitSurfaceConfig(sb, targetDir, plan.fontSize, allEnvVars); emitNewWindow(sb, plan.newWindow); sb.add(1, "set paneRoot to terminal 1 of selected tab of win"); sb.blank(); @@ -448,7 +444,7 @@ export function generateTreeAppleScript( const rootLeaf = firstLeaf(plan.tree); const rootPaneVar = paneVar(rootLeaf.name); - emitSurfaceConfig(sb, targetDir, plan.fontSize, plan.theme, allEnvVars); + emitSurfaceConfig(sb, targetDir, plan.fontSize, allEnvVars); emitNewWindow(sb, plan.newWindow); sb.add(1, `set ${rootPaneVar} to terminal 1 of selected tab of win`); sb.blank(); diff --git a/src/setup.test.ts b/src/setup.test.ts index 8704f45..ebdf948 100644 --- a/src/setup.test.ts +++ b/src/setup.test.ts @@ -453,9 +453,9 @@ describe("EDITOR_CATALOG", () => { it("each entry has cmd, name, desc fields", () => { for (const entry of EDITOR_CATALOG) { - expect(entry.cmd).toBeTruthy(); - expect(entry.name).toBeTruthy(); - expect(entry.desc).toBeTruthy(); + expect(entry.cmd).toBeTypeOf('string'); + expect(entry.name).toBeTypeOf('string'); + expect(entry.desc).toBeTypeOf('string'); } }); @@ -473,9 +473,9 @@ describe("SIDEBAR_CATALOG", () => { it("each entry has cmd, name, desc fields", () => { for (const entry of SIDEBAR_CATALOG) { - expect(entry.cmd).toBeTruthy(); - expect(entry.name).toBeTruthy(); - expect(entry.desc).toBeTruthy(); + expect(entry.cmd).toBeTypeOf('string'); + expect(entry.name).toBeTypeOf('string'); + expect(entry.desc).toBeTypeOf('string'); } }); }); @@ -490,8 +490,8 @@ describe("LAYOUT_INFO", () => { it("each entry has desc and diagram fields", () => { for (const [, info] of Object.entries(LAYOUT_INFO)) { - expect(info.desc).toBeTruthy(); - expect(info.diagram).toBeTruthy(); + expect(info.desc).toBeTypeOf('string'); + expect(info.diagram).toBeTypeOf('string'); expect(info.diagram).toContain("┌"); } }); diff --git a/src/starship.test.ts b/src/starship.test.ts index c56473a..c4b0650 100644 --- a/src/starship.test.ts +++ b/src/starship.test.ts @@ -210,6 +210,49 @@ describe("getPresetConfigPath", () => { }); }); +describe("listStarshipPresets caching", () => { + it("returns cached result on repeated calls without re-executing starship", () => { + mockResolveCommand.mockReturnValue("/usr/local/bin/starship"); + mockExecFileSync.mockReturnValue("tokyo-night\npastel-powerline\n"); + + const first = listStarshipPresets(); + const second = listStarshipPresets(); + + expect(first).toEqual(["tokyo-night", "pastel-powerline"]); + expect(second).toEqual(first); + // execFileSync should only be called once — second call uses cache + expect(mockExecFileSync).toHaveBeenCalledTimes(1); + }); + + it("re-fetches after resetStarshipCache clears the preset cache", () => { + mockResolveCommand.mockReturnValue("/usr/local/bin/starship"); + mockExecFileSync.mockReturnValue("tokyo-night\n"); + + const first = listStarshipPresets(); + expect(first).toEqual(["tokyo-night"]); + + resetStarshipCache(); + + mockExecFileSync.mockReturnValue("tokyo-night\ngruvbox-rainbow\n"); + const second = listStarshipPresets(); + expect(second).toEqual(["tokyo-night", "gruvbox-rainbow"]); + expect(mockExecFileSync).toHaveBeenCalledTimes(2); + }); + + it("does not cache empty results from missing starship", () => { + mockResolveCommand.mockReturnValue(null); + const first = listStarshipPresets(); + expect(first).toEqual([]); + + // Now starship becomes available + resetStarshipCache(); + mockResolveCommand.mockReturnValue("/usr/local/bin/starship"); + mockExecFileSync.mockReturnValue("tokyo-night\n"); + const second = listStarshipPresets(); + expect(second).toEqual(["tokyo-night"]); + }); +}); + describe("starship path caching", () => { it("calls resolveCommand only once across multiple function calls", () => { mockResolveCommand.mockReturnValue("/usr/local/bin/starship"); diff --git a/src/starship.ts b/src/starship.ts index 0638ce3..3b97847 100644 --- a/src/starship.ts +++ b/src/starship.ts @@ -10,6 +10,9 @@ const STARSHIP_DIR = join(CONFIG_DIR, "starship"); /** Cached resolved path for the starship binary — avoids repeated shell-outs. */ let cachedStarshipPath: string | null | undefined; +/** Cached preset list — avoids repeated `starship preset --list` calls. */ +let cachedPresets: string[] | null = null; + function getStarshipPath(): string | null { if (cachedStarshipPath === undefined) { cachedStarshipPath = resolveCommand("starship"); @@ -20,6 +23,7 @@ function getStarshipPath(): string | null { /** @internal — exported for testing only */ export function resetStarshipCache(): void { cachedStarshipPath = undefined; + cachedPresets = null; } /** Check whether the `starship` binary is available in PATH. */ @@ -29,16 +33,18 @@ export function isStarshipInstalled(): boolean { /** Return the list of built-in Starship preset names, or [] if unavailable. */ export function listStarshipPresets(): string[] { + if (cachedPresets !== null) return cachedPresets; const starshipBin = getStarshipPath(); if (!starshipBin) return []; try { const output = execFileSync(starshipBin, ["preset", "--list"], { encoding: "utf-8", }); - return output + cachedPresets = output .split("\n") .map((l) => l.trim()) .filter(Boolean); + return cachedPresets; } catch { return []; } diff --git a/src/tree.test.ts b/src/tree.test.ts index 43dd44b..ab8a41a 100644 --- a/src/tree.test.ts +++ b/src/tree.test.ts @@ -396,7 +396,6 @@ describe("buildTreePlan", () => { expect(plan.autoResize).toBe(true); expect(plan.editorSize).toBe(75); expect(plan.fontSize).toBeNull(); - expect(plan.theme).toBeNull(); expect(plan.newWindow).toBe(false); expect(plan.fullscreen).toBe(false); expect(plan.maximize).toBe(false); @@ -410,7 +409,6 @@ describe("buildTreePlan", () => { autoResize: false, editorSize: 60, fontSize: 14, - theme: "nord", newWindow: true, fullscreen: true, maximize: true, @@ -419,7 +417,6 @@ describe("buildTreePlan", () => { expect(plan.autoResize).toBe(false); expect(plan.editorSize).toBe(60); expect(plan.fontSize).toBe(14); - expect(plan.theme).toBe("nord"); expect(plan.newWindow).toBe(true); expect(plan.fullscreen).toBe(true); expect(plan.maximize).toBe(true); diff --git a/src/tree.ts b/src/tree.ts index b8b363c..6f03de9 100644 --- a/src/tree.ts +++ b/src/tree.ts @@ -31,7 +31,6 @@ export interface TreeLayoutPlan { fullscreen: boolean; maximize: boolean; float: boolean; - theme: string | null; } // ---------- Parser internals ---------- @@ -329,7 +328,6 @@ interface TreePlanOptions { fullscreen?: boolean; maximize?: boolean; float?: boolean; - theme?: string | null; } /** Build a TreeLayoutPlan from a resolved tree and optional settings. */ @@ -350,7 +348,6 @@ export function buildTreePlan( fullscreen: opts?.fullscreen ?? false, maximize: opts?.maximize ?? false, float: opts?.float ?? false, - theme: opts?.theme ?? null, }; } diff --git a/src/utils.ts b/src/utils.ts index 648b2a3..461fcee 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -6,7 +6,7 @@ import { homedir } from "node:os"; /** Regex for safe command names — only letters, digits, hyphens, dots, underscores, plus signs. */ export const SAFE_COMMAND_RE = /^[a-zA-Z0-9_][a-zA-Z0-9_.+-]*$/; -/** Known Ghostty.app install locations on macOS. */ +/** @internal — exported for testing only */ export const GHOSTTY_PATHS = [ "/Applications/Ghostty.app", join(homedir(), "Applications", "Ghostty.app"), diff --git a/src/validation.test.ts b/src/validation.test.ts index c807401..2d9e3ac 100644 --- a/src/validation.test.ts +++ b/src/validation.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from "vitest"; -import { parseIntInRange, parsePositiveFloat, validateIntFlag, validateFloatFlag } from "./validation.js"; +import { parseIntInRange, parsePositiveFloat, validateIntFlag, validateFloatFlag, ENV_KEY_RE } from "./validation.js"; describe("parseIntInRange", () => { it("returns ok:true with parsed value for valid integer in range", () => { @@ -120,6 +120,40 @@ describe("validateIntFlag", () => { }); }); +describe("ENV_KEY_RE", () => { + it("matches simple uppercase names", () => { + expect(ENV_KEY_RE.test("PATH")).toBe(true); + expect(ENV_KEY_RE.test("HOME")).toBe(true); + }); + + it("matches names starting with underscore", () => { + expect(ENV_KEY_RE.test("_PRIVATE")).toBe(true); + expect(ENV_KEY_RE.test("_")).toBe(true); + }); + + it("matches mixed case with digits", () => { + expect(ENV_KEY_RE.test("MY_VAR_2")).toBe(true); + expect(ENV_KEY_RE.test("node_env")).toBe(true); + expect(ENV_KEY_RE.test("A1B2C3")).toBe(true); + }); + + it("rejects names starting with a digit", () => { + expect(ENV_KEY_RE.test("1BAD")).toBe(false); + expect(ENV_KEY_RE.test("9_VAR")).toBe(false); + }); + + it("rejects empty string", () => { + expect(ENV_KEY_RE.test("")).toBe(false); + }); + + it("rejects names with special characters", () => { + expect(ENV_KEY_RE.test("MY-VAR")).toBe(false); + expect(ENV_KEY_RE.test("MY.VAR")).toBe(false); + expect(ENV_KEY_RE.test("MY VAR")).toBe(false); + expect(ENV_KEY_RE.test("MY=VAR")).toBe(false); + }); +}); + describe("validateFloatFlag", () => { it("returns parsed value on success", () => { expect(validateFloatFlag("font-size", "14.5")).toBe(14.5); diff --git a/src/validation.ts b/src/validation.ts index 6b6d3cc..7e5efe8 100644 --- a/src/validation.ts +++ b/src/validation.ts @@ -7,6 +7,9 @@ import { exitWithUsageHint } from "./utils.js"; +/** Valid environment variable key name: letters, digits, underscores, starting with letter or underscore. */ +export const ENV_KEY_RE = /^[a-zA-Z_][a-zA-Z0-9_]*$/; + type ParseResult = { ok: true; value: number;