diff --git a/.agents/skills/create-pr/SKILL.md b/.agents/skills/create-pr/SKILL.md index 01f1b4fae5..032d3bf41d 100644 --- a/.agents/skills/create-pr/SKILL.md +++ b/.agents/skills/create-pr/SKILL.md @@ -12,7 +12,6 @@ This guide covers best practices for creating pull requests in the warp reposito ## Related Skills - `fix-errors` - Fix presubmit failures (formatting, linting, tests) before opening PR -- `rust-unit-tests` - Write unit tests for your changes, if applicable (see "Testing Requirements" below) - `warp-integration-test` - Add or update integration coverage for user-visible flows, regressions, and P0 use cases - `add-feature-flag` - Gate changes behind feature flags @@ -156,7 +155,7 @@ Code with non-trivial logic should have unit tests to validate functionality: - Sufficiently-simple functions - Trivial getters/setters -See the `rust-unit-tests` skill for guidance on writing unit tests. +Follow the repository's local testing conventions for guidance on writing unit tests. ### UI components need layout validation tests diff --git a/.agents/skills/implement-specs/SKILL.md b/.agents/skills/implement-specs/SKILL.md index d30b84c955..459339bebb 100644 --- a/.agents/skills/implement-specs/SKILL.md +++ b/.agents/skills/implement-specs/SKILL.md @@ -73,7 +73,7 @@ Before considering the work complete, verify that the code matches the current s Prefer: -- `rust-unit-tests` for unit tests and regression coverage +- unit tests and regression coverage that follow the repository's local testing conventions - integration or end-to-end tests for important user flows ## Best Practices @@ -88,4 +88,3 @@ Prefer: - `spec-driven-implementation` - `write-product-spec` - `write-tech-spec` -- `rust-unit-tests` diff --git a/.agents/skills/spec-driven-implementation/SKILL.md b/.agents/skills/spec-driven-implementation/SKILL.md index 3214ca47dc..73c8dc054b 100644 --- a/.agents/skills/spec-driven-implementation/SKILL.md +++ b/.agents/skills/spec-driven-implementation/SKILL.md @@ -122,7 +122,7 @@ The checked-in specs should describe the feature that actually ships, not just t Before considering the work complete, make sure verification maps back to the specs. Prefer tests and artifacts that validate the product behavior directly: -- use the `rust-unit-tests` skill for crate-level unit tests and regression coverage +- unit tests and regression coverage that follow the repository's local testing conventions - integration tests for critical user flows - loom walkthroughs or equivalent feature demonstrations when appropriate - screenshots or videos when useful for UI-heavy work diff --git a/.agents/skills/update-skill/SKILL.md b/.agents/skills/update-skill/SKILL.md index a17cd9e2d8..b301684896 100644 --- a/.agents/skills/update-skill/SKILL.md +++ b/.agents/skills/update-skill/SKILL.md @@ -36,7 +36,7 @@ Use this skill when the user needs to work with PDF files... Every SKILL.md must start with YAML frontmatter containing: - **name**: Kebab-case identifier (lowercase letters, numbers, hyphens only) - - Example: `add-feature-flag`, `rust-unit-tests`, `update-skill` + - Example: `add-feature-flag`, `pdf-processing`, `update-skill` - **description**: Specific description of what the skill does and when to use it - Must be non-empty - Should include key terms for skill discovery @@ -101,7 +101,6 @@ Keep only essential workflow and procedural instructions in SKILL.md. Move detai For reference on structure and style: - `.agents/skills/add-feature-flag/SKILL.md` - Multi-step workflow with clear sequential steps -- `.agents/skills/rust-unit-tests/SKILL.md` - Comprehensive guide with code examples and helper utilities - `.agents/skills/remove-feature-flag/SKILL.md` - Cleanup workflow with search commands ## Best Practices diff --git a/WARP.md b/WARP.md index 9b9fec9f6a..835a031eb3 100644 --- a/WARP.md +++ b/WARP.md @@ -38,9 +38,13 @@ Environment variables: ### Platform Setup - `./script/bootstrap` - Platform-specific setup (calls platform-specific bootstrap scripts) +- `./script/bootstrap --install-common-skills` - Platform setup plus common agent skill installation from `skills-lock.json`. +- `./script/install_common_skills --if-needed` - Install or refresh shared agent skills from the standard `npx skills` project lock. - `./script/install_cargo_build_deps` - Install Cargo build dependencies - `./script/install_cargo_test_deps` - Install Cargo test dependencies +`skills-lock.json` is the standard project lock file managed by `npx skills`. `script/run` checks this lock before building and restores the checked-in project skills with the pinned `skills@1.5.6` CLI when the local install stamp is stale. To update the locked common skills, run `npx --yes skills@1.5.6 update -p -y` and commit the resulting `skills-lock.json` and `.agents/skills` changes. + ## Architecture Overview This is a Rust-based terminal emulator with a custom UI framework called **WarpUI**. diff --git a/script/bootstrap b/script/bootstrap index 59c814e416..cea3f72aa3 100755 --- a/script/bootstrap +++ b/script/bootstrap @@ -1,24 +1,99 @@ #!/usr/bin/env bash +set -eo pipefail # Bootstrap dispatches to the platform-specific install scripts. Each step # that needs root sources `script/warp_sudo` and asks for confirmation # before running. Pass -y / --yes (or set WARP_SKIP_SUDO_PROMPT=1) to skip # the prompts in unattended environments. +OS_TYPE="$(uname -s)" +INSTALL_COMMON_SKILLS=0 +PLATFORM_ARGS=() + +usage() { + cat </dev/null)"; then + case "${git_path}" in + /*) + echo "${git_path}" + ;; + *) + echo "${REPO_ROOT}/${git_path}" + ;; + esac + return + fi + + echo "${REPO_ROOT}/.agents/skills/.common-skills-lock.hash" +} + +stamp_hash() { + local file="$1" + + if [[ -f "${file}" ]]; then + cat "${file}" + fi +} + +install_from_lock() { + ( + cd "${REPO_ROOT}" + npx --yes "skills@${SKILLS_CLI_VERSION}" experimental_install + ) +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --if-needed) + IF_NEEDED=1 + shift + ;; + --non-interactive) + shift + ;; + --force) + FORCE=1 + shift + ;; + --quiet) + QUIET=1 + shift + ;; + -h|--help) + usage + exit 0 + ;; + *) + echo "error: unknown argument: $1" >&2 + usage >&2 + exit 1 + ;; + esac +done + +if [[ "${WARP_SKIP_COMMON_SKILLS_INSTALL:-}" = "1" ]]; then + log "Skipping common-skills install because WARP_SKIP_COMMON_SKILLS_INSTALL=1." + exit 0 +fi + +if [[ ! -f "${LOCK_FILE}" ]]; then + echo "error: missing skills lock file: ${LOCK_FILE}" >&2 + exit 1 +fi + +LOCK_HASH="$(lock_hash)" +STAMP_FILE="$(stamp_file)" +STAMP_HASH="$(stamp_hash "${STAMP_FILE}")" + +if [[ "${FORCE}" -ne 1 && "${IF_NEEDED}" -eq 1 && "${STAMP_HASH}" = "${LOCK_HASH}" ]]; then + log "Common skills are already up to date." + exit 0 +fi + +echo "Installing common agent skills from skills-lock.json." +install_from_lock +mkdir -p "$(dirname "${STAMP_FILE}")" +printf "%s\n" "${LOCK_HASH}" > "${STAMP_FILE}" diff --git a/script/macos/bootstrap b/script/macos/bootstrap index fea3bae4c0..7ae633a557 100755 --- a/script/macos/bootstrap +++ b/script/macos/bootstrap @@ -27,6 +27,7 @@ if [ -z "$DEVELOPER_DIR" ] || [ ! -x "$DEVELOPER_DIR/usr/bin/xcodebuild" ] || [ exit 1 fi echo "Selected Xcode at $XCODE_APP" + echo "You may be prompted for your password so bootstrap can set Xcode as the active developer directory." warp_sudo xcode-select --switch "$XCODE_APP/Contents/Developer" fi # Mimic actually launching XCode, which performs some necessary set-up of the diff --git a/script/run b/script/run index 25f1925bf5..c06a2ffef7 100755 --- a/script/run +++ b/script/run @@ -19,6 +19,8 @@ cd "${REPO_ROOT}" OS_TYPE="$(uname -s)" FEATURES="gui" +INSTALL_COMMON_SKILLS=1 +FORCE_COMMON_SKILLS=0 ./script/install_channel_config || echo "Skipping internal channel config installation (no repo access)." @@ -63,6 +65,10 @@ while (( "$#" )); do exit 1 fi ;; + --install-common-skills) + FORCE_COMMON_SKILLS=1 + shift + ;; --release) CARGO_PARAMS+=("$1") MAC_ARGS+=("$1") @@ -87,6 +93,14 @@ while (( "$#" )); do esac done +if [[ "$INSTALL_COMMON_SKILLS" -eq 1 ]]; then + if [[ "$FORCE_COMMON_SKILLS" -eq 1 ]]; then + ./script/install_common_skills --force --non-interactive || echo "Skipping common-skills update." + else + ./script/install_common_skills --if-needed --non-interactive --quiet || echo "Skipping common-skills update." + fi +fi + # These cargo features were removed and replaced by environment variables read # by warp-channel-config. Intercept them here so that existing --features # invocations keep working. diff --git a/script/windows/bootstrap.ps1 b/script/windows/bootstrap.ps1 index 71109641d7..53f1d05cd8 100644 --- a/script/windows/bootstrap.ps1 +++ b/script/windows/bootstrap.ps1 @@ -1,7 +1,52 @@ #!/usr/bin/env powershell +param( + [switch]$Help, + [switch]$InstallCommonSkills +) $ErrorActionPreference = 'Stop' +function Show-Usage { + Write-Output 'Usage: .\script\windows\bootstrap.ps1 [-Help] [-InstallCommonSkills]' + Write-Output '' + Write-Output 'Prepare this checkout for Warp development on Windows.' + Write-Output '' + Write-Output 'Options:' + Write-Output ' -Help Show this help message.' + Write-Output ' -InstallCommonSkills Install or update common agent skills from skills-lock.json.' + Write-Output '' + Write-Output 'Environment:' + Write-Output ' WARP_SKIP_COMMON_SKILLS_INSTALL=1' + Write-Output ' Skip installing common agent skills.' +} + +function Show-BootstrapPreview { + Write-Output 'Warp bootstrap is starting for Windows.' + Write-Output 'It will:' + Write-Output ' - Check for Git for Windows.' + Write-Output ' - Install Rust if cargo is unavailable.' + Write-Output ' - Install Visual Studio Build Tools, jq, CMake, InnoSetup, and gcloud as needed.' + Write-Output ' - Install Cargo test dependencies.' + + if (-not $InstallCommonSkills) { + Write-Output ' - Skip common agent skills unless -InstallCommonSkills is provided.' + } elseif ($env:WARP_SKIP_COMMON_SKILLS_INSTALL -eq '1') { + Write-Output ' - Skip common agent skills because WARP_SKIP_COMMON_SKILLS_INSTALL=1.' + } else { + Write-Output ' - Install or update common agent skills from skills-lock.json if needed.' + } + + Write-Output 'Run .\script\windows\bootstrap.ps1 -Help to see options and environment overrides.' + Write-Output '' +} + +if ($Help) { + Show-Usage + exit 0 +} + +Show-BootstrapPreview + # Git for Windows can be installed system-wide (Program Files) or per-user (LOCALAPPDATA\Programs\Git). $gitBinCandidates = @( "$env:PROGRAMFILES\Git\bin", @@ -13,6 +58,11 @@ if (-not $gitBinDir) { Write-Error 'https://gitforwindows.org/' exit 1 } +$env:PATH = "$gitBinDir;$env:PATH" + +function Install-CommonSkill { + & "$gitBinDir\bash.exe" "$PWD\script\install_common_skills" --if-needed +} if (-not (Get-Command -Name cargo -Type Application -ErrorAction SilentlyContinue)) { Write-Output 'Installing rust...' @@ -69,3 +119,7 @@ if ($identityToken.Trim().Length -eq 0) { Read-Host gcloud auth login } + +if ($InstallCommonSkills) { + Install-CommonSkill +} diff --git a/skills-lock.json b/skills-lock.json new file mode 100644 index 0000000000..70515a6672 --- /dev/null +++ b/skills-lock.json @@ -0,0 +1,65 @@ +{ + "version": 1, + "skills": { + "create-pr": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/create-pr/SKILL.md", + "computedHash": "d9f06f0219b3d402cf1ff3b0ee3a4d6d38f259a0dcf6103a99b1815c2b81d95b" + }, + "diagnose-ci-failures": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/diagnose-ci-failures/SKILL.md", + "computedHash": "7a7bbe627bbf747c05e37f9a815ddaedea30a588c452e9279b1305ec172d1006" + }, + "fix-errors": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/fix-errors/SKILL.md", + "computedHash": "f5099304ec8e6bc8be578ebc973de31e46ff1e8f0fe671d1641a395a9470ff7a" + }, + "implement-specs": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/implement-specs/SKILL.md", + "computedHash": "bd2787a63cfdc917275769ef2d9c7adc701cfaf1a74344b3a6248d4116a8923e" + }, + "resolve-merge-conflicts": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/resolve-merge-conflicts/SKILL.md", + "computedHash": "5376b5692901c624e8f20a5a04aeea5f5a94f5168d29852a8a639aded6408f2e" + }, + "review-pr": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/review-pr/SKILL.md", + "computedHash": "5bb3ee6ea66cd018fe6bc08d003493fc2abce29441af73e86fd35a590312eaff" + }, + "spec-driven-implementation": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/spec-driven-implementation/SKILL.md", + "computedHash": "e334d0f6f0e8fc39055314acad911f36d92d1919372b5e2973cc99d7f8c901b4" + }, + "update-skill": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/update-skill/SKILL.md", + "computedHash": "1e23c5a5c37ed084eced7fa507031e3cdb8e23f09cd5d004e00efd6f66bf200f" + }, + "write-product-spec": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/write-product-spec/SKILL.md", + "computedHash": "f20d141e67057570b1fe7c902d5e563415c5ae4a72027d069550ff5b4c5917f2" + }, + "write-tech-spec": { + "source": "warpdotdev/common-skills", + "sourceType": "github", + "skillPath": ".agents/skills/write-tech-spec/SKILL.md", + "computedHash": "702924ae98de5466f452eba2effbd004de5edd91617097372c4725a5b5035919" + } + } +} diff --git a/specs/common-skills-installation/TECH.md b/specs/common-skills-installation/TECH.md new file mode 100644 index 0000000000..950c4a647f --- /dev/null +++ b/specs/common-skills-installation/TECH.md @@ -0,0 +1,87 @@ +# Common Skills Installation — Tech Spec + +## Context +This PR replaces the custom `.agents/common-skills.lock` flow with the standard project lock managed by `npx skills`. The checked-in `skills-lock.json` records each common skill from `warpdotdev/common-skills`, including its source, skill path, and content hash (`skills-lock.json:1`). The repository also checks in the restored `.agents/skills/*` copies so local and cloud agents can discover skills directly from the checkout. + +The main install entrypoint is `script/install_common_skills`. It points at `skills-lock.json` (`script/install_common_skills:6`) and stores a local, untracked stamp under `.git/warp/common-skills-lock.hash` (`script/install_common_skills:7`). The script hashes the lock (`script/install_common_skills:39`) and skips work when that hash matches the stamp (`script/install_common_skills:119`). When the stamp is missing or stale, it restores from the lock by running `npx --yes "skills@${SKILLS_CLI_VERSION}" experimental_install` (`script/install_common_skills:72`) and writes the new stamp (`script/install_common_skills:127`). + +`script/run` now checks common skills before launching a local build. It enables the check by default (`script/run:22`) and calls `script/install_common_skills --if-needed --non-interactive --quiet` unless the user explicitly passes `--install-common-skills`, which forces a restore (`script/run:68`, `script/run:96`). Bootstrap remains opt-in through `./script/bootstrap --install-common-skills` and delegates to the same installer (`script/bootstrap:21`, `script/bootstrap:77`). `WARP.md` documents the standard update command and the files reviewers should expect to change (`WARP.md:41`). + +## Diagrams +### Local agent installation and update flow +```mermaid +flowchart TD + A["warpdotdev/warp checkout"] --> B["skills-lock.json"] + A --> C[".agents/skills/*"] + + B --> D["script/run"] + D --> E["script/install_common_skills --if-needed --quiet"] + E --> F["Hash skills-lock.json"] + F --> G{"Does .git/warp/common-skills-lock.hash match?"} + + G -->|Yes| H["Skip restore"] + H --> I["Continue local build/run"] + + G -->|No| J["npx --yes skills@1.5.6 experimental_install"] + J --> K["Read skills-lock.json"] + K --> L["Fetch locked skills from warpdotdev/common-skills"] + L --> M["Restore .agents/skills/*"] + M --> N["Write .git/warp/common-skills-lock.hash"] + N --> I + + O["Developer updates common skills"] --> P["npx --yes skills@1.5.6 update -p -y"] + P --> Q["Update skills-lock.json hashes"] + P --> R["Update changed .agents/skills/* files"] + Q --> S["Commit lock + skill changes"] + R --> S + S --> A +``` + +### Oz cloud agent environment setup flow +```mermaid +flowchart TD + A["Oz Claude cloud-agent run requested"] --> B["Oz selects environment"] + B --> C["Environment clones or syncs warpdotdev/warp"] + C --> D["Checkout contains skills-lock.json and .agents/skills/*"] + + D --> E["Environment setup invokes ./script/install_common_skills --if-needed"] + E --> F["Hash skills-lock.json in cloud checkout"] + F --> G{"Does cloud-local stamp match?"} + + G -->|Yes| H["Skip restore"] + G -->|No or first setup| I["npx --yes skills@1.5.6 experimental_install"] + I --> J["Read skills-lock.json"] + J --> K["Fetch locked skills from warpdotdev/common-skills"] + K --> L["Restore .agents/skills/*"] + L --> M["Write .git/warp/common-skills-lock.hash"] + + H --> N["Start Claude agent"] + M --> N + N --> O["Repo-local skills are discoverable"] + N --> P["Optional explicit skill spec"] + P --> Q["Example: --skill warpdotdev/warp:create-pr"] +``` + +## Proposed changes +The implementation should keep `skills-lock.json` as the single source of truth for common skills installed from `warpdotdev/common-skills`. The repo should not maintain a second custom lock format or a separate GitHub workflow for scheduled common-skill updates. + +`script/install_common_skills` owns restoration from the lock. It should remain small and deterministic: compute a hash for `skills-lock.json`, compare it with a checkout-local stamp, run `npx --yes skills@1.5.6 experimental_install` only when needed, and update the stamp after a successful restore. The stamp belongs under `.git` so running the script does not create or modify tracked files unless the lock itself has been updated intentionally. + +`script/run` should call the installer before building so local developer runs pick up lock changes automatically. This makes `script/run` the dependency-update check point requested during review: when a branch changes `skills-lock.json`, the next run restores the matching skill contents without requiring a separate workflow. `--install-common-skills` is retained as a force-install escape hatch. + +`script/bootstrap --install-common-skills` should continue to be an opt-in bootstrap path and delegate to the same installer. This keeps platform setup and normal run setup consistent. + +For Oz cloud runs, this PR provides the repository-side installer that environment setup should call after cloning or syncing the repository and before starting the Claude agent. A fresh environment will have no `.git/warp/common-skills-lock.hash`, so `./script/install_common_skills --if-needed` restores from `skills-lock.json` once. A reused environment skips the restore when the stamp matches the checked-out lock. After this step, the Claude agent can discover repo-local checked-in skills, and Oz can still pass an explicit skill spec such as `warpdotdev/warp:create-pr` when the run should start from a specific skill. The Oz environment hook itself lives outside this repo; the implementation boundary here is making the repo checkout self-sufficient and idempotent when that hook invokes the script. + +Updates to common skills should be explicit developer actions: run `npx --yes skills@1.5.6 update -p -y`, review the generated `skills-lock.json` and `.agents/skills/*` changes, and commit them together. This preserves dependency-review semantics without adding repository-specific scheduled automation. + +## Testing and validation +Validate the shell changes with `bash -n script/install_common_skills script/run script/bootstrap`. + +Validate the Windows bootstrap script parses with PowerShell: `pwsh -NoProfile -Command '$null = [scriptblock]::Create((Get-Content -Raw "script/windows/bootstrap.ps1"))'`. + +Validate the restore path by running `./script/install_common_skills --if-needed --quiet` from a checkout without a matching local stamp. It should run the pinned `skills@1.5.6` restore command, restore the locked `.agents/skills/*` contents, and write `.git/warp/common-skills-lock.hash`. + +Validate the skip path by running `./script/install_common_skills --if-needed --quiet` again. It should exit successfully without output and without changing the worktree. + +Validate update behavior by running `npx --yes skills@1.5.6 update -p -y` in a test checkout or intentional update branch. If upstream common skills changed, the diff should be limited to `skills-lock.json` and the affected `.agents/skills/*` files.