diff --git a/.claude/rules/ci.md b/.claude/rules/ci.md new file mode 100644 index 0000000000..4929d3b00c --- /dev/null +++ b/.claude/rules/ci.md @@ -0,0 +1,124 @@ +--- +paths: + - ".github/workflows/**/*" + - ".github/actions/**/*" + - ".github/scripts/**/*" + - ".github/actionlint.yaml" + - ".github/*baseline*" + - "scripts/ci/**/*" + - ".swiftlint.yml" + - "**/.swiftlint.yml" +--- + +# CI + +The required check names are in `AGENTS.md` (CI). Reading a red check: `.claude/skills/fix-issue/references/verification-apple.md`, Reading CI. The release and plugin builds: `RELEASING.md`. + +## Rulesets and required checks + +- There are no gate jobs. A test job runs on `!cancelled() && needs.changes.outputs.run != 'false'`. A job its `if:` skips reports success, and a failed changes job runs the suite rather than skipping it. +- `Changes (macOS)` and `Changes (iOS)` are not required checks. +- A PR need not be up to date with main to merge (`strict_required_status_checks_policy: false`). +- A second ruleset blocks force-push and deletion of `main`, with no bypass. +- A required check's name is its job's `name:`. Renaming a job changes the ruleset in the same change, and every open PR then merges main in before its checks report the new name. + +## Runners and toolchain + +- Every Xcode job in the test and release workflows runs on the `xcode-27` runner label, a GitHub preview image. actionlint learns the label from `.github/actionlint.yaml`. `build-plugin.yml` picks its runner from the ref it builds (`RELEASING.md`). +- The Xcode version is written once, in `.github/actions/setup-xcode/action.yml`, which runs `xcode-select --switch` and prints `xcodebuild -version`. A workflow never selects Xcode another way. +- XcodeGen comes from `.github/actions/setup-xcodegen`, pinned by version and SHA-256. +- The compilation cache is off on CI (`COMPILATION_CACHE_ENABLE_CACHING=NO`); `Configs/Base.xcconfig` turns it on locally. + +## macOS Tests (`macos-tests.yml`) + +- Triggers: pull request, push to main, dispatch. A PR keeps only its newest run; each main commit has its own concurrency group and is never cancelled. +- `Changes (macOS)` outputs `run` (detector `macos`) and `plugins` (detector `plugins`). The test job has a 120-minute limit and sets `NSUnbufferedIO=YES`. +- Setup, in order: checkout, setup-xcode, setup-xcodegen, `scripts/download-libs.sh`, an empty `Configs/Secrets.xcconfig`, `generate-project.sh macos`, SPM restore, resolve, SPM save. +- One `build-for-testing` with no `-testPlan` builds both plans and writes one `.xctestrun` per plan, with signing, the index store and the compilation cache off. +- `COMPILE_PLUGINS` is true on every push and dispatch, and on a PR when the `plugins` detector is not false. It adds Go, the Dameng and HANA bridge builds, `xcodebuild build -scheme AllPlugins`, then `scripts/ci/verify-plugin-loads.sh`. +- `test-without-building -testPlan Unit`, then `-testPlan UI`. The UI plan runs after a Unit failure too, so one round trip reports both. +- No shards, and no parallel, iteration or retry flags: the plans decide. +- `scripts/ci/summarize-xcresult.sh` always runs. Evidence uploads only on failure (`.claude/skills/fix-issue/references/verification-apple.md`, Reading CI). + +## iOS and Package Tests (`ios-tests.yml`) + +- Triggers and concurrency as macOS Tests. `Changes (iOS)` outputs `run` (detector `ios`) and `oracle` (detector `oracle`). +- 50 minutes on `xcode-27`, simulator `iPhone 18 Pro`. The iOS platform is downloaded only when that simulator is missing. +- `TableProMobileTests` runs with `-parallel-testing-enabled NO`: a second booted simulator killed runs with "(ipc/mig) server died" on the 3-core runner. `TableProMobile/project.yml` says `parallelizable: true`, which every run overrides. +- `swift test --package-path Packages/TableProCore --parallel`, then TableProOracle with `--force-resolved-versions` when its detector is not false. Each step has `if: !cancelled()`, so one red step never hides the next. +- The summary step always runs. Evidence uploads only on failure. + +## Summaries (`scripts/ci/summarize-xcresult.sh`) + +- Reads `.xcresult` bundles and `swift test` xunit files, and skips a missing input. +- Writes build errors with file and line, per-bundle counts and minutes, up to 50 failed cases and the 20 slowest to the step summary. +- Exits 1 when a bundle or a `swift test` run executed nothing, because `xcodebuild` and `swift test` both call that success. +- For `X.xml` it also reads `X-swift-testing.xml`. +- It never fails on a failed case; the test step already did. + +## SwiftLint (`swiftlint.yml`) + +- PR, push to main, dispatch; Ubuntu, 10 minutes. +- Runs the digest-pinned `ghcr.io/realm/swiftlint` image over the whole repo with `lint --strict`. The image's entrypoint is `swiftlint`, so the command never repeats it. +- Fails when stderr has `Invalid configuration` or `is not a valid rule identifier`, because SwiftLint drops a custom rule whose regex does not compile and still exits 0. + +## PR title (`pr-title.yml`) + +- Runs on opened, edited, reopened and synchronize, so every head commit gets the check. +- Runs `scripts/ci/check-pr-title.py`, then `scripts/check-banned-words.sh` on the PR's added lines and on its body. The banned-words step runs both and exits with the worse result. + +## Repo Hygiene (`repo-hygiene.yml`) + +- PR, push to main, dispatch, no path filter; Ubuntu, 15 minutes. +- Steps, in order: + - actionlint, which also shellchecks every inline `run:` block; + - `shellcheck --severity=warning` on every `*.sh` under `scripts` and `.claude/skills`; + - the tooling tests: `scripts/ci/test_*.py`, `scripts/test_localization.py`, `.github/scripts/test_update_registry.py` and two shell tests; + - `check-lib-sourcing.py`, `check-plugin-manifest.py`, `check-deployment-target.sh`, `check-ios-shared-isolation.py`; + - `localization.py plugins` and `plugins --target ios`; + - `check-string-catalogs.py`, `check-license-inventory.py`, `check-entitlements.py`, `check-test-plans.py`; + - `check-pluginkit-bump-cadence.py`, `check-sparkle-version.py`; + - `scripts/audit-refactor-health.sh --check`, `check-hub-ratchet.py`; + - the Go checks on the HANA helper, when the `hana` detector is true. +- `audit-refactor-health.sh --check` fails a file defining a production `DatabaseType` outside `Packages/TableProCore/Sources/TableProCoreTypes/DatabaseType.swift`, unless `.github/duplicate-contract-baseline.txt` lists its path. Moving a listed file moves its baseline path in the same commit. Never add an entry to let a new copy pass. +- `check-hub-ratchet.py` fails when an app hub grows past `.github/hub-ratchet-baseline.json`. `--update` only lowers a value, so growing one is a hand edit a reviewer sees. + +## Change detector (`scripts/ci/detect-changed-paths.sh`) + +- `detect-changed-paths.sh [--base ] macos|plugins|ios|oracle|hana` prints `true` or `false`. An unknown suite or flag exits 2. +- A PR passes its base SHA, a push `github.event.before`. No base, or the all-zero SHA of a new ref, prints true. +- `macos` watches the app, test, plugin, package, native, config and libs folders, `TestPlans/`, `project.yml`, the app's `Package.resolved`, the build scripts and CI scripts it runs, both setup actions and its workflow. `Packages/` covers TableProMacCore. +- `plugins` watches `Plugins/`, `Packages/TableProCore/`, `Packages/TableProOracle/`, `Native/`, `Libs/`, `Configs/`, `project.yml`, the bridge scripts and `verify-plugin-loads.sh`. +- `ios` watches `TableProMobile/`, `Libs/`, `Plugins/TableProPluginKit/` (the package target is a symlink to it), and every `../` path it reads out of `TableProMobile/project.yml`. +- `oracle` watches `Packages/TableProOracle/`; `hana` watches `Native/HanaBridge/`. Each suite also watches its own workflow. +- A path a suite newly reads joins that suite's pattern in the same change, or the suite skips on it. +- Release-only files never trigger a suite: `CHANGELOG.md`, `appcast.xml`, `TablePro/Resources/WhatsNew.md`, `.github/release-flags.json`, and `Configs/Version.xcconfig` while its diff touches only the two version keys. +- Every file the release commit stages (`.claude/skills/release/SKILL.md`, Commit, tag, push) is on that list; `appcast.xml` is `build.yml`'s own push. +- A release push that touches anything else runs the suites, and the tag waits for them. + +## What runs where + +- A pull request: both changes jobs; macOS Tests and iOS and Package Tests unless their detector says false (Oracle only when its detector is not false); SwiftLint; Repo Hygiene; PR title. `AllPlugins` only when the `plugins` detector is not false. +- A push to main: the same minus PR title, never cancelled, `AllPlugins` always, the SPM cache saved on a miss. Every code push runs both plans in full; a docs-only or release-only push skips the suites through the detector. +- A `v*` tag runs only `build.yml`; a `plugin-*-v*` tag runs only `build-plugin.yml` (`RELEASING.md`). The test workflows never run on a tag. +- `workflow_dispatch` exists on every workflow except `pr-title.yml`. + +## Caches and runner budget + +- Swift package checkouts live in `~/.spm-cache`, keyed `spm--` with prefix restore. The cache is saved only on a push to main and only on a miss: a cache saved on a PR ref is visible to that PR alone. +- `build.yml` restores that cache and never saves it, since a save in tag scope is unreadable from other tags. +- There is no other cache: no libs or DerivedData cache, and none in the iOS, plugin or Ubuntu workflows. +- CI fits the GitHub Free and Pro plans: no paid larger runner, no self-hosted Mac. At most 5 macOS jobs run at once, and main runs queue behind PR runs for those slots. + +## Writing workflow and lint code + +- A SwiftLint custom rule is severity `error`, has a `message` that says what to do and why, and names its files with `included:` or `excluded:`. +- A `scripts/ci/check-*.py` gets a `test_*.py` beside it and a step in Repo Hygiene. +- Prove a new SwiftLint rule fires: plant a violation, run `swiftlint lint --strict --no-cache `, remove it. +- Nested `.swiftlint.yml` files narrow the rules with `only_rules` under `TableProTests`, `Plugins` (except `Plugins/TableProPluginKit`, linted in full), `TableProMobile` and `TableProUITests`. A new rule applies there only once it joins that file's list. +- A local `swiftlint` run only prints the invalid-rule warning that fails CI (SwiftLint above), so read stderr after editing a rule. +- Run `actionlint` and `shellcheck --severity=warning` on what you changed before pushing; Repo Hygiene runs both. +- Write macOS `run:` blocks for bash 3.2, the `/bin/bash` the runner has used. `done < <(script)` throws away the script's exit status, so write the output to a file and read that. +- Reading a producer's output one argument per line turns a single blank line into one empty argument. +- Under `set -u`, bash 3.2 fails on `"${ARR[@]}"` for an empty array; write `${ARR[@]+"${ARR[@]}"}`. +- `git diff --name-only` quotes non-ASCII paths. Use `git -c core.quotePath=false diff --no-renames --name-only -z` into a file and match with `LC_ALL=C grep -z`. diff --git a/.claude/rules/data-grid.md b/.claude/rules/data-grid.md index 174e7a4317..07ff0e3f56 100644 --- a/.claude/rules/data-grid.md +++ b/.claude/rules/data-grid.md @@ -15,4 +15,3 @@ paths: - **`gridStyleMask` stays empty; `DataGridBodyChrome` draws column separators and the stripes.** AppKit's vertical grid lines add one subview per column and make every layout pass quadratic. Take separator positions from `presentsColumn` and `rect(ofColumn:)`. - **Scroll the grid with `NSView.scroll(_:)`**, never `clipView.scroll(to:)` plus `reflectScrolledClipView`, which leaves the header behind. - **Accessibility goes through `DataGridCellAccessibilityView`**, mounted for on-screen rows once `DataGridAccessibility.isActive`; `NSTableView` builds its cell tree from views only. Keep that path working whenever row drawing changes. -- **UI tests click at an offset from the `data-grid` element**, never on a row or cell element, which XCUITest reports as obscured. diff --git a/.claude/rules/mysql-driver.md b/.claude/rules/mysql-driver.md index 45f40f4266..6ffab23820 100644 --- a/.claude/rules/mysql-driver.md +++ b/.claude/rules/mysql-driver.md @@ -1,6 +1,7 @@ --- paths: - "Plugins/MySQLDriverPlugin/**/*" + - "Packages/TableProCore/Sources/TableProMySQLCore/**/*" - "TableProMobile/**/*MySQL*" - "TableProMobile/**/*MariaDB*" --- diff --git a/.claude/rules/plugin-system.md b/.claude/rules/plugin-system.md index 0c88f1bd88..44b30df779 100644 --- a/.claude/rules/plugin-system.md +++ b/.claude/rules/plugin-system.md @@ -2,6 +2,10 @@ paths: - "Plugins/**/*" - "Packages/TableProCore/Sources/TableProPluginKit/**/*" + - "Packages/TableProCore/Sources/TableProMySQLCore/**/*" + - "Packages/TableProCore/Sources/TableProPostgreSQLCore/**/*" + - "Packages/TableProCore/Sources/TableProRedisCore/**/*" + - "Packages/*/Package.swift" - "project.yml" - "TableProMobile/project.yml" - ".github/workflows/build-plugin.yml" @@ -11,18 +15,61 @@ paths: # Plugins and PluginKit -TableProPluginKit is built with Library Evolution, so an already-built plugin keeps loading under a newer app. These rules keep that true: +TableProPluginKit is built with Library Evolution, so an already-built plugin keeps loading under a newer app. -- **Never remove or change a published requirement or public signature.** A plugin built against it references its symbols by name and fails to load ("Bundle failed to load executable") when one disappears, even a requirement that defaulted to `nil`. To add a field to a transfer struct, add a new initializer overload and mark the old one `@_disfavoredOverload`. -- **Add a requirement only with a default implementation**, then bridge it in `PluginDriverAdapter`. -- **Every ABI change bumps `currentPluginKitVersion`** (in `PluginManager.swift`) and `TableProPluginKitVersion` in every plugin `Info.plist`, at most once per release cycle: later changes in the same cycle reuse the pending number (`scripts/ci/check-pluginkit-bump-cadence.py` checks it). A breaking change also raises `minimumCompatiblePluginKitVersion` and needs every registry plugin re-released with `scripts/release-all-plugins.sh ` before or with the app release. -- **Run `verify.sh abi `** (`scripts/check-pluginkit-abi.sh`) for any change under `Plugins/TableProPluginKit/`; no CI job runs it. -- **Mark a public enum `@frozen` only when its case set is truly closed.** `PluginCapability` and the transfer structs stay non-frozen so they can grow. -- **A driver fix reaches users of a shipped app** through `scripts/release-plugin-for-shipped-app.sh [appTag]`. Never relabel a binary built from `main` with an older kit version. +## ABI -Building and structure: +- Never remove or change a published requirement or public signature. A plugin references its symbols by name and fails to load ("Bundle failed to load executable") when one disappears, even a requirement that defaulted to `nil`. +- To add a field to a transfer struct, add an initializer overload and mark the old one `@_disfavoredOverload`. +- Add a requirement only with a default implementation, then bridge it in `PluginDriverAdapter`. +- Additive: a new requirement with a default, a new field on a non-`@frozen` struct, a new case on a non-`@frozen` enum, a reordering. +- Breaking: a changed, removed or renamed requirement or signature, a requirement without a default, a new case on a `@frozen` enum, a change to a frozen type's layout. +- Every ABI change bumps `currentPluginKitVersion` (in `PluginManager.swift`) and `TableProPluginKitVersion` in every plugin `Info.plist`, at most once per release cycle: later changes in the cycle reuse the pending number. `scripts/ci/check-pluginkit-bump-cadence.py` checks it in Repo Hygiene. +- An additive change re-publishes nothing. Built plugins keep loading, and the bump makes an older app refuse a plugin rebuilt against the new API instead of failing its load. +- A breaking change also raises `minimumCompatiblePluginKitVersion`, and every registry plugin is re-released with `scripts/release-all-plugins.sh ` before the app tag, because the release's `Registry Readiness` job fails while a registry driver has no binary in the app's kit range (`RELEASING.md`, The release pipeline). +- Check with `verify.sh abi ` (`scripts/check-pluginkit-abi.sh`). It builds PluginKit at HEAD and at the base with the same toolchain and diffs the public `.swiftinterface`. A base with no interface passes; a diff prints the additive and breaking rules and exits 1. +- It refuses uncommitted changes, so run it after the commit and before the push. No workflow runs it. +- The diff is textual, so a `-` line can be a declaration that moved. Only a removed line with no matching added line, or a requirement added without a default, is breaking. This lists the removed lines that do not come back: -- **The app scheme builds only the bundled plugins**; run `verify.sh plugins` (the `AllPlugins` aggregate) whenever `Plugins/` changes. -- **A new driver** needs its `project.yml` target and an entry in the `AllPlugins` list, a `DatabaseType` constant, an entry in `.github/plugin-registry.json` keyed by its tag slug, a curated snapshot in `PluginMetadataRegistry` (without it the type never appears in the New Connection picker), a row in `../tablepro-docs/databases/index.mdx` (a paired docs pull request), and a CHANGELOG entry. -- **A `Plugins/` file that `TableProMobile/project.yml` also compiles** marks every top-level declaration `nonisolated` and depends on no plugin-only code, because the iOS target defaults to `MainActor` (`scripts/ci/check-ios-shared-isolation.py` checks it). -- **Plugin tags** are `plugin--v`, where the slug is a key in `.github/plugin-registry.json`; check `git tag -l "plugin-*"` before creating one. + ```bash + L= + comm -23 <(grep -E '^-[^-]' "$L" | sed 's/^-[[:space:]]*//' | sort -u) \ + <(grep -E '^\+[^+]' "$L" | sed 's/^+[[:space:]]*//' | sort -u) + ``` + +- Mark a public enum `@frozen` only when its case set is closed. `PluginCapability` and the transfer structs stay non-frozen so they can grow. +- PluginKit code with no callers stays: removing it is an ABI change. + +## One PluginKit per process + +- R1 (the rule is in `AGENTS.md`, Invariants that apply everywhere): a plugin that linked a package product depending on the `TableProPluginKit` package would carry its own copy of PluginKit's types, and the host's casts across that boundary fail. Products a plugin and the iOS app share stay PluginKit-free for that reason. +- The R1 check runs per binary (each plugin, each plugin test bundle, the app) and must print nothing: + + ```bash + nm -m "$BIN" | grep -v undefined | grep -E '_\$s17TableProPluginKit[0-9A-Za-z_]*Mn$' \ + | awk '{print $NF}' | xcrun swift-demangle \ + | awk '!/\(extension in / || /\(extension in TableProPluginKit\)/' + ``` + +- Without the demangle filter, `TablePro.debug.dylib` prints one false positive: an extension the app declares on a PluginKit type. `TableProPluginKit.framework` itself prints many lines, which makes it the positive control. +- R2: `/Build/Products/Debug/TableProPluginKit.swiftmodule` must not exist. The only PluginKit module is inside `TableProPluginKit.framework`. +- No CI check runs R1 or R2. Run both after changing a package manifest, a package dependency or a plugin's linkage. +- A bundled plugin and its test bundle share one linkage template in `project.yml`, so they never link differently. + +## Building + +- The `TablePro` scheme builds only the bundled plugins, and a registry plugin's compile error still ends in `BUILD SUCCEEDED`. Run `verify.sh plugins` (the `AllPlugins` aggregate) after any change under `Plugins/`. +- `scripts/ci/verify-plugin-loads.sh ...` dlopens every built `.tableplugin` and names any that will not load. It does not instantiate the principal class. + +## Structure + +- PluginKit-free logic a plugin shares with the iOS app lives in a TableProCore product, such as `TableProMySQLCore`, `TableProPostgreSQLCore` or `TableProRedisCore`. +- A new driver needs: its `project.yml` target and an `AllPlugins` entry, a `DatabaseType` constant, an entry in `.github/plugin-registry.json` keyed by its tag slug, a curated snapshot in `PluginMetadataRegistry` (without it the type never appears in the New Connection picker), a row in `../tablepro-docs/databases/index.mdx` through a paired docs PR, and a CHANGELOG entry. +- A new engine ships as a registry plugin (`AGENTS.md`, Scope). +- Where a plugin's tests go, and how a test target is added in `project.yml`: `.claude/rules/tests.md`, Where a test goes and Adding a test target. + +## Tags and shipped apps + +- Plugin tags are `plugin--v`, the slug a key in `.github/plugin-registry.json`. Check `git tag -l "plugin-*"` before creating one. +- A driver fix reaches users of a shipped app through `scripts/release-plugin-for-shipped-app.sh [appTag]`. Never relabel a binary built from `main` with an older kit version. +- How a plugin tag builds and publishes: `RELEASING.md`, Plugins. diff --git a/.claude/rules/redis-driver.md b/.claude/rules/redis-driver.md index fbe0d496c9..ac1c6fa551 100644 --- a/.claude/rules/redis-driver.md +++ b/.claude/rules/redis-driver.md @@ -1,6 +1,7 @@ --- paths: - "Plugins/RedisDriverPlugin/**/*" + - "Packages/TableProCore/Sources/TableProRedisCore/**/*" --- # Redis driver diff --git a/.claude/rules/tests.md b/.claude/rules/tests.md index 80cd3369df..f7c199d638 100644 --- a/.claude/rules/tests.md +++ b/.claude/rules/tests.md @@ -2,13 +2,143 @@ paths: - "TableProTests/**/*" - "TableProUITests/**/*" + - "Plugins/*Tests/**/*" - "Packages/*/Tests/**/*" + - "TableProMobile/TableProMobileTests/**/*" + - "TestPlans/**/*" + - "scripts/ci/check-test-plans.py" + - "scripts/clean-test-residue.sh" --- # Tests -- **Swift Testing, filtered by suite type name.** `-only-testing:TableProTests/` matches the Swift type, not the file or the `@Suite` display name, and a filter that matches nothing still prints `TEST SUCCEEDED`. -- **No top-level `@Suite` without a trait in `TableProTests`.** A type's `@Test` functions are found without it, and each top-level `@Suite` costs the module compile time quadratically. `scripts/ci/check-test-suite-attributes.py` enforces it. -- **UI suites subclass `UITestCase`**, the only launch path that isolates storage; a bare `XCUIApplication()` or `: XCTestCase` under `TableProUITests/` fails a source-scanning test. -- **A test must not depend on the machine**: pin the locale it formats with, use a private `NSPasteboard(name:)`, stub the network, inject the clock. -- **A UI test that fails only on CI for a reason you cannot establish** goes into `.github/macos-ui-test-quarantine.txt` with the reason, rather than a guessed fix. +UI flows: `.claude/rules/ui-tests.md`. Running tests and reading verdicts: `.claude/skills/fix-issue/references/verification-apple.md`. + +## Where a test goes + +| Code under test | Target | Folder | Host | Runs in | +| --- | --- | --- | --- | --- | +| A TableProCore module | `Tests` | `Packages/TableProCore/Tests/Tests/` | none | `swift test`, iOS and Package Tests | +| A PluginKit API | `TableProPluginKitTests` | `Packages/TableProCore/Tests/TableProPluginKitTests/` | none | `swift test`, iOS and Package Tests | +| Logic a plugin and the iOS app share without PluginKit | the test target of its TableProCore product (`TableProMySQLCoreTests`, `TableProMSSQLCoreTests`) | `Packages/TableProCore/Tests/` | none | `swift test`, iOS and Package Tests | +| Oracle wire logic | `TableProOracleCoreTests` | `Packages/TableProOracle/Tests/TableProOracleCoreTests/` | none | `swift test`, iOS and Package Tests when the `oracle` detector fires | +| A TableProMacCore module | `Tests` | `Packages/TableProMacCore/Tests/Tests/` | none | Unit plan | +| The editor or the grammars | `TableProEditorKitTests`, `TableProTextEngineTests`, `TableProGrammarsTests` | `Packages/TableProEditor/Tests/`, `Packages/TableProGrammars/Tests/` | none | Unit plan | +| A bundled driver's logic | `DriverTests` | `Plugins/DriverTests/` | none | Unit plan | +| A bundled format plugin | `FormatPluginTests`, one bundle for all of them | `Plugins/FormatPluginTests/` | none | Unit plan | +| App code that needs the app host: AppKit windows, `DatabaseManager.shared`, a coordinator | `TableProTests` | `TableProTests/` | TablePro.app | Unit plan | +| The iOS app | `TableProMobileTests` | `TableProMobile/TableProMobileTests/` | the iOS app on a simulator | iOS and Package Tests, `verify.sh ios` | +| A new end-to-end user flow | `TableProUITests` | `TableProUITests/` | XCUITest drives TablePro.app | UI plan | + +- Take the first row that can hold the test. A hostless target beats `TableProTests`. +- A registry-only driver gets no test target; `AllPlugins` only compiles it. An app-side gate around a registry engine (Safe Mode, the MCP write tier, the query classifier) is app code with a hosted test. `TableProMSSQLCoreTests` and `TableProOracleCoreTests` stay, because the iOS app links that code. +- A check that needs a live server, or a service no workflow starts, is a hand-run probe in `scripts/probes/`, never a test. No workflow runs a probe. + +## Plans + +- `TestPlans/Unit.xctestplan` is the `TablePro` scheme's default plan. `TestPlans/UI.xctestplan` holds `TableProUITests` alone. +- Unit holds `TableProTests`, the hostless plugin bundles, the Editor and Grammars package targets and the TableProMacCore package targets. +- TableProCore and TableProOracle test targets are in no plan and run under `swift test`. Some depend on the `TableProPluginKit` package, and a plan target that does makes the app and plugins compile against that module instead of the resilient framework. A package joins the Unit plan only when nothing it builds depends on that package; the top of `Packages/TableProMacCore/Package.swift` says so. +- Unit plan settings: language `en`, region `US`, timeouts on with a 60 s default and a 120 s maximum per case, no code coverage, the sandbox variable (Isolation below). UI plan settings are in `.claude/rules/ui-tests.md`. +- The Unit plan never retries. A red case is a bug, or a flake to fix. +- The Unit plan runs serially. Its Xcode targets carry `"parallelizable": false`; the package targets carry no key, so their suites run in parallel in-process. A new Xcode target entry stays serial too. +- Why serial: in-process parallelism starts every `TableProTests` suite at once and starves the main actor, so time limits kill cases and then the host. Parallel clones never split the Swift Testing suites of a hosted target: they all run in one clone. The plugin bundles in parallel save about 1% of the Unit step. +- `-parallel-testing-enabled YES` overrides the plan: it turns in-process Swift Testing parallelism on even where the `.xctestrun` says off. +- Build for testing scheme-wide, or let `verify.sh` build. `build-for-testing -testPlan UI` alone has no `TableProTests` in its graph, so it links TableProMCP statically into the app. `test-without-building -testPlan Unit` on those products then loads two copies, and the MCP cases fail with "expected error of type MCPProtocolError, but MCPProtocolError was thrown". `verify.sh` runs `xcodebuild test -testPlan` for the plan it tests, and CI builds once with no plan. + +## Adding a test target + +- Xcode test targets are defined in `project.yml`. A bundled plugin's target uses the `PluginTests` template plus that plugin's linkage template, and compiles `Plugins/`, `Plugins/Shared` and `Plugins/` against the PluginKit framework. +- Name the folder after the target: `verify.sh` finds a test's target by that path component. +- `TableProTests` compiles only `TableProTests/`. Never add a plugin or package source to it. +- `TableProTests` depends only on what the app does not link: `TableProTestSupport`, `TableProPostgreSQLCore`, `TableProMCPTestSupport`. The app's packages reach it through `TEST_HOST`. +- A second link compiles a second copy of a module into the test bundle. Check with `nm -m` on `/Build/Products/Debug/TablePro.app/Contents/PlugIns/TableProTests.xctest/Contents/MacOS/TableProTests`: it must define no `Mn` descriptor of a module the app links. For TableProMCP (the number is the name's length) this prints nothing: + + ```bash + nm -m "$BIN" | grep -v undefined | grep -E '_\$s11TableProMCP[0-9A-Za-z_]*Mn$' + ``` + +- Add the target to its plan. An Xcode target goes in by its PBXNativeTarget id from the generated `TablePro.xcodeproj/project.pbxproj`, never the scheme's id. A package target goes in with `containerPath: container:Packages/` and its name as identifier. +- `scripts/ci/check-test-plans.py`, run by Repo Hygiene, fails on: a test target missing from its plan or in the wrong one; a plan entry with no target; a TableProCore or TableProOracle target in a plan; a wrong `containerPath`; a duplicate entry; an Xcode test target with no folder of its name; `enabled: false`, `skippedTests` or `selectedTests`. +- It reads target types from `project.yml`, templates included, and test targets from every `Packages/*/Package.swift`, so a new package needs no script change. + +## Writing a test + +- New tests are Swift Testing. Convert an XCTest file you rewrite. `TableProUITests` stays XCTest. +- No bare top-level `@Suite` in `TableProTests`, `Plugins/*Tests` or `Packages/TableProMacCore/Tests`. Each top-level `@Suite` expansion walks every other one, so compile time grows with their square. Keep `@Suite` only to carry a trait. SwiftLint's `suite_without_trait` enforces it in those folders; the other package test folders are not covered yet, so follow it there by hand. +- Hosted tests reach app code through `@testable import TablePro`. MemberImportVisibility is on, so a file that names a PluginKit enum case also imports `TableProPluginKit`. +- A plugin bundle test compiles the plugin's sources into its own module: it imports `TableProPluginKit` and `Testing`, never `@testable import`. +- A test does not depend on the machine. Pin the locale, use `.recordingClipboard` rather than the real pasteboard, keep the network out (`OfflineRegistry` for the plugin registry), inject the clock (`ManualClock`). +- Wait for an effect no event reports with `waitUntil`, which polls every 5 ms and throws `WaitTimeout` naming the file and line. Await an event when one exists. Never a fixed `Task.sleep` or a `Task.yield` loop. +- `.serialized` orders the cases of one suite only; it is not isolation. A suite that swaps a process-wide value across a suspension nests under `GlobalStateSuites`, whose `.serialized` orders every suite inside it. Users of `.appSettings { }` and `.recordingClipboard` nest there. +- A regression test proves something only once it fails without the fix. Revert the fix and watch it fail, or name the change that turns it red. + +## Isolation and the app host + +- When `TABLEPRO_UI_TEST_SANDBOX` is set, `AppStorageEnvironment.shared` resolves every store, the defaults suite and the keychain inside it. `main.swift` calls `AppStorageEnvironment.bootstrap()` before any static initializer runs. The rule for production code is in `AGENTS.md`, Invariants that apply everywhere. +- The Unit plan sets the sandbox to `$(TARGET_TEMP_DIR)/UnitTestStorage`, under DerivedData. The unit host deletes the sandbox's `TablePro/` folder at every launch, so every run starts empty. +- The defaults suite is `com.TablePro.uitest.<16 hex digits>`, a digest of the sandbox path, so each plan, checkout and worktree gets its own. A sandbox without the `.defaults-suite` marker is new and empties its suite, since cfprefsd keeps a suite after its folder is deleted. Plugins derive the same suite through `PluginHostStorage`. +- Under isolation the keychain is `SandboxKeychainStore`, a `keychain.json` in the sandbox. +- AppKit autosave names (split views, window frames, tables) are nil in the unit host and under the sandbox, because AppKit writes those records to the real domain. The rule for a new one is in `AGENTS.md`, Invariants that apply everywhere. +- Stores default to `AppStorageEnvironment.shared.defaults`. ManagedPolicy, AppleLanguages, Sparkle and AppKit stay on `.standard`. +- Sparkle reads the real domain, so under isolation the app starts neither the updater nor sync (`runPostLaunchActivationIfNeeded` in `AppDelegate`). +- `TABLEPRO_UI_TESTING=1` without a sandbox makes the app log a fault and exit. The app cannot detect XCUITest by itself. +- The test host gets no shell environment. A variable goes into the plan's environment entries, or an in-process `setenv`. +- Under XCTest the app skips plugin loading and its normal startup. Keep that guard in `AppDelegate`; the `PluginManager` tests depend on it. Plugin metadata in a hosted test comes from the curated built-in table, so test plugin logic in its bundle. +- `PluginManager.waitForInitialLoad()` returns at once unless `loadPlugins()` started a load. Its 10 s timeout, on the injected `initialLoadClock`, still covers a load that hangs. +- A known write to the real domain, accepted: a test that changes the general settings runs `AppLanguage.apply()`, which writes `AppleLanguages` to `.standard`. +- Proof a change writes nothing real: `ls ~/Library/Preferences | wc -l` and `defaults read com.TablePro | wc -l` give the same numbers before and after one Unit run and one UI run. +- `scripts/clean-test-residue.sh` lists what older test runs left on the Mac: test defaults domains, test keys and sandboxed autosave records in `com.TablePro`, `mcp-audit-test_` databases, test folders in the user temp directory. It never touches per-connection keys. +- Its `--apply` backs everything up to `~/TablePro-test-residue-/` before removing it, and refuses while TablePro or a TablePro test run is active. Give the user the commands (dry run, `--apply`, dry run again until every "Would remove" count is 0) rather than running `--apply` on their Mac. + +## Shared helpers + +Use one before writing a local helper. + +| Helper | Where | Use | +| --- | --- | --- | +| `waitUntil` | `TableProTestSupport` | poll for an effect no event reports | +| `AsyncGate` | `TableProTestSupport` | hold callers until the case calls `open()`; `arrived(_:)` waits for that many callers | +| `ManualClock` | `TableProTestSupport` | time moves only on `advance(by:)`; production code takes `clock: any Clock = ContinuousClock()` | +| `BoundedCall`, `HeldOpenWriter`, `BackgroundPipeWriter`, `BackgroundPipeReader` | `TableProTestSupport` | bound a call that can block; drive a pipe from another thread | +| `GlobalStateSuites` | `TableProTests/Helpers/` | parent of every suite that swaps a process-wide value | +| `EphemeralDefaults` | `TableProTests/Helpers/` | a defaults suite that deletes its domain and plist with its owner | +| `.appSettings { }`, `.recordingClipboard` | `TableProTests/Helpers/` | traits that swap the app settings or the clipboard for one case | +| `CoordinatorFixture` | `TableProTests/Helpers/` | a coordinator that is torn down with the fixture | +| `OfflineRegistry` | `TableProTests/Helpers/` | a registry client that never reaches the network | +| `InMemoryColumnLayoutStore`, `InMemoryKeychain`, `PluginDriverStub`, `TestFixtures` | `TableProTests/Helpers/` | stores and driver doubles | +| `AlwaysAllowGate`, `AlwaysDenyGate` | `TableProTests/Helpers/StubExecutionGates.swift` | an `ExecutionGate` that never raises the real alert | +| `TableProMCPTestSupport` | `Packages/TableProMacCore/Tests/TableProMCPTestSupport/` | MCP doubles: `MCPTestClock`, `MockHttpServer`, `RecordingBridgeLogger` | +| `TableProTLSTestFixtures` | `Packages/TableProCore/Tests/TableProTLSTestFixtures/` | TLS fixtures, also used by the iOS tests | + +`TableProTestSupport` is the TableProCore product at `Packages/TableProCore/Tests/TableProTestSupport/`. + +## Known failures and flakes + +- Mark a known failure in code: `@Test(.disabled("why"), .bug(""))`, `withKnownIssue { }`, or `XCTExpectFailure` in XCTest. `.disabled(if:)` only for a real precondition. +- There are no quarantine files, and a plan never skips a test (`check-test-plans.py` refuses it). +- To reproduce an intermittent failure, build scheme-wide, then run the built plan repeatedly. This is the one place `-test-iterations` belongs: + + ```bash + xcodebuild test-without-building -destination platform=macOS \ + -xctestrun /Build/Products/TablePro_Unit_macosx-arm64.xctestrun \ + -only-testing:/ -test-iterations 10 -run-tests-until-failure + ``` + +- A UI flow that fails only on CI: `.claude/rules/ui-tests.md`, Known failures. + +## Traps + +- One crashing test kills the host, and every later case reports failed at 0.000 s. Find the crash in `~/Library/Logs/DiagnosticReports`. +- `close()` on a test-created `NSWindow` releases it twice and kills the host. Set `isReleasedWhenClosed = false`, or use `orderOut(nil)`. +- A hosted case that reaches the real `ExecutionGate` raises an `NSAlert` that nothing answers, and the run stalls. Inject a gate. +- `NSPasteboard.general` fails headless. Use `.recordingClipboard`. +- On the main actor, a `Task.yield` loop starves what it waits for. +- Awaiting a finished Task does not suspend. +- A synchronous `@MainActor` test that pumps the run loop never runs main-actor Tasks. +- Injected `ConnectionSession`s start `.disconnected`. A shared service that treats "has a driver" as live starts real work from unrelated tests, so such a service checks for `.connected`. +- In an offscreen window a floating subview's container is offset by the header height, and `needsDisplay` cannot be observed. Assert the geometry, not the flag. +- `cacheDisplay` of an `NSTableView` runs an alternating-row pass the screen never shows and copies translucent fills as they are. Test the grid's drawing by calling it into a bitmap. +- CI runner windows are 1x and a developer Mac is 2x, so centred geometry lands half a point apart. Use `1 / window.backingScaleFactor` as the tolerance. +- AppKit's private view hierarchy differs between OS builds (selection material, effect views). Gate such a test on a host probe, `.enabled("why") { await Suite.probe() }`, and pair it with a test that runs everywhere. +- Inside `#expect`, a key path passed to a `rethrows` method (`allSatisfy(\.flag)`) does not compile, nor does a key path to a main-actor property (`filter(\.isSelected)`). Write the closure. The compile error fails the whole target, so no suite runs. diff --git a/.claude/rules/theme.md b/.claude/rules/theme.md index 54b92eefaa..697e8e263b 100644 --- a/.claude/rules/theme.md +++ b/.claude/rules/theme.md @@ -9,14 +9,14 @@ paths: - **A theme colors content and the window backdrop.** Content is every code view, the data grid, value text and wells, value trees, diffs, record tables, diagrams, charts and the result-area states. `window.background` fills the detail pane and, on macOS 26 and later, `NSWindow.backgroundColor` of the four content windows (main, Data File, Compare & Sync, value viewers); the sidebar, inspector and toolbar glass take their tone from it. Controls, Accent, Highlight, Settings, Welcome, the connection form, sheet and popover chrome, SwiftUI `List`, `Form` and `Table` backgrounds and every bar stay system; the result bar only drops its material while `window.background` themes the detail pane. `NSApp.appearance` is the only lever on chrome. - **A code field takes the editor theme wherever it appears.** Startup Commands, Pre-Connect Script, a favorite's query, Rewind and Agent SQL wear `editor.*`; the form or sheet around them stays system. -- **`ThemeEngine` is the single source of theme colors, and a slot exists only with a reader.** `ThemeSlot` is the one registry the decoder, encoder and theme editor enumerate, and `ThemeSlotCoverageTests` fails a slot nothing reads. Every slot is optional: one a theme leaves out draws `ThemeSlot.nativeColor`. Default Light and Default Dark override syntax only, with Xcode's Default values. +- **`ThemeEngine` is the single source of theme colors, and a slot exists only with a reader.** `ThemeSlot` is the one registry the decoder, encoder and theme editor enumerate. Nothing checks for a slot no view reads, so a change that removes a slot's last reader removes the slot. Every slot is optional: one a theme leaves out draws `ThemeSlot.nativeColor`. Default Light and Default Dark override syntax only, with Xcode's Default values. - **Every site passes what it draws natively.** A themed call site names a slot and, as its fallback, the exact system color or style it drew before, so while the active theme leaves the slot out the pixels are an unthemed app's. A default changes only on purpose, named in the PR and the CHANGELOG. - **The slot family follows what the surface shows.** Code, values, value trees and wells: `editor.*`, with tree tones from `editor.syntax.*`. Records, schema objects, diagram nodes, chart plots and states that stand in for a grid: `dataGrid.*`. Diff rows: `dataGrid.inserted`, `modified` and `deleted` over the editor background and text. A region behind panes: `window.background`. Identity, severity and status hues stay system. - **Two color tiers.** `palette[slot]` and `palette.color(_:otherwise:)` are dynamic colors over the live theme pair; use them wherever a view holds a color. `palette.color` returns a new object on every call, so never compare it to skip a write. `resolved[slot]` is the active theme's override or the system color object itself, never a flattened copy; bake it into a `CGColor` only inside the drawing view's appearance (`resolved.cgColor(_:otherwise:in:)`), and check `resolved.isOverridden(_:)` before replacing a native drawing path (row selection, caret, header). - **The carriers.** SwiftUI: `.themeBackground(_:native:)`, `.themeForeground(_:native:)`, `CodeWell`, `.valueWell(clipsContent:)`, `TextValueEditor(style: .code)`. AppKit: `ThemeTextViewStyle` on an `NSTextView`; `ThemeTableChrome` (`.records` or `.code`) with `ThemeTableRowView` rows on a content `NSTableView` or `NSOutlineView`, never a source list; `ThemeWindowBackdrop` on a content window. -- **The file list is closed.** `ThemeColorReadGuardTests` names every file that may read the theme, use a carrier, or call a helper that reads it (`TreeValueTone.color`, `HighlightedSQLTextView`, `TableProEditorTheme.make()`). A new content surface joins it in the same change; chrome never does. +- **The file list is closed.** The SwiftLint rule `theme_read_outside_content` names, in its `excluded:` list, every file that may read the theme, use a carrier, or call a helper that reads it (`TreeValueTone.color`, `HighlightedSQLTextView`, `TableProEditorTheme.make()`). A new content surface joins that list in the same change; chrome never does. - **Hold every AppKit carrier strongly**, as a stored property on its owner. Its subscription and its row views keep weak references, so a carrier held only in a local stops at the first theme change. - **A theme change has to reach the view.** `AppEvents.shared.themeChanged` fires on a theme change, a light/dark flip, an Accent or Highlight change and an Increase Contrast toggle. A SwiftUI view that reads the engine observes it (`@ObservedObject private var themeEngine = ThemeEngine.shared`), because SwiftUI caches `Color(nsColor:)` by object identity; an AppKit view that bakes a color re-bakes on it. Never call `withAlphaComponent` on a system color outside an `NSColor(name:dynamicProvider:)`: it freezes the color. - **No fixed color on selectable-row text.** A theme color does not turn white on the emphasized selection the way `.primary` does, so `.themeForeground` never goes on a container of selectable rows. Row text keeps a hierarchical style unless the theme also sets that table's selection. - **The window backdrop yields to Increase Contrast** and draws native there, since the format has no increased-contrast variant. Content slots keep the theme. -- **Never fill chrome.** No fill on the sidebar, inspector frame, toolbar or titlebar, and no `.tint` with a theme color. AppKit keeps its own selection unless the theme sets the selection slot. `PaneBackgroundGuardTests` bans `.scrollContentBackground(.hidden)`, `.listRowBackground(` and a window `backgroundColor` write outside `ThemeWindowBackdrop`. +- **Never fill chrome.** No fill on the sidebar, inspector frame, toolbar or titlebar, and no `.tint` with a theme color. AppKit keeps its own selection unless the theme sets the selection slot. SwiftLint's `pane_hidden_background` bans `.scrollContentBackground(.hidden)` and `.listRowBackground(` in app code, and `window_background_outside_backdrop` bans a window color write outside `ThemeWindowBackdrop`. diff --git a/.claude/rules/ui-lifecycle.md b/.claude/rules/ui-lifecycle.md index 18d29ccdfa..5361606c79 100644 --- a/.claude/rules/ui-lifecycle.md +++ b/.claude/rules/ui-lifecycle.md @@ -4,15 +4,16 @@ paths: - "TablePro/ViewModels/**/*" - "TablePro/Core/Services/Infrastructure/**/*" - "Packages/TableProEditor/**/*" - - "TableProUITests/**/*" --- # Views and lifecycle -- **Appearance is not lifetime.** Switching connection unparents a pane without destroying it, so `onDisappear` fires on every switch; release there only what `onAppear` rebuilds. Real teardown belongs in `dismantleNSViewController` or `ConnectionWorkspace.teardown()`. -- **The same holds on the way back.** Re-adding a pane fires `onAppear` again and restarts every `.task` and `.task(id:)`, even with an unchanged id, while `@State` survives. A load, a fetch, a cache reset or a one-time setup started there must be idempotent in its owner: skip what is already loaded, and record a load as done only when it finished, so a load cancelled by a switch runs again on return. Work that must outlive the pane, like the initial schema load, is an unstructured task the coordinator owns. -- **A popover that edits data owns its editing model**: a `@State` copy seeded in `init`, with edits reported through a callback. A popover's content does not re-render when the view presenting it does. -- **A coupled edit is one whole-value write** through a model method (`HighlightRule.selectingOperator(_:)` is the shape), never two field writes through a `Binding`: the second resolves against the same stale value and the first is lost. -- **Never put `.accessibilityIdentifier` on a SwiftUI container alone**; it replaces every descendant's identifier in that hosting tree. Pair it with `.accessibilityElement(children: .contain)` first. -- **Keep focus, selection, undo, the responder chain and IME working** after the change, and keep UI state on the main actor (no `Task.detached` to escape it). -- **SwiftUI targets macOS 13.** An API from a later release needs `if #available` and a fallback. Several views are AppKit on purpose because the SwiftUI version misbehaves; check before converting one. +- Appearance is not lifetime. Switching connection unparents a pane without destroying it, so `onDisappear` fires on every switch; release there only what `onAppear` rebuilds. Real teardown belongs in `dismantleNSViewController` or `ConnectionWorkspace.teardown()`. +- Re-adding a pane fires `onAppear` again and restarts every `.task` and `.task(id:)`, even with an unchanged id, while `@State` survives. A load, fetch, cache reset or one-time setup started there is idempotent in its owner: it skips what is loaded and records a load as done only when it finished, so a load a switch cancelled runs again on return. +- Work that must outlive the pane, like the initial schema load, is an unstructured task the coordinator owns. +- A popover that edits data owns its editing model: a `@State` copy seeded in `init`, with edits reported through a callback. A popover's content does not re-render when the view presenting it does. +- A coupled edit is one whole-value write through a model method (`HighlightRule.selectingOperator(_:)` is the shape), never two field writes through a `Binding`: the second resolves against the same stale value and the first is lost. +- Never put `.accessibilityIdentifier` on a SwiftUI container alone: it replaces every descendant's identifier in that hosting tree. Pair it with `.accessibilityElement(children: .contain)` first. How a UI test queries a leaf control is in `.claude/rules/ui-tests.md`. +- Keep focus, selection, undo, the responder chain and IME working after the change, and keep UI state on the main actor (no `Task.detached` to escape it). +- SwiftUI targets macOS 13. An API from a later release needs `if #available` and a fallback. +- A view that is AppKit on purpose says why in its type comment (`FavoritesOutlineView`: a SwiftUI `List` cannot draw an emphasized selection under a bare `NSHostingController`). Read it before converting. diff --git a/.claude/rules/ui-tests.md b/.claude/rules/ui-tests.md new file mode 100644 index 0000000000..c6e92c9988 --- /dev/null +++ b/.claude/rules/ui-tests.md @@ -0,0 +1,75 @@ +--- +paths: + - "TableProUITests/**/*" + - "TestPlans/UI.xctestplan" +--- + +# UI tests + +The UI plan holds end-to-end flows only. Swift Testing rules, isolation and shared helpers are in `.claude/rules/tests.md`. + +## What earns a UI flow + +- A UI flow is an end-to-end user flow that runs deterministically. Everything else is a unit test, preferably on the real view in a window. +- The UI plan stays at 17 minutes or less on CI. A new flow has to fit in that budget. +- Verify a click, drag or focus behavior with XCUITest, never with synthetic `CGEvent`s posted into the user's session. + +## The UI plan + +- `TestPlans/UI.xctestplan`: timeouts on, 180 s default and 300 s maximum per case, no code coverage, `retryOnFailure` with `maximumTestRepetitions` 2, so a failed case runs once more. +- A case that passed only on the retry is a flake. Fix it in the same PR, or open an issue for it. `verify.sh` lists it and CI annotates it (`.claude/skills/fix-issue/references/verification-apple.md`, Reading CI). +- The plan has no language keys: `launchApp` passes them as launch arguments. + +## The harness + +- Every suite subclasses `UITestCase` (`TableProUITests/Support/UITestCase.swift`), the only launch path that names a sandbox. The SwiftLint rule `ui_test_base_class` fails `XCUIApplication()` or `: XCTestCase` anywhere else. +- The sandbox is one folder per checkout under `$TMPDIR/TableProUITests/`, recreated for each test. Teardown terminates every launched app before deleting it, because the app writes on the way down. +- `continueAfterFailure` is false. A failed test attaches each launched app's element tree as `element-tree-`. +- An interruption monitor answers a system privacy alert, since XCTest's own handler misses macOS 26's local-network alert. +- `launchApp(environment:arguments:)` sets `TABLEPRO_UI_TESTING`, the sandbox variable, and `-AppleLanguages (en) -AppleLocale en_US`. +- A defaults override works only as a launch argument, never in `environment`: the argument domain is read first and never written. `-SUEnableAutomaticChecks YES` pins a Sparkle setting that way. +- `launchWithFixture` seeds a SQLite session on the six-table "Fixture" database (`UITestFixture`) and waits until the object browser lists something. A `TABLEPRO_UI_TEST_OPEN_*` variable in `environment` skips the session restore. +- `ConnectionSeed` defaults to the fixture SQL, Safe Mode `silent` and one `SELECT 1;` tab. A seed with `isOpen: false` is saved but never restored. +- Other launchers: `launchWithFixtureTable`, `launchWithDataFile`, `launchWithConnectionShare`, `launchOpeningURL`. + +## Driving the app + +- A command with a key equivalent: `typeShortcut(_:)` with an `AppShortcut` case. A menu click goes through XCUITest's menu traversal, which can stall 10 s. +- A command without one: `menuItem(_:under:in:)`, clicked without probing first, since resolving the item opens its menu. Database menu titles resolve under their menu; the SwiftLint rule `ui_menu_query_scope` fails `app.menuBars.menuItems[...]` for them. +- Never click an item low in a long menu. On the 1024x768 runner the menu scrolls under the pointer and the click lands on another item. Read `isEnabled` with the menu open, press Escape, then send the key equivalent. +- A contextual menu item: `contextMenuItem(_:in:)`, which takes the hittable match. Never match a contextual menu by identifier or with `app.menus.firstMatch`. +- Wait with `UITestPoll.until` or `waitToExist`, never `waitForExistence`, whose first check comes about a second late. +- Wait for `isHittable` (`waitUntilHittable`) before clicking in a pane that animates open. The element exists while it slides in, the click goes nowhere, and nothing fails until later. Assert the click's own result before what it renders. +- Scope every query to the window or sheet that owns the element. An app-rooted query walks the whole tree, which takes minutes beside a loaded grid, and can match a copy elsewhere: `app.buttons["OK"]` once matched the Touch Bar's. Windows and sheets are direct children, so use `window(_:in:)` or `children(matching:)`. +- A link-styled SwiftUI button is not in `buttons`. Query `descendants(matching: .any)` by identifier and wait on `isHittable`. +- A SwiftUI `Form` field has no title of its own. Give the leaf control an `.accessibilityIdentifier` and query that; the container rule is in `.claude/rules/ui-lifecycle.md`. Settings toggles are `Switch` elements, not checkboxes. +- The SQL editor's identifier sits on the SwiftUI wrapper and never reaches the text view: use `editorTextView(in:)`. `typeQuery(_:in:)` retypes until the text arrives, since keys typed before the editor installs its monitor are lost. +- Keyboard focus: `holdsKeyboardFocus(_:)`, never `hasFocus`, which raises through KVC. +- Scroll at a point inside the content, never the window, whose scroll lands on the title bar. `scrollUntilHittable(_:in:timeout:)` centres the element step by step. +- A synthesized scroll is not a trackpad gesture. Run a scroll test against the code before your fix once, to see it fail. +- A control in the results status bar can sit under the sidebar on the 1024 pt runner while its frame says otherwise. Switch result views from View > Result View: `showResultView(_:in:)`, `showData(in:window:)`, `showStructure(in:window:)`. + +## The grid + +- Every grid read and click goes through `FixtureGrid` (`cell`, `value`, `waitForValue`, `clickCell`, `editCell`, `clickHeader`) or `gridPoint(in:of:dy:)`. +- `FixtureGrid` matches a cell by the identifier `DataGridCellAccessibilityView`, the leaf that carries the value, not the AXCell wrapper AppKit puts around it. +- `FixtureFile.rows` and `FixtureFile.execute` read and write the fixture database directly, to set up or check data without the UI. +- Never call `click()` on a grid element. Rows and cells read as obscured by their columns, a row element has an empty frame so an offset click lands on the window's close button, and a header never becomes hittable. The app side is in `.claude/rules/data-grid.md`. +- A point offset from the grid measures from its top edge, above the header. Take `dy` from a row's frame (`row.frame.midY - grid.frame.minY`); a small constant hits the header and opens the column menu. +- `gridPoint` starts right of the object browser, which overlaps the grid on the 1024 pt runner. +- Wait for `waitForClickableRows(in:)` before a coordinate click. A grid enters the tree before its query returns, and a point taken off an empty frame posts at infinity. + +## Known failures + +- A flow that fails only on the CI runner, for a reason you cannot establish, gets `XCTExpectFailure` with the reason and an issue link, not a guessed fix. The other known-failure forms are in `.claude/rules/tests.md`. + +## Running on this Mac + +- The UI plan needs Automation Mode. When it is off, `verify.sh` reports INCONCLUSIVE; the remedy is in `.claude/skills/fix-issue/references/verification-apple.md`, INCONCLUSIVE. +- Never pass `CODE_SIGNING_ALLOWED=NO` to a local `build-for-testing` for the UI plan: macOS kills the unsigned runner before it connects. CI passes it and is fine. +- XCUITest cannot start while another TablePro is running: every case fails after a 60 s terminate timeout. Never quit the user's app. Record the run INCONCLUSIVE and let CI run it. + +## Diagnosing + +- Read the failed case's `element-tree-` attachment first. Add `print(window.debugDescription)` for one diagnostic run only: on a loaded grid it costs minutes. +- CI evidence (the recording, synthesized events, element trees): `.claude/skills/fix-issue/references/verification-apple.md`, Reading CI. diff --git a/.claude/skills/fix-issue/SKILL.md b/.claude/skills/fix-issue/SKILL.md index 06bdfd6ce1..a32517cf71 100644 --- a/.claude/skills/fix-issue/SKILL.md +++ b/.claude/skills/fix-issue/SKILL.md @@ -20,54 +20,55 @@ The user's standing requirements are the defaults: - **Full scope.** Fix every case the cause reaches, not only the reported one. - **Related defects ship too**, inside the scope boundary (Phase 1). -- **Native and documented.** The mechanism the platform documents and the design guideline behind it. No hack, no custom reimplementation, no quick win in place of the real fix. +- **Native and documented.** The mechanism the platform documents and the design guideline behind it. No hack, no custom reimplementation, no quick patch in place of the real fix. - **The reporter's proposed fix is input, not the spec.** When research shows a different native mechanism is correct, build that and say why. -The platform's guide and its area rules define done. This skill does not repeat them. +The platform's guide and its area rules define done. -Before Phase 1, read the one harness file for where you run, and only that one: `references/claude-code.md` (Claude Code) or `references/codex.md` (Codex). Talk to the user, questions included, in their language; everything in the repo is English. +Before Phase 1, read the one harness file for where you run, and only that one: `references/claude-code.md` (Claude Code) or `references/codex.md` (Codex). Talk to the user in their language, questions included; everything in the repo is English. ## Platform -TablePro is one native app per platform, each with its own code, toolchain, base branch and design guidelines. What differs between them lives in a profile, `references/platform-.md`, so this file reads the same for all of them. +TablePro is one native app per platform, each with its own code, toolchain, base branch and design guidelines. What differs lives in a profile, `references/platform-.md`, so this file reads the same for every platform. -Pick the platform from the report: the issue's Platform field, a `platform-*` label, the operating system the reporter names, or the app the user is talking about. Ask when the report does not say. Then read the one profile whose "Covers" section matches. It gives the base branch, the guide that defines done, the native standard, the verify steps and how to capture the screen. It also names two files to open only when they are needed: `references/research-.md` for the research lane, and `references/verification-.md` when a verdict needs interpreting. +Pick the platform from the report: the issue's Platform field, a `platform-*` label, the operating system the reporter names, or the app the user means. Ask when the report does not say. Then read the one profile whose Covers section matches. It gives the base branch, the guide that defines done, the native standard, the verify steps and how to capture the screen. It names two more files, each read only when needed: `references/research-.md` for the research lane, and `references/verification-.md` when a verdict or a red check needs reading. -- **No profile, no run.** A platform with no profile in this checkout cannot be fixed from it: say so and stop. The same goes for a value the profile leaves empty. Never borrow another platform's base branch, commands or guidelines to fill the gap. -- **One run per app.** The same problem in two apps is two runs, since the code, the base branch and the pull request are separate. +- **No profile, no run.** A platform with no profile in this checkout cannot be fixed from it: say so and stop. The same goes for a value the profile leaves empty. Never borrow another platform's base branch, commands or guidelines. +- **One run per app.** The same problem in two apps is two runs: the code, the base branch and the pull request are separate. +- `verify.sh` runs the steps of the machine's own platform unless `--platform ` names another. ## Autonomy -Invoking the skill authorizes the whole run: investigate, branch, implement, verify, commit, push, open the PR and keep it green. There is no approval gate. Ask only when the expected behavior has two reasonable readings, or a finding needs a product decision; ask once, recommendation first, and keep working on what does not depend on the answer. Never ask about the environment (a peer's build, a wedged test runner, a full disk): handle it and mention it in the report. +Invoking the skill authorizes the whole run: investigate, branch, implement, verify, commit, push, open the PR and keep it green. There is no approval gate. Ask only when the expected behavior has two reasonable readings or a finding needs a product decision: once, recommendation first, and keep working on what does not depend on the answer. Never ask about the environment (a peer's build, a wedged test runner, a full disk): handle it and mention it in the report. Never, even in an authorized run: force-push, rewrite published history, touch release tags, publish plugins or libraries, deploy a CloudKit schema, or discard changes you did not make. When the fix needs one of these, build everything else and name what is left for the user. ## Phase 0: Intake -1. **Read the report.** An issue: `gh issue view --repo TableProApp/TablePro --comments` (the comments often hold the real complaint). A URL: fetch it. A screen recording: `ffmpeg -i -vf "fps=1,scale=1280:-1" /frames/%03d.png` and look at the frames around the action. Open every image. -2. **Pick the platform** and read its profile ("Platform" above). -3. **Make a worktree**, because several sessions share the main checkout: +1. **Read the report.** An issue: `gh issue view --repo TableProApp/TablePro --comments`; the comments often hold the real complaint. A URL: fetch it. A screen recording: `ffmpeg -i -vf "fps=1,scale=1280:-1" /frames/%03d.png`, then look at the frames around the action. Open every image. +2. **Pick the platform** and read its profile. +3. **Make a worktree**, since several sessions share the main checkout: ```bash .claude/skills/fix-issue/scripts/worktree.sh --prune-merged # reclaims trees whose PRs are merged or closed .claude/skills/fix-issue/scripts/worktree.sh / # prints the tree's path on its last line ``` - `` is the profile's worktree base, and the tree starts from a fresh fetch of it. From here on everything runs in it (`git -C `, the tree's own `verify.sh`), and the main checkout is left alone. -4. **Write the problem statement** to `/.analysis//problem.md` (`` is the branch with `/` turned into `-`, the folder `verify.sh` logs to; it survives a compaction or a usage limit): what happens, what should happen, the smallest reproduction, the platform and the environment. A code pointer from the reporter is a hint, not a fact. + `` is the profile's worktree base, and the tree starts from a fresh fetch of it. From here on everything runs in the tree, every git call as `git -C `, and the main checkout is left alone. What the script links and when it prunes a tree: the profile's verification file, Worktrees. +4. **Write the problem statement** to `/.analysis//problem.md`. `` is the branch with `/` turned into `-`, the folder `verify.sh` logs to; the file survives a compaction or a usage limit. It holds what happens, what should happen, the smallest reproduction, the platform and the environment. A code pointer from the reporter is a hint, not a fact. 5. **Choose the mode.** *Direct* when, after reading the code yourself, you can point at the line that causes it, the correct behavior is not in doubt, and the fix stays in one subsystem without changing an interaction or adding a feature: you investigate alone. *Full* otherwise, or when unsure: the investigation fans out to lanes. A Direct run that finds the cause elsewhere switches to Full. ## Phase 1: Investigate -Answer three questions; Full mode gives each to a lane (`references/lanes.md`). +Answer three questions. Full mode gives each to a lane (`references/lanes.md`). -- **Cause** (`trace`). The code path the reported scenario actually takes, proven from the dispatch code when several paths exist; the mechanism, separate from the symptom; the blast radius, every input, type, engine and sibling surface the same cause reaches. -- **Correct behavior** (`research`, skipped for purely internal logic). For UI, the design guideline and the documented API, checked against the platform's API ground truth and its version floor. For a driver, the dependency's own source at the version we ship. For a new feature or changed interaction, also comparable clients. Sources: the profile's research file. +- **Cause** (`trace`). The code path the reported scenario takes, proven from the dispatch code when several paths exist; the mechanism, apart from the symptom; the blast radius, meaning every input, type, engine and sibling surface the same cause reaches. +- **Correct behavior** (`research`, skipped for purely internal logic). For UI, the design guideline and the documented API, checked against the platform's API ground truth and its version floor. For a driver, the dependency's own source at the version we ship. For a new feature or a changed interaction, also comparable clients. Sources: the profile's research file. - **Related defects** (`hunt`). What else is broken in the code this change touches, and where else the same cause or pattern lives. Only defects with a concrete failure scenario count. -**Scope boundary.** A verified defect ships in this PR when it sits in code the change touches, or shares its root cause or pattern, anywhere in the platform's code. A pre-existing defect outside that code goes under "Left open" in the PR with its evidence, unfixed, and so does the same defect in another platform's app. +**Scope boundary.** A verified defect ships in this PR when it sits in code the change touches, or shares its root cause or pattern, anywhere in the platform's code. A pre-existing defect outside that code goes under the PR's "Not in this PR" heading with its evidence, unfixed, and so does the same defect in another platform's app. -**Verify each related defect before it enters scope**: read the cited code, check nothing upstream prevents the failure, check the path is reachable. In Full mode one `verify` lane does the whole list. +**Verify each related defect before it enters scope**: read the cited code, check nothing upstream prevents the failure, check the path is reachable. In Full mode one `verify` lane takes the whole list. -**Measure, do not assume.** When the fix rests on how a library, a binary dependency or a framework behaves, run a probe against what we ship; the profile's research file says how to build one. An unmeasured claim is a hypothesis, including a lane's and your own. A probe that settles a fact the code hard-codes by hand gets committed where the profile says probes live. +**Measure, do not assume.** When the fix rests on how a library, a binary dependency or a framework behaves, run a probe against what we ship; the profile's research file says how. An unmeasured claim is a hypothesis, a lane's and your own alike. A probe that settles a fact the code hard-codes by hand is committed where the profile's Ship section says. ## Phase 2: Plan @@ -77,55 +78,59 @@ Write `/.analysis//plan.md`, under 80 lines, opening with a progress - **refactor or patch**, with the reason: refactor when the current shape cannot express the correct behavior without a special case, the bug comes from a wrong model, or a patch leaves the same class of bug alive; patch when the design is sound and the mistake is local; - the design: the API or pattern, its ownership boundary, the design guideline; - the scope as a checklist: files in order, blast-radius cases, each verified related defect with `file:line`; -- the tests that fail before the fix, and UI automation when a flow changed; -- CHANGELOG, docs, localization and screenshots needed. +- the tests that fail before the fix, in the targets the profile's Done section says to check first; +- the CHANGELOG, docs, localization and screenshots the change needs. -In Full mode, the `critic` lane attacks the plan; fold in what survives an evidence check. Then implement. +**Rules read.** List the files `plan.md` touches. Read `/AGENTS.md`, then every `/.claude/rules/*.md` whose row in its Area rules table matches one of them, and name them on a `Rules read:` line in `plan.md`. A file added later gets the same check before you edit it. The tree's copies are the ones that apply; a harness may have loaded another checkout's. + +In Full mode the `critic` lane attacks the plan; fold in what survives an evidence check. Then implement. ## Phase 3: Implement - When the change is visible on screen, build the fresh tree and capture the "before" shots first. -- Follow the plan's order and do the refactor it calls for. Apply the guide's per-change checklist as you write, not afterwards. -- **A large change may be implemented in parallel**: when the plan spans areas that share no files, split it across up to six subagents, each owning a disjoint file list in `plan.md`, editing only those and building nothing. You read every diff, integrate and verify. Your harness file says how. +- Follow the plan's order and do the refactor it calls for. Apply the guide's per-change checklist while you write, not afterwards. +- **A large change may be split across implementers**: when the plan spans areas that share no files, give up to six subagents a disjoint file list each in `plan.md`. Each prompt names the tree, `plan.md` and the exact files the implementer owns, and says to edit nothing else, build nothing, and finish with the files it changed. You read every diff, integrate and verify. The harness file says which agent runs them. - Competitors are research only: never name another client in code, commits, the PR, CHANGELOG or docs. ## Phase 4: Verify -Run every step through the tree's wrapper, written out in full each time (the variable shortcut does not word-split in zsh): +Run every step through the tree's wrapper, written out in full each time, since a variable holding the command does not word-split in zsh: ```bash /.claude/skills/fix-issue/scripts/verify.sh --root --no-wait ``` -The profile lists the platform's steps and when each one runs. One step is the same everywhere: `agent-docs`, when `AGENTS.md` or `.claude/` changed. The wrapper runs the steps of the machine's own platform unless `--platform ` says otherwise. +The profile lists the platform's steps and when each runs; the guide's Every change section adds the rest. -- Check the `root:` line names your tree. The verdict is `PASS`, `FAIL` or `INCONCLUSIVE` (exit 0, 1, 2), also written to `.verdict`. +- Check that the `root:` line names your tree, then read the verdict line. What each verdict means is in the guide (`AGENTS.md`, Commands). - **Verify in batches**: build once after a coherent set of edits, then re-run only a failed step. -- **Pick tests by what you changed**, not by the test files you edited, and check the executed count: on every platform a filter that matches nothing can still pass. -- **`INCONCLUSIVE` is the environment**, never a pass and never a reason to debug your code. A missing toolchain is one: the step belongs on a machine that builds that platform. The profile's verification file has the remedies. +- **Never run two builds in one tree.** `--no-wait` does not wait for a build already running there, and the second one fails on the locked build database. +- **Pick tests by what you changed**, not by the test files you edited, and compare the executed count with what you meant to run. +- **INCONCLUSIVE is the environment**: report it, never debug your code for it. The profile's verification file has the remedies. - When cheap, show the new test failing on the old code. ## Phase 5: Review -Have the complete diff reviewed by a model that did not write it; your harness file names the reviewer. In Full mode the first round adds an adversarial pass on the design. Add a security pass when the change touches credentials, SQL construction, query execution, plugin loading, MCP or AI permissions, or sync, and any extra pass the profile names. +A model that did not write the diff reviews all of it; the harness file names the reviewer. In Full mode the first round adds an adversarial pass on the design. Add a security pass when the change touches credentials, SQL construction, query execution, plugin loading, MCP or AI permissions, or sync, and any pass the profile names. -Fix what is real and inside the scope boundary without asking; a real finding about pre-existing behavior outside it goes under "Left open". Re-verify, review again, and stop after three rounds: what is still open goes under "Left open". +Fix what is real and inside the scope boundary without asking. A real finding about pre-existing behavior outside it goes under "Not in this PR". Re-verify, review again, and stop after three rounds; what is still open goes under "Not in this PR". ## Phase 6: Ship 1. **Stage explicit paths**, never `git add -A`, `.` or a directory, and compare `git -C diff --cached --stat` with what you changed. -2. **Run `scripts/check-banned-words.sh --staged`** and the same check over the PR body file. -3. **Commit, then push, in separate calls**, after `git -C branch --show-current` in its own call. The PR title is the squash commit subject: Conventional Commits, at most 72 characters. Never override the git identity. If SSH fails: `git -c credential.helper='!gh auth git-credential' push https://github.com/TableProApp/TablePro.git `. -4. **Open the PR, ready for review**: `gh pr create --repo TableProApp/TablePro --base --head --title "" --body-file `, where `` is the profile's pull request base, the body following `.github/pull_request_template.md` and readable in under a minute. It is not a log: no investigation story, no review history, no list of every test run. -5. **Screenshots**, only when the change is visible: before and after, light and dark, from a build isolated and captured the way the profile's "Screen" section says. Look at each before using it. Attach with `gh pr create --attach './after.png#After'` and reference it as `![After](./after.png)`. +2. **Run `scripts/check-banned-words.sh --staged`**, and the same script over the PR body file. +3. **Commit, then push, in separate calls**, after `git -C branch --show-current` in a call of its own. The PR title becomes the squash commit subject; its format, types and scopes are in the top comment of `.github/pull_request_template.md`. Never override the git identity. If SSH fails: `git -c credential.helper='!gh auth git-credential' push https://github.com/TableProApp/TablePro.git `. +4. **Open the PR, ready for review**: `gh pr create --repo TableProApp/TablePro --base --head --title "" --body-file `, where `` is the profile's pull request base. The body follows `.github/pull_request_template.md` and reads in under a minute. It is not a log: no investigation story, no review history, no list of every test run. +5. **Screenshots**, only when the change is visible: before and after, light and dark, from an isolated build captured the way the profile's Screen section says. Look at each one before using it. Attach with `gh pr create --attach './after.png#After'` and reference it as `![After](./after.png)`. ## Phase 7: Land -Watch the checks until they finish (the harness file says how, without spending tokens): +Watch the checks until they finish. `gh pr checks --repo TableProApp/TablePro --watch --interval 60` exits when every check is done, nonzero when one failed; `gh run view --log-failed` prints a failed job's log. The harness file says how to wait without spending tokens. -- a check this PR broke: fix it, re-verify, push a new commit; -- a merge conflict: `git -C fetch origin && git -C merge origin/`, resolve, re-run the profile's first step (the base may have added files), re-verify, push; never rebase or force-push; -- a check that is not this PR's (red on the base branch too, a runner fault), or one still queued after 45 minutes with no job started: leave it and report it. +- A red check: read it with the profile's verification file (Reading CI) before changing anything. +- A check this PR broke: fix it, re-verify, push a new commit. +- A merge conflict: `git -C fetch origin && git -C merge origin/`, resolve, re-run the profile's first step (the base may have added files), re-verify, push. Never rebase or force-push. +- A check that is not this PR's (red on the base branch too, a runner fault), or one still queued after 45 minutes with no job started: leave it and report it. Stop after two rounds of CI fixes. Never merge the PR. diff --git a/.claude/skills/fix-issue/references/claude-code.md b/.claude/skills/fix-issue/references/claude-code.md index f2cd3febb4..3a2e0ffa24 100644 --- a/.claude/skills/fix-issue/references/claude-code.md +++ b/.claude/skills/fix-issue/references/claude-code.md @@ -1,12 +1,11 @@ # Running under Claude Code -How `SKILL.md` maps onto Claude Code. - ## Tracking and asking - Keep a task list with the phases this run uses (`TodoWrite` or the task tools, when present), and progress in `plan.md` either way. -- Ask with `AskUserQuestion`, recommendation first, only for the cases in "Autonomy". -- The Bash tool's working directory resets to the main checkout between calls: start every command for the tree with `cd &&` or pass `-C `. +- Ask with `AskUserQuestion`, recommendation first, only for the cases in Autonomy. +- The Bash tool's working directory resets to the main checkout between calls: start every command for the tree with `cd &&`, or pass `-C `. +- Claude Code injects `AGENTS.md` and path rules from the session's checkout, which can be on another branch than the tree. The tree's copies, read in `SKILL.md` Phase 2 (Rules read), win. ## Lanes @@ -78,24 +77,24 @@ with a findings array holding only the real ones. return { results, verdict } ``` -- Fill in every `<...>` and hardcode it: the tool's `args` can arrive empty. `meta` is a pure literal; `Date.now()`, `Math.random()` and an argless `new Date()` throw. -- Return raw results; an `agent(...).then(...)` chain inside `parallel()` has come back empty. -- When a digest is missing or the return looks empty, read the lane's report file, or `journal.jsonl` for the run. -- A lane that stalls is re-run as one background `Agent` with the same brief. Never `SendMessage` a lane that is still running: that starts a second copy. -- Run the `critic` lane as one background `Agent`; long critic turns trip the workflow's stall watchdog. +- Fill in every `<...>` and hardcode it: the tool's `args` can arrive empty. `meta` is a pure literal; `Date.now()`, `Math.random()` and an argless `new Date()` throw there. +- Return raw results. An `agent(...).then(...)` chain inside `parallel()` has come back empty. +- When a digest is missing or the return looks empty, read the lane's report file, or the run's `journal.jsonl`. +- Re-run a stalled lane as one background `Agent` with the same brief. Never `SendMessage` a lane that is still running: that starts a second copy. +- Run the `critic` lane as one background `Agent`, since a long critic turn trips the workflow's stall watchdog. - Afterwards, `git -C status --short` confirms no lane edited the tree. -Parallel implementers (`SKILL.md` Phase 3) are background `Agent`s in one message, each prompt naming the tree, `plan.md` and the exact files it owns, and saying to edit nothing else, build nothing, and finish with the files it changed. +Parallel implementers (`SKILL.md`, Phase 3) are background `Agent`s in one message. ## Long commands -Run the profile's build and test steps, the Codex review and the CI watch with `run_in_background: true`. You are re-invoked when each exits, so never poll in between. Keep the quick steps (generating the project, lint) in the foreground, and never run two builds in one tree. - -CI watch: `gh pr checks --repo TableProApp/TablePro --watch --interval 60` exits when every check finishes, non-zero on a failure; `gh run view --log-failed` gives the log. A background command stops after two hours; if checks are still queued then, Phase 7's backlog rule applies. +- Run the build and test steps, the Codex review and the Phase 7 watch with `run_in_background: true`. You are re-invoked when each exits, so never poll in between. +- Keep the quick steps, `generate` and `lint`, in the foreground. +- A background command stops after two hours. If checks are still queued then, Phase 7's backlog rule applies. ## Review: Codex -Run the Codex plugin's companion from inside the tree (`cd &&` on every call; its job state is keyed by the checkout): +Run the Codex plugin's companion from inside the tree (`cd &&` on every call), since its job state is keyed by the checkout: ```bash CODEX="$(ls -1d "$HOME"/.claude/plugins/cache/openai-codex/codex/*/scripts/codex-companion.mjs | sort -V | tail -1)" @@ -107,9 +106,9 @@ node "$CODEX" result - `review` every round; `adversarial-review` in Full mode on the first round. One at a time, each in the background with `--wait`. - Read the findings with `result`, never from stdout, which shows only progress whether the job succeeded or died. `status` says whether it finished. -- When `status` shows Codex cannot review (not installed, out of credits, usage limit), use `Skill(code-review)` and say so in the report. A setup or login error goes back to the user with `/codex:setup`. +- When `status` shows Codex cannot review (not installed, out of credits, a usage limit), use `Skill(code-review)` and say so in the report. A setup or login error goes back to the user with `/codex:setup`. - The security pass is a background `Agent` told to review `git -C diff origin/...HEAD` (the profile's pull request base) for injection, credential exposure, authorization and unsafe execution, reporting only high and medium findings with an exploit scenario. `Skill(security-review)` reads the main checkout, not the tree. ## Screenshots -The profile's "Screen" section names the tools that launch, drive and capture the app. Open each shot with `Read` before using it. +The profile's Screen section names the tools that launch, drive and capture the app. Open each shot with `Read` before using it. diff --git a/.claude/skills/fix-issue/references/codex.md b/.claude/skills/fix-issue/references/codex.md index f167ab5c7f..b1ff130a84 100644 --- a/.claude/skills/fix-issue/references/codex.md +++ b/.claude/skills/fix-issue/references/codex.md @@ -1,12 +1,12 @@ # Running under Codex -How `SKILL.md` maps onto Codex. Codex loads the skill through `.agents/skills/fix-issue` and the project guide from `AGENTS.md`. +Codex loads the skill through `.agents/skills/fix-issue` and the project guide from `AGENTS.md`. ## Rules and tracking -- Codex loads the root `AGENTS.md` and nothing else by itself. When the profile's "Done" section names another guide, read it. Before editing, read every rule under `.claude/rules/` whose `paths:` frontmatter matches a file you will touch (the guide lists them). +- Codex loads the root `AGENTS.md` of the checkout it started in and nothing else by itself. The tree's guide and rules come from `SKILL.md` Phase 2 (Rules read). When the profile's Done section names another guide, read it. - Keep progress in `plan.md`, and in the plan or goal tool your session offers, if any. -- Ask with `request_user_input`, recommendation first, only for the cases in "Autonomy". +- Ask with `request_user_input`, recommendation first, only for the cases in Autonomy. ## Lanes @@ -19,20 +19,20 @@ Spawn the lanes in parallel with `spawn_agent`, collect them with `wait_agent`, | verify, critic, review | `adversarial_reviewer` | | ABI review | `plugin_abi_reviewer` | -Each message names the brief (`.claude/skills/fix-issue/references/lanes.md`: the shared rules and the lane's section), the platform profile, the tree, `problem.md` or `plan.md`, and the subsystem. These agents are read-only: a lane returns its report as its final message and you save it to `/.analysis//lanes/.md`, and a probe that needs compiling is yours to run. +Each message names the brief (`.claude/skills/fix-issue/references/lanes.md`: the shared rules and the lane's section), the platform profile, the tree, `problem.md` or `plan.md`, and the subsystem. These agents are read-only: a lane returns its report as its final message, you save it to `/.analysis//lanes/.md`, and you run any probe that needs compiling. -Parallel implementers (`SKILL.md` Phase 3) use the default agent type, since the project agents cannot write. Each message names the tree, `plan.md` and the exact files it owns, and says to edit nothing else, build nothing, and finish with the files it changed. +Parallel implementers (`SKILL.md`, Phase 3) use the default agent type, since the project agents cannot write. ## Commands -- A build can write outside the tree (the profile's verification file says where), `gh` and `git push` need the network, and `verify.sh` logs under `.analysis/`. When the sandbox refuses one, request escalation for that command; never skip the step. -- Give the profile's build and test steps a timeout of at least 30 minutes, and never run two builds in one tree. -- CI: `gh pr checks --repo TableProApp/TablePro --watch --interval 60` blocks until the checks finish, or re-run `gh pr checks ` every few minutes; `gh run view --log-failed` gives a failed job's log. +- A build writes outside the tree (the profile's verification file, Build, says where); `gh` and `git push` need the network; `verify.sh` logs under `.analysis/`. When the sandbox refuses one of these, request escalation for that command. Never skip the step. +- Give the build and test steps a timeout of at least 30 minutes. +- The Phase 7 watch either blocks with `--watch`, or you re-run `gh pr checks --repo TableProApp/TablePro` every 60 s. ## Review -Spawn `adversarial_reviewer` over the diff pinned to commit SHAs captured before spawning (`git diff `), since a symbolic range can resolve to nothing once the branch merges. Give it the mechanism the fix rests on in one sentence, and ask for security coverage when Phase 5 calls for it. Add `plugin_abi_reviewer` when the profile calls for the ABI pass. The report says the reviewer was a Codex agent. +Spawn `adversarial_reviewer` over the diff pinned to commit SHAs captured before spawning (`git diff `), since a symbolic range can resolve to nothing once the branch merges. Give it the mechanism the fix rests on in one sentence, and ask for security coverage when Phase 5 calls for it. Add `plugin_abi_reviewer` when the profile calls for the ABI pass. The report says a Codex agent reviewed the change. ## Images -Look at screenshots and video frames with `view_image`. The profile's "Screen" section names the tools that launch, drive and capture the app. +Look at screenshots and video frames with `view_image`. The profile's Screen section names the tools that launch, drive and capture the app. diff --git a/.claude/skills/fix-issue/references/lanes.md b/.claude/skills/fix-issue/references/lanes.md index 66d360d093..2ca17404be 100644 --- a/.claude/skills/fix-issue/references/lanes.md +++ b/.claude/skills/fix-issue/references/lanes.md @@ -1,39 +1,39 @@ # Lane briefs -The lead hands each lane the path of `/.analysis//problem.md` (and, for `critic`, `plan.md`), the platform profile (`references/platform-.md`), the subsystem in play, and the one section of this file it runs. A lane reads its own section, the rules below and the profile, which says what the guide, the native standard and the ground truth are on this platform. +The lead hands each lane the path of `/.analysis//problem.md` (and `plan.md` for `critic`), the platform profile (`references/platform-.md`), the subsystem in play, and the one section of this file it runs. A lane reads its section, the rules below and the profile, which names the guide, the native standard and the ground truth on this platform. ## Rules for every lane -- **Read-only on the repo.** Never edit, `git restore`, `git checkout`, stash or reset anything in the checkout. An experiment runs on a copy in the scratchpad. A prompt that says "read-only" is not a sandbox, so this rule is what keeps a lane from reverting the lead's work. -- **Evidence or silence.** Code claims carry a `path/file:123` you opened. Platform claims carry a doc URL or an exact symbol, plus the line of the platform's API ground truth when the question is whether an API exists. Dependency claims carry the line of the dependency's own source or a probe's measured output. Competitor claims carry the source you read. -- **Label every claim** `confirmed` (opened, measured or cited) or `inferred` (reasoned but unproven). An honest "could not establish" beats a confident guess, because a wrong confirmed claim is acted on. -- **Report shape.** Write the full report wherever the harness file says, then return a digest of at most 15 lines: a verdict sentence, the confidence label, up to 8 anchors (`ref`: what is there), the unknowns, and your recommendation. The lead opens the full report only where the plan depends on it, so the digest is not a summary of everything you found. +- **Read-only on the repo.** Never edit, `git restore`, `git checkout`, stash or reset anything in the checkout. An experiment runs on a copy in the scratchpad. A prompt that says "read-only" is not a sandbox; this rule is what keeps a lane from reverting the lead's work. +- **Evidence or silence.** A code claim carries a `path/file:123` you opened. A platform claim carries a doc URL or an exact symbol, plus the line of the platform's API ground truth when the question is whether an API exists. A dependency claim carries the line of the dependency's own source or a probe's measured output. A competitor claim carries the source you read. +- **Label every claim** `confirmed` (opened, measured or cited) or `inferred` (reasoned, unproven). An honest "could not establish" beats a confident guess, because a wrong confirmed claim gets acted on. +- **Report shape.** Write the full report where the harness file says, then return a digest of at most 15 lines: a verdict sentence, the confidence label, up to 8 anchors (`ref`: what is there), the unknowns, and your recommendation. The lead opens the full report only where the plan depends on it, so the digest is not a summary of everything you found. ## trace Find the cause of the reported behavior. 1. The files, types and functions involved, with `file:line`. -2. The real call path the reported scenario takes: what triggers it, what state flows through it, and where the wrong behavior starts. When several paths reach the behavior, prove which one this scenario takes from the dispatch code. -3. The mechanism, separated from the symptom. When the current structure cannot express the correct behavior without a special case, say so and why. +2. The real call path the reported scenario takes: what triggers it, what state flows through it, and where the wrong behavior starts. When several paths reach the behavior, prove from the dispatch code which one this scenario takes. +3. The mechanism, apart from the symptom. When the current structure cannot express the correct behavior without a special case, say so and why. 4. The blast radius: every other input, type, engine, state or sibling surface the same cause reaches. 5. The invariants in the guide and its area rules that this area touches, and whether they already record this area breaking before. -6. The existing tests here and where a real regression test belongs. The profile's verification file lists the tests that look present and never run. -7. Where the fix lands, among the targets the profile's "Covers" section lists. +6. The existing tests here and where a real regression test belongs. The profile's verification file lists tests that look present and never run. +7. Where the fix lands, among the targets the profile's Covers section lists. ## research Establish the correct behavior from the authoritative source. The profile's research file (`references/research-.md`) lists where to look and what the ground truth is. - **UI or interaction**: the design guideline section, quoted and linked ("the guidelines say so" without a quote is not evidence); the native API, named exactly, with its documented behavior, its availability against the platform's version floor and its gotchas; any standard control that already does this. Confirm every symbol against the API ground truth, which settles what a web page only describes. -- **Driver or dependency**: the dependency's own source at the version we actually ship, the doc comments for each symbol in play, and what a call returns when it cannot do the job. Where the source does not settle it, build a probe against what we link, in the scratchpad, and report the output verbatim. A measurement outranks every document. +- **Driver or dependency**: the dependency's own source at the version we ship, the doc comments for each symbol in play, and what a call returns when it cannot do the job. Where the source does not settle it, build a probe against what we link, in the scratchpad, and report its output verbatim. A measurement outranks every document. - **New feature or changed interaction**: how comparable clients behave, starting with the one most users arrive from. Search each one's docs and changelog for the surface in its own words, read the source of the open-source ones, write one line per client marked confirmed or inferred, then say where they agree: agreement across three clients is a strong signal of what users expect. Where a client and the platform's design guidelines disagree, the guidelines win. This is input to the design, never text for the repo. End with a concrete recommendation: the API, the states, the example strings. ## hunt -Find what else is wrong, not the reported bug. Everything you report that survives verification ships in this PR, so report only defects, each with a `file:line` and a failure scenario someone could actually hit. +Find what else is wrong, not the reported bug. Everything you report that survives verification ships in this PR, so report only defects, each with a `file:line` and a failure scenario someone could hit. Look in the subsystem the fix touches, then wherever the same cause or pattern lives: @@ -43,13 +43,13 @@ Look in the subsystem the fix touches, then wherever the same cause or pattern l - work done twice, or results merged from two executions whose order is not guaranteed - forks of this logic that have drifted apart -For each finding give: title, `file:line`, evidence, failure scenario, and whether the primary fix is wrong or incomplete without it. Style, naming and "I would have written it differently" do not count. Three real findings beat fifteen speculative ones, and an empty list is a real answer. +For each finding give its title, `file:line`, evidence, failure scenario, and whether the primary fix is wrong or incomplete without it. Style, naming and "I would have written it differently" do not count. Three real findings beat fifteen speculative ones, and an empty list is a real answer. ## verify Try to refute each finding in the list you are given. Default to refuted when unsure. -For each one: read the cited code and its surroundings, check that nothing upstream already prevents the failure, that the path is reachable in the shipping app, and that no test or documented contract contradicts it. Measure it when measuring settles it. Return one row per finding, `real` or `refuted`, with how to reproduce it or why it fails. A finding is `real` only when you can state the reproduction. +For each one, read the cited code and its surroundings, and check that nothing upstream already prevents the failure, that the path is reachable in the shipping app, and that no test or documented contract contradicts it. Measure it when measuring settles it. Return one row per finding, `real` or `refuted`, with how to reproduce it or why it fails. A finding is `real` only when you can state the reproduction. ## critic diff --git a/.claude/skills/fix-issue/references/platform-apple.md b/.claude/skills/fix-issue/references/platform-apple.md index 7cfbdd01b9..4461e56466 100644 --- a/.claude/skills/fix-issue/references/platform-apple.md +++ b/.claude/skills/fix-issue/references/platform-apple.md @@ -4,7 +4,10 @@ The macOS app and the iOS app, with everything they build from. Every profile ha ## Covers -A report from macOS or iOS. The code is the repository root: `TablePro/`, `Plugins/`, `Packages/`, `Native/`, `TableProMobile/`, the tests in `TableProTests/` and `TableProUITests/`, and the build scripts under `scripts/`. A fix lands in the app, a bundled plugin, a registry-only plugin (name its target) or `TableProMobile`. +- A report from macOS or iOS. +- Code: `TablePro/`, `Plugins/`, `Packages/`, `Native/`, `TableProMobile/` and the build scripts in `scripts/`. +- Tests: `TableProTests/`, `TableProUITests/`, `Plugins/*Tests/`, `Packages/*/Tests/` and `TableProMobile/TableProMobileTests/`. The plans are in `TestPlans/`. +- A fix lands in the app, a bundled plugin, a registry-only plugin (name its target), a package or `TableProMobile/`. ## Base @@ -13,39 +16,43 @@ A report from macOS or iOS. The code is the repository root: `TablePro/`, `Plugi ## Done -`AGENTS.md` and the `.claude/rules/` files whose `paths:` match what you edit. The per-change checklist is its "Every change" section. Codex loads `AGENTS.md` and not the rules, so under Codex read the matching rules yourself. +- The tree's `AGENTS.md` and every `.claude/rules/` file its Area rules table matches to what you edit (`SKILL.md`, Phase 2, Rules read). The per-change checklist is its Every change section. +- Before naming a test in `plan.md`, read `.claude/rules/tests.md` (Where a test goes): a path rule loads only once a matching file is open. Add a UI flow only when `.claude/rules/ui-tests.md` (What earns a UI flow) says so. ## Native standard -The AppKit or SwiftUI mechanism Apple documents, and the Human Interface Guidelines rule behind it. The version floor is the deployment target, macOS 13: an API introduced later needs an `if #available` branch, and the plan names the fallback. - -Sources, the API ground truth and how to build a probe: `references/research-apple.md`. +- The AppKit or SwiftUI mechanism Apple documents, and the Human Interface Guidelines rule behind it. +- The version floor is the deployment target: macOS 13 (`project.yml`) and iOS 18 (`TableProMobile/project.yml`). An API introduced later needs an `if #available` branch, and the plan names the fallback. +- Sources, the API ground truth and how to build a probe: `references/research-apple.md`. ## Verify -`verify.sh` runs these steps on a Mac without `--platform`: - -| Step | When | -| --- | --- | -| `generate` | a new tree; a new, moved or deleted file; `project.yml` | -| `build`, `test ...` | always | -| `uitest ...` | a user flow changed | -| `package [filter]` | `Packages/` changed | -| `ios ...` | `TableProMobile/` or a file it shares changed | -| `plugins` | `Plugins/` changed | -| `abi ` | `TableProPluginKit` changed (commit first) | -| `lint ...` | every changed Swift file | -| `l10n` | strings or plugin messages changed | -| `docs []` | a paired docs change, or a change to a menu, shortcut, driver list or docs.tablepro.app link | - -- The first step after a merge from the base is `generate`. -- Pick suites by the types you changed: `grep -rl TableProTests`. +`verify.sh` runs these steps on a Mac without `--platform`. `build`, `test` and `lint` take the arguments in `AGENTS.md`, Commands. + +| Step | Arguments | When | +| --- | --- | --- | +| `generate` | none | a new tree; a new, moved or deleted file; a `project.yml` change; first after a merge from the base. Runs `scripts/generate-project.sh` for both projects | +| `build` | AGENTS | always | +| `test` | AGENTS | always, naming the suites that own the types you changed | +| `uitest` | `[Test...]`; none runs the whole UI plan | an end-to-end flow changed or was added | +| `package` | ` [regex]` | `Packages/` changed and you want the whole package; `test ` already routes a package suite | +| `ios` | ` [Suite...]`, at least one | `TableProMobile/` or a file it compiles changed | +| `plugins` | none | anything under `Plugins/` changed. Builds `AllPlugins`, building the HANA helper first when it is missing and Go is installed | +| `abi` | `` | PluginKit changed: after the commit, before the push (`.claude/rules/plugin-system.md`, ABI) | +| `lint` | AGENTS | every changed Swift file | +| `l10n` | none | strings, or plugin or package messages, changed. Passes only when every plugin and package string is in both the macOS and iOS catalogs, and prints the `--add` command for each target that failed | +| `docs` | `[docs-checkout]`, default `tablepro-docs` beside the main checkout | AGENTS Every change 4 applies, or a menu, shortcut, driver list or docs.tablepro.app link changed | + +- `agent-docs` runs on every platform; AGENTS Every change 5 says when. +- Pick tests by the types you changed, not the test files you edited: `verify.sh` finds a type name in every test target of the tree. - Extra review pass: the ABI pass when PluginKit changed. -- Traps and remedies: `references/verification-apple.md`. +- Test names, verdicts, the remedies `verify.sh` does not print, and red CI checks: `references/verification-apple.md`. ## Screen -Launch a Debug build with `TABLEPRO_UI_TEST_SANDBOX` set to a scratch directory, so it never touches real connections. Drive it with `osascript` and capture with `screencapture`; Screen Recording must be granted to whatever runs them. A registry-only plugin cannot load in a sandboxed Debug build; say so when that blocks a shot. Reuse the shots in `../tablepro-docs/images/` when a page shows that surface. +- Launch a Debug build with `TABLEPRO_UI_TEST_SANDBOX` set to a scratch directory, so it never touches the real connections, defaults or keychain. Drive it with `osascript` and capture with `screencapture`; Screen Recording must be granted to whatever runs them. +- A registry-only plugin from the `plugins` step is signed ad hoc, and the plugin signature check refuses it. The app then installs the published plugin in its place, so the shot shows the old behavior. Launch the Debug build with `TABLEPRO_ALLOW_UNSIGNED_PLUGINS=1`, or re-sign the copy in `/TablePro/Plugins/` with the identity `codesign -dvv` prints for the app. +- Reuse the shots in `../tablepro-docs/images/` when a docs page shows that surface. ## Ship diff --git a/.claude/skills/fix-issue/references/research-apple.md b/.claude/skills/fix-issue/references/research-apple.md index e2a002ae2c..324ed54c81 100644 --- a/.claude/skills/fix-issue/references/research-apple.md +++ b/.claude/skills/fix-issue/references/research-apple.md @@ -1,23 +1,23 @@ # Research sources: Apple -Where to find the correct behavior on macOS and iOS, and what counts as evidence for it. The method is in the research brief (`references/lanes.md`); this file is the sources. +The method is in the research brief (`references/lanes.md`); this file is the sources. ## Apple platform -- **SDK interface, the API ground truth: whether an API exists and what its signature is**: +- **SDK interface, the API ground truth** for whether an API exists and what its signature is: `$(xcrun --sdk macosx --show-sdk-path)/System/Library/Frameworks/.framework/Modules/.swiftmodule/arm64e-apple-macos.swiftinterface`. - Grep it for the symbol and its `@available`. It is what the compiler reads, so web docs give the intent and this file settles the facts. + For iOS, `--sdk iphoneos` and `arm64e-apple-ios.swiftinterface`. Grep it for the symbol and its `@available`. The compiler reads this file, so a web page gives the intent and this file settles the facts. - **Human Interface Guidelines**: `https://developer.apple.com/design/human-interface-guidelines`. For a database client the macOS sections that matter most are windows, panels, sheets, toolbars, sidebars, menus, tables and lists, and selection. Quote the rule; "the HIG says so" without a quote is not evidence. -- **AppKit** (`https://developer.apple.com/documentation/appkit`) and **SwiftUI** (`https://developer.apple.com/documentation/swiftui`). Check whether a SwiftUI modifier already does the job before dropping to AppKit, and check the reverse too: several TablePro views are AppKit because the SwiftUI version misbehaves, and the area rules record why. -- **Version floor**: the deployment target is macOS 13. An API from macOS 14, 15 or 26 needs an `if #available` branch, and the plan names the fallback. Name the modern API, and when the only option is deprecated, name its replacement. +- **AppKit** (`https://developer.apple.com/documentation/appkit`) and **SwiftUI** (`https://developer.apple.com/documentation/swiftui`). Check whether a SwiftUI modifier already does the job before dropping to AppKit, and check the reverse too: a TablePro view that is AppKit on purpose says why in its type comment. +- **Version floor**: `references/platform-apple.md`, Native standard. Name the modern API, and when the only option is deprecated, name its replacement. - **TablePro's own docs** in TableProApp/docs (`../tablepro-docs`): a fix must not contradict what users have been told. ## Dependencies The source is the vendored header and the library we ship, not a web page. -- Headers live under `Plugins/*/C*/include/` and `Libs/`. State the version we actually link, because a build script has named a version we never shipped. -- Quote the header's doc comments for each symbol in play, especially deprecation notes and what a call returns when it cannot do the job. +- Headers live under `Plugins/*/C*/include/` and `Libs/`. State the version we link, since a build script has named a version we never shipped. +- Quote the header's doc comments for each symbol in play, above all deprecation notes and what a call returns when it cannot do the job. - Where the header does not settle it, build a probe and report its output verbatim: C compiled against `Libs/*.a`, a `swiftc` harness, or the vendored CLI. ## Comparable clients @@ -33,4 +33,4 @@ A reporter describing how something "should" work is usually describing the clie | Beekeeper Studio | Open source | `github.com/beekeeper-studio/beekeeper-studio` | | DBeaver | Widest driver and dialect coverage, for engine quirks | `github.com/dbeaver/dbeaver` | -The changelog says when and why a behavior changed. None of these apps can be run from here, so every claim about one is confirmed from a source you read or marked inferred. +A changelog says when and why a behavior changed. None of these apps can be run from here, so every claim about one is confirmed from a source you read or marked inferred. diff --git a/.claude/skills/fix-issue/references/verification-apple.md b/.claude/skills/fix-issue/references/verification-apple.md index 46b845d38c..e63300c7be 100644 --- a/.claude/skills/fix-issue/references/verification-apple.md +++ b/.claude/skills/fix-issue/references/verification-apple.md @@ -1,59 +1,113 @@ -# Verification traps: Apple +# Verification on Apple -Read this when a `verify.sh` verdict needs interpreting on macOS or iOS, or before writing a UI test. The usage line is `verify.sh [--platform ] [--root ] [--run ] [--no-wait] [--offline] [args]`, and on a Mac the platform is this one unless the option names another. `--run ` puts logs in `/logs`. Without `--root`, the root is the checkout the current directory is in, falling back to the one the script lives in. `verify.sh tail [n]` re-reads a stored log, and `verify.sh parse ` prints the verdict the live run wrote to `.verdict`, re-deriving it from the log only when no receipt exists. +Read this when a verdict, a test name or a red CI check needs explaining. The commands, the verdict words and the verdict rules are in `AGENTS.md`, Commands; when each step runs is in `references/platform-apple.md`, Verify. -## Why the wrapper, always +## Usage -A raw `xcodebuild` failure comes back as a head-and-tail excerpt of about 10,000 characters with no log path, so the one run you need in full is the one you cannot get back. The wrapper also exports `DEVELOPER_DIR` (a Command Line Tools `xcode-select` has no `xcodebuild` and no `sourcekitd`), names the project explicitly, waits for another `xcodebuild` in the same checkout, and checks failing cases against both quarantine files and the known environment failures before it calls a run red. +- `verify.sh [options] [args]`, plus `verify.sh tail [lines]` and `verify.sh parse `. +- Options go before or after the step: `--platform `, `--root `, `--run `, `--no-wait`, `--offline`. An unknown option is a usage error. +- The platform comes from `uname -s`, and Darwin is `apple`. A platform named with `--platform` that has no backend exits 3; a default one with no backend is INCONCLUSIVE. +- The root is the checkout your shell is in when it shares the script's git common directory, else the checkout the script lives in. `--root` overrides both. Check the `root:` line. +- `--offline` adds `-disableAutomaticPackageResolution -onlyUsePackageVersionsFromResolvedFile` to `build`, `plugins`, `test` and `uitest`. +- The Apple steps live in `.claude/skills/fix-issue/scripts/lib/platform-apple.sh`, whose header lists each one with its arguments. + +## Why the wrapper + +- A raw `xcodebuild` failure comes back as a head-and-tail excerpt of about 10,000 characters with no log path, so the one run you need in full is lost. +- Without `DEVELOPER_DIR` the wrapper takes the selected Xcode, then `/Applications/Xcode.app`, then `/Applications/Xcode-beta.app`. A Command Line Tools selection has no `xcodebuild` and no `sourcekitd`. +- It names the project explicitly and waits for a build of the same checkout (Worktrees). + +## Naming tests + +- A bare `Suite` is a Swift type name: a `struct`, `class`, `enum`, `actor` or `extension` declared in a `.swift` file under any folder whose name ends in `Tests`, tracked or untracked. It is not the file name, the `@Suite` display name or a `@Test` function. +- A nested Swift Testing suite is `/`. +- A target the Unit or UI plan lists runs through that plan. A package test target no plan lists runs under `swift test` in its package. +- A name no target declares exits 3 before anything builds. +- A name declared in two targets exits 3 and asks for `/`. +- A name declared only in a folder no plan or package lists exits 3; for `TableProMobileTests` it names `verify.sh ios`. +- A Unit test passed to `uitest`, or a UI test passed to `test`, exits 3 and names the right step. So does a `swift test` suite passed to `uitest`. +- `test` with no argument runs the Unit plan only, never the TableProCore or TableProOracle test targets; those run with `verify.sh package `. +- A `swift test` suite runs as `swift test --filter` on `.` in its package. One step can run a plan and several packages, and the worst verdict wins. +- A raw `xcodebuild -only-testing` with a name that is not a type runs 0 cases and still prints `TEST SUCCEEDED`. `verify.sh` and CI fail that run; a raw command does not. +- In zsh an unquoted `$SUITES` holding several names is one argument: write the names out, or use `${=SUITES}`. + +## Verdicts + +- A test verdict comes from the result bundle or the xunit file, never the console banner. +- The result-bundle rules, first match wins: + 1. nothing executed and the bundle holds build errors: FAIL, "the test build failed"; + 2. nothing executed and `xcodebuild` exited nonzero: FAIL; + 3. a failed case: FAIL; + 4. a runner failure outside any test ("System Failures"): FAIL; + 5. a nonzero exit with no failed case: FAIL; + 6. a filter that ran nothing, or nothing executed at all: FAIL; + 7. otherwise PASS. +- The xunit rules: a nonzero exit or a failed case is FAIL, nothing executed is FAIL, else PASS. `.xml` and `-swift-testing.xml` both count. +- Skipped cases do not count as executed. Expected failures are counted apart from failures. +- A case that passed only on the UI plan's retry is listed under "passed only on retry, a flake to fix" and does not fail the step. What to do with it: `.claude/rules/ui-tests.md`, The UI plan. +- The step prints executed, passed, failed and skipped counts and one count per target. Compare executed with what you meant to run. +- A build step whose log lacks `** BUILD SUCCEEDED **` is FAIL, or INCONCLUSIVE when a known environment cause explains it. + +## INCONCLUSIVE + +- INCONCLUSIVE always names a cause outside the change. A test run gets it only when nothing executed and a known cause explains it; a run that executed tests is PASS or FAIL. +- The script prints each cause with its remedy. Two need more than it prints. +- "The test runner hung before establishing connection" twice in a row is a wedged `testmanagerd`, and every later test run on the Mac hangs the same way. When `pgrep -f Developer/usr/bin/xcodebuild` prints nothing, run `kill -9 $(pgrep testmanagerd)`. SIGTERM does not stop it; launchd starts a fresh one on the next run. +- "Timed out while enabling automation mode" means Automation Mode is off. Run an untouched suite as a control (`verify.sh uitest `) and grep its log for `automation mode`. If it fails the same way, take a `screencapture -x` to see whether the "Enable UI Automation" prompt is up; scripting cannot see it. +- Only the user turns Automation Mode on: by answering that prompt with an admin password, or by running `sudo automationmodetool enable-automationmode-without-authentication` once, after which XCTest enables it with no prompt. `automationmodetool` prints which of the two this Mac needs. Unit tests, lint and screenshots keep working meanwhile; say in the PR that CI runs the UI flow. + +## Logs and files + +- Logs go to `/.analysis//logs/`, or to `/logs/` with `--run `. `.analysis/` is gitignored. +- A log is `[-]-.log` (`build-TablePro-.log`, `test--.log`), with a numeric suffix when two start in the same second. +- Each run writes `.verdict`, and `verify.sh parse ` prints it back. +- A test run also writes `.filters`. A plan run keeps `.xcresult` only when it did not pass, since a failing bundle runs to about 150 MB. A `swift test` run writes `.xml` and `-swift-testing.xml`. ## Build -- **A step whose tool is not installed reports `INCONCLUSIVE`** and runs nothing (`xcodebuild`, `xcodegen`, `swiftlint`, `swift`). The step belongs on a Mac that has it. -- **`xcodebuild` writes outside the tree**, to `~/Library/Developer/Xcode/DerivedData`, which a sandbox has to allow. -- **The `TablePro` scheme builds only the bundled plugins.** A compile error in a registry-only plugin still ends `BUILD SUCCEEDED`, so run `verify.sh plugins` whenever `Plugins/` changed. -- **`Could not resolve package dependencies`** means SwiftPM tried the network: re-run with `--offline`. -- **`Unable to open base configuration reference file`** means `Configs/Secrets.xcconfig` is missing, which is normal in a worktree that `worktree.sh` did not create. -- **`cannot find 'X' in scope` for code you just wrote** means the project was not regenerated. -- **MemberImportVisibility errors** on a new file that uses `Combine` or `TableProPluginKit` members are real: add the `import`. -- **CI's Swift compiler is older than this machine's.** A compile error that only CI reports is real (a shadowed `let x = x` inside a closure that initializes `x` has been one): fix it, never re-run hoping it passes. -- **SourceKit diagnostics in the editor are noise here** (`No such module 'TablePro'` and the like). Only an `xcodebuild` run counts. -- **One plugin's scheme is its target name in `project.yml`**, which is not always `Driver`: `MSSQLDriver`, but `SnowflakeDriverPlugin` and `TrinoDriverPlugin`. `xcodebuild -list -project /TablePro.xcodeproj` lists them; `verify.sh build ` builds one. -- **`verify.sh plugins` builds the HANA helper first** when the tree has none and Go is installed, since `AllPlugins` otherwise fails on `tablepro-hana-helper has no arm64 slice`. -- **`scripts/check-pluginkit-abi.sh` has no CI wiring.** For any PluginKit change, `verify.sh abi ` is the only check that runs. - -## Unit tests - -- **Run the suites that own the types you changed, plus their neighbors**, not the whole target, which takes far longer and tells you little more. The wrapper mutes cases listed in the quarantine files. A suite that fails only on one machine is a test bug: fix it with a pinned locale, a private pasteboard, a stubbed network or an injected clock. -- **Filter by Swift type name.** `-only-testing` matches the type, not the file name, not the `@Suite` display name and not a `@Test` function, and a filter that matches nothing still prints `TEST SUCCEEDED`. Resolve each name with `grep -rn "struct \|class " TableProTests` and compare the executed count with the number of tests those suites hold. -- **`The test runner hung before establishing connection`**, twice in a row, is a wedged `testmanagerd`, and every later run on the machine hangs the same way for about 14 minutes each. When no `xcodebuild` is running (`pgrep -f Developer/usr/bin/xcodebuild` prints nothing), `kill -9 $(pgrep testmanagerd)` (SIGTERM does not stop it); launchd starts a fresh one on the next run. -- **Zero cases executed** is a wedged host, an empty filter, or a test target that did not compile. The live run tells the last apart and reports it as `FAIL` with the compile errors; the other two are `INCONCLUSIVE`. -- **To run one test case**, which the wrapper cannot do because it takes suites, call `xcodebuild ... "-only-testing:TableProTests//()"` yourself with `DEVELOPER_DIR` set and the output redirected to a scratch log. Swift Testing's expectation text is in the result bundle (`xcrun xcresulttool get test-results tests --path `), not in the log. -- **A test behind `#if canImport(C...)` compiles to nothing** where that module is absent, so it looks present and never runs. -- **To decide whether a failing suite is yours**, grep its file for the symbols you changed. Zero references plus a known environment cause is a faster answer than a baseline build. - -## Package and iOS tests - -- `verify.sh package [filter]` runs `swift test` in `Packages/` with the pinned versions. Package test targets such as `TableProMSSQLCoreTests` live inside `Packages/TableProCore`; pass the test type as the filter. -- `verify.sh ios ` runs the iOS unit tests on the first available iPhone simulator, generating the iOS project if the tree has none. It needs `TableProMobile/Secrets.xcconfig`, which `worktree.sh` links. Run it when `TableProMobile/` changed, or a file it compiles from `Plugins/` or `Packages/`. - -## UI tests - -- **Launch only through `UITestCase`.** A bare `XCUIApplication()` or `: XCTestCase` under `TableProUITests/` fails a source-scanning guard, because storage isolation depends on the launch path. The app cannot detect XCUITest by itself, so it relies on the variables `UITestCase` sets. -- **The accessibility tree differs between this machine and the CI runner.** A contextual `NSMenu` has its identifier locally and not on CI, so reach it with `app.windows.firstMatch.menus.firstMatch`. The data grid is identifier `data-grid`, since the window holds several tables. Click the grid at an offset from `data-grid`, never on a row or cell element, which XCUITest reads as obscured. -- **`.accessibilityIdentifier` on a SwiftUI container replaces the identifier of every child** in that hosting tree. When the container needs one, put `.accessibilityElement(children: .contain)` before it. -- **Diagnose by dumping the tree** (`print(app.windows.firstMatch.debugDescription)` inside a `UITestCase`), not by guessing. -- **When UI tests cannot start on this machine** (`Timed out while enabling automation mode`, zero cases), reproduce it on an untouched suite, then say in the PR that CI runs the suite. When a test fails only on the runner for a reason you cannot establish, mark it XCTExpectFailure with the reason and an issue link instead of guessing at a fix. +- `xcodebuild` writes outside the tree, to `~/Library/Developer/Xcode/DerivedData`, which a sandbox has to allow. +- `cannot find 'X' in scope` for code you just wrote means the project was not regenerated: run `generate`. +- MemberImportVisibility errors on a new file that uses `Combine` or `TableProPluginKit` members are real: add the import. +- CI selects its Xcode in `.github/actions/setup-xcode/action.yml`. A compile error only CI reports is real: compare `xcodebuild -version` with that file, then fix the error. Never re-run hoping it passes. +- SourceKit diagnostics in the editor are noise. Only an `xcodebuild` run counts. +- A plugin's scheme is its target name in `project.yml`, not always `Driver`: `MSSQLDriver`, but `SnowflakeDriverPlugin`. `xcodebuild -list` lists them, and `verify.sh build ` builds one. +- After a package's public API or a type's stored layout changes, a target can still compile against the old `Build/Products/Debug/.swiftmodule` in DerivedData. The link then fails on the old symbol, or the test host crashes in an outlined destroy. Delete that module's `.swiftmodule` and `.o`, or the checkout's DerivedData folder, and rebuild. A clean CI build never sees it. + +## Test runs + +- Run the suites that own what you changed, plus their neighbors, not a whole plan. CI runs the plans in full (`.claude/rules/ci.md`, What runs where). +- Swift Testing's expectation text is in the result bundle, not the log: `xcrun xcresulttool get test-results tests --path `. +- A test behind `#if canImport(C...)` compiles to nothing where that module is absent, so it looks present and never runs. +- To decide whether a failing suite is yours, grep its file for the symbols you changed. + +## Packages and iOS + +- `package [regex]` runs `swift test --parallel` with an xunit file. SwiftPM writes the XCTest xunit file only for a parallel run. A package with a `Package.resolved` also gets `--force-resolved-versions`. +- A `Package.resolved` written by one toolchain can fail `--force-resolved-versions` on another when a transitive dependency was renamed. Re-pinning only moves the failure; drop the dependency or remove the rename from the graph. +- `ios ...` runs `TableProMobileTests` on the first available iPhone simulator with `-parallel-testing-enabled NO -skipPackagePluginValidation CODE_SIGNING_ALLOWED=NO ARCHS=arm64`. `ARCHS=arm64` is there because the prebuilt iOS libraries have arm64 simulator slices only. +- `ios` generates the iOS project when the tree has none. It needs `TableProMobile/Secrets.xcconfig`, which `worktree.sh` links. ## Lint -- **`lint` is SwiftLint only.** The check of `CLAUDE.md` and `.claude/` against the tree is its own step, `agent-docs`, because a stale reference already on `main` made every code lint red. -- **Pass file paths, not directories.** `.swiftlint.yml` limits `included:` to `TablePro` and `Packages`, and a directory argument outside it lints nothing while reporting zero violations. The wrapper names any directory it dropped. -- **Never remove a `swiftlint:disable force_unwrapping`** to satisfy a local run. The CI toolchain differs, and those disables are needed there. +- `lint` is SwiftLint only. `agent-docs` is its own step, so a stale doc reference never turns a code lint red; `AGENTS.md`, Writing style and docs, says what it checks. +- `.swiftlint.yml` `included:` holds six roots: `TablePro`, `Packages`, `Plugins`, `TableProMobile`, `TableProTests`, `TableProUITests`. +- Some folders lint with fewer rules through a nested `only_rules` config: `.claude/rules/ci.md`, Writing workflow and lint code. +- A directory outside `included:` lints nothing and reports 0 violations; the step names it under "NOT LINTED". Pass files. +- Paths go to SwiftLint as given, resolved against the shell's current directory, not `--root`. Pass absolute paths when the shell is elsewhere. +- `force_cast` and `force_unwrapping` are warnings, so only `--strict` fails them. A plain `swiftlint lint` passes code CI rejects. + +## Reading CI + +- Artifacts exist only for a failed job and are kept 7 days. `macos-tests-` holds every `.xcresult` plus `app-diagnostics` (TablePro crash reports and 2 hours of its unified log); `ios-tests-` holds `iOS.xcresult`. +- The step summary lists build errors, failed cases and the slowest cases (`.claude/rules/ci.md`, Summaries). A case that passed only on the UI plan's retry shows as a `Passed only on retry` warning annotation while the job stays green. +- `xcbeautify` can hide a compiler error behind "exit code 65". Read the check-run annotations, which also work while the run is going: `gh api repos/TableProApp/TablePro/check-runs//annotations`. Or open `Build.xcresult` from the artifact. +- UI evidence: `gh run download -n macos-tests-`, then `xcrun xcresulttool get test-results activities --test-id --path UI.xcresult` and `xcrun xcresulttool export attachments --path UI.xcresult --output-path `. The `element-tree-` attachment shows which window held focus; the recording and the Synthesized Event plists show where each event landed. +- A long silent stretch in the build log is a target's emit-module job, not a stall. +- PR CI tests the PR merged into main as of the trigger, and a re-run reuses that merge. After main is fixed, or a required check is renamed, merge main into the PR instead of re-running. ## Worktrees -- `worktree.sh [base]` creates `.claude/worktrees/` from `origin/main` by default and links `Configs/Secrets.xcconfig`, `Libs/*.a`, `Libs/dylibs`, `Libs/ios/*.xcframework` and the native bridge outputs. Run `verify.sh --root generate` before its first build. -- `worktree.sh --prune-merged [--dry-run]` removes each tree whose pull requests are all merged or closed, together with its DerivedData folder (about 7 GB each), but only when `git status` is clean and no process has its working directory inside it. A tree it keeps says why. Long-lived Codex broker processes started in a tree are the usual reason one is kept. -- A worktree builds into its own DerivedData, and `verify.sh` only waits for builds of the same tree, so `--no-wait` is safe there. -- The shell's working directory can drift between calls, so every `git` call for a worktree carries its own `cd &&` or `git -C `. -- `worktree.sh --remove ` refuses to remove a tree with uncommitted changes. Commit, or leave it. +- `.claude/skills/fix-issue/scripts/worktree.sh [base]` creates `.claude/worktrees/` from a fresh fetch of the base, `origin/main` by default. It links `Configs/Secrets.xcconfig`, `TableProMobile/Secrets.xcconfig`, `Libs/*.a`, `Libs/dylibs`, `Libs/ios/*.xcframework` and the native bridge outputs from the main checkout, and prints the path to pass as `--root`. Run `generate` before its first build. +- `worktree.sh --prune-merged [--dry-run]` removes each tree whose pull requests are all merged or closed, with its DerivedData folder. It removes one only when `git status` is clean and no process has its working directory inside; a kept tree says why. A Codex broker started in a tree is the usual process that keeps it. +- `worktree.sh --remove ` keeps the branch and refuses a tree with uncommitted or untracked changes. +- Each tree builds into its own DerivedData. `build`, `plugins`, `test`, `uitest` and `ios` wait only for an `xcodebuild` of the same checkout, found by its project path or working directory, so `--no-wait` is safe in your own tree. diff --git a/.claude/skills/fix-issue/scripts/lib/platform-apple.sh b/.claude/skills/fix-issue/scripts/lib/platform-apple.sh index 9e7fe6b7fa..ab6c93d2a9 100644 --- a/.claude/skills/fix-issue/scripts/lib/platform-apple.sh +++ b/.claude/skills/fix-issue/scripts/lib/platform-apple.sh @@ -47,7 +47,7 @@ diagnose_environment() { found=0 fi if grep -q 'Timed out while enabling automation mode\|failed to initialize for UI testing' "$log" 2> /dev/null; then - note "cause: Automation Mode is off and XCTest could not turn it on. Check with automationmodetool; enabling it needs an admin password." + note "cause: Automation Mode is off and XCTest could not turn it on. automationmodetool prints whether enabling it needs an admin password; the remedy is in references/verification-apple.md, INCONCLUSIVE." found=0 fi return $found diff --git a/.claude/skills/fix-issue/scripts/verify.sh b/.claude/skills/fix-issue/scripts/verify.sh index 17e7f280aa..58a50450c0 100755 --- a/.claude/skills/fix-issue/scripts/verify.sh +++ b/.claude/skills/fix-issue/scripts/verify.sh @@ -8,7 +8,7 @@ # # Usage: # verify.sh [options] [args] # a step of the platform, listed below -# verify.sh [options] agent-docs # AGENTS.md, .claude/rules and this skill against the tree +# verify.sh [options] agent-docs # the agent docs (check-doc-symbols.sh lists them) against the tree # verify.sh tail [lines] # re-read a stored log without rerunning # verify.sh parse # re-read the verdict for a stored log # @@ -226,7 +226,7 @@ case "$STEP" in note "$(tail -1 "$log")" else STATUS=FAIL - note "$(grep -E '^(CLAUDE|AGENTS|\.claude)' "$log" | sed 's/^/ /' | head -10)" + note "$(grep -E '^(CLAUDE|AGENTS|CONTRIBUTING|RELEASING|\.claude|\.github)' "$log" | sed 's/^/ /' | head -10)" fi emit "$log" $code ;; diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index cfe2a4560e..311deee4b8 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -12,49 +12,54 @@ description: > # Release -Run the stages in order, and do not start one until the one before it is real: 1 decide the shape, -2 release (always), 3 plugins (when they changed), 4 to 6 blog post, newsletter and X posts (big -releases only). A normal release stops after stage 3: the changelog and the docs changelog are its -announcement. For a big release the blog post is the announcement, and the newsletter and X posts -are written from the finished post, never from the changelog, so the three cannot disagree. - -House rules this skill relies on and does not repeat: `.claude/rules/changelog.md` (entry shape, -sections, credits, lead block), `.claude/rules/plugin-system.md` (PluginKit ABI, plugin tags) and the -commit format in `AGENTS.md` ("Every change", item 7). +- You never push to this repository's `main`. Every push made with the release key (the release + commit, a withdrawal) is the user's to run in their own terminal, because that key never enters + ssh-agent (`RELEASING.md`, Keys and rulesets). Wait for them to confirm it landed. +- Tags are always annotated, `-a -m`: the owner's git has `tag.gpgSign=true`, and a plain + `git tag ` fails with "no tag message?". +- The release commit holds exactly the four files that Commit, tag, push stages. +- Once a release run is past **Publish the release**, only **Re-run failed jobs** + (`RELEASING.md`, When a release run fails). + +Run the stages in order, starting one only when the one before it is real: 1 decide the shape, 2 +release (always), 3 plugins (when they changed), 4 to 6 blog post, newsletter and X posts (big +only). A normal release stops after stage 3; the changelog and docs changelog are its announcement. +A big release's newsletter and X posts come from the finished post, so the three cannot disagree. + +House rules relied on, not repeated: `.claude/rules/changelog.md`, `.claude/rules/plugin-system.md`, +the commit format in the top comment of `.github/pull_request_template.md`, and `RELEASING.md` for +the pipeline. ## Stage 1: Decide the shape -The release is **big** when at least one of these holds: - -- `### Added` has something a reader would change their behavior over: a new pane or mode, a new way - to connect, a new database, a rebuilt surface they will not recognize. -- Something people rely on was removed or changed under them. -- A long-standing class of bugs users have asked about is gone. - -Otherwise it is **normal**: fixes, small changes, plugin work. The entry count per section is a -sanity check, not the rule. Say which shape you picked and why before any work. Ask when it is -borderline: an unwanted newsletter costs an hour, a skipped one costs a launch. +The release is **big** when `### Added` has something a reader would change their behavior over (a +new pane or mode, a new way to connect, a new database, a rebuilt surface they will not recognize), +when something people rely on was removed or changed under them, or when a long-standing class of +bugs users have asked about is gone. Otherwise it is **normal**: fixes, small changes, plugin work. +The entry count per section is a sanity check, not the rule. Say which shape you picked and why +before any work, and ask when it is borderline: an unwanted newsletter costs an hour, a skipped one +costs a launch. ## Stage 2: Release ### Pre-flight -Check each one. If any fails, stop and say what is wrong. - -1. The version is semver (`X.Y.Z`, a suffix like `-beta.1` is fine), newer than `MARKETING_VERSION` - in `Configs/Version.xcconfig`, and its tag is free (`git tag -l "v"`). Ask if it is missing. -2. The tree is clean (`git status --porcelain`), or the user says the changes belong in the release. - `[Unreleased]` has entries; if not, say the release has no notes. -3. `swiftlint lint --strict` is clean. Fix findings first, through their own pull request. -4. If `Plugins/TableProPluginKit/` changed since the last tag, settle the ABI now (stage 3, PluginKit), - because a breaking bump publishes plugins before the app tag. -5. `scripts/export-cloudkit-schema.sh --check` passes. A field in the snapshot but not in Production - makes CloudKit reject every record that carries it, so the user deploys it in CloudKit Console - before the tag. It needs the cktool management token on this Mac. -6. Stop when you are not on `main` or it is not `origin/main`. The tag runs no suite: its `Tested on main` job takes, for - `macOS Tests` and `iOS and Package Tests` each, the newest `main` commit where that job ran, and - fails the release when it is red or when files the suite reads changed after it. Report both - verdicts below, and stop when either is a failure. +Check each. If one fails, stop and say what is wrong. + +1. The version is semver (`X.Y.Z`; a suffix like `-beta.1` is fine), newer than `MARKETING_VERSION` + in `Configs/Version.xcconfig`, and its tag is free (`git tag -l "v"`). Ask if missing. +2. The tree is clean (`git status --porcelain`), or the user says the changes belong in the + release. `[Unreleased]` has entries; if not, say the release has no notes. +3. `swiftlint lint --strict` is clean: the tag's checks do not cover SwiftLint. Fix findings + through their own pull request. +4. If `Plugins/TableProPluginKit/` changed since the last tag, do Stage 3, PluginKit, now, before + the tag. +5. `scripts/export-cloudkit-schema.sh --check` passes; it needs the cktool management token on this + Mac. A field in the snapshot but not in Production makes CloudKit reject every record carrying + it, so the user deploys it in CloudKit Console before the tag. +6. After `git fetch origin main`, you are on `main` and it equals `origin/main`. Report both suite + verdicts and stop when either is a failure. What the tag's `Tested on main` job checks on top of + this: `RELEASING.md`, The release pipeline. ```bash for pair in 'macos-tests.yml|macOS Tests' 'ios-tests.yml|iOS and Package Tests'; do @@ -69,6 +74,14 @@ Check each one. If any fails, stop and say what is wrong. done ``` +7. The release key exists: `test -f ~/.ssh/tablepro_release`. If not, stop and give the user both + commands to run, the first with a passphrase: + + ```bash + ssh-keygen -t ed25519 -f ~/.ssh/tablepro_release -C "release commit push" + gh repo deploy-key add ~/.ssh/tablepro_release.pub --allow-write --title "Release commit push" -R TableProApp/TablePro + ``` + ### Credit the entries Do this before rewording anything: the script reads each entry's commit with `git blame`, and a line @@ -80,15 +93,15 @@ python3 scripts/ci/changelog_credits.py ``` An outside contributor's entry ends `(#2905 by @digows)`, a maintainer's ends `(#2905)`. The output -ends with every contributor and every entry left bare. A bare entry came from a direct push: credit it -from `git log` or leave it, and never guess a handle. Blame credits the last commit to touch a line, so -check the contributors against `gh pr list --state merged --search "merged:>=" ---json author` and restore any handle a later rewording took. The script is idempotent. +ends with every contributor and every entry left bare. A bare entry came from a direct push: credit +it from `git log` or leave it, and never guess a handle. Blame credits the last commit to touch a +line, so check the contributors against `gh pr list --state merged --search "merged:>=" --json author` +and restore any handle a later rewording took. The script is idempotent. ### Tighten the entries -Entries arrive one PR at a time and drift long. Measure them without their credit, then look for the -repair-story tells: +Entries arrive one PR at a time and drift long. Measure them without their credit, then look for +the repair-story tells: ```bash awk '/^## \[Unreleased\]/{f=1;next} /^## \[/{f=0} f' CHANGELOG.md | grep '^- ' \ @@ -108,17 +121,18 @@ the `#` references and handles before and after to prove none were dropped. ### Bump the version In `Configs/Version.xcconfig`, set `MARKETING_VERSION` to the new version and add 1 to -`CURRENT_PROJECT_VERSION`: Sparkle compares the build number, and the release job fails if it did not -rise. Touch no other version: plugins and PluginKit pin `1.0` in `project.yml`, and the iOS app has -`Configs/Version-iOS.xcconfig`. +`CURRENT_PROJECT_VERSION`. Touch no other version: plugin and PluginKit versions live in `project.yml`, +and the iOS app's in `Configs/Version-iOS.xcconfig`. The release job checks both values +(`RELEASING.md`, The release pipeline). ### Finalize CHANGELOG.md 1. Add `## [] - ` below `## [Unreleased]`, leaving Unreleased empty. In the footer, point `[Unreleased]` at `v...HEAD` and add `[]: https://github.com/TableProApp/TablePro/compare/v...v`. -2. Each `###` heading appears once, in the order Added, Changed, Deprecated, Removed, Fixed, Security. - Merge a repeated heading into the first and move an out-of-order block, then check nothing was lost: +2. Each `###` heading appears once, in the order Added, Changed, Deprecated, Removed, Fixed, + Security. Merge a repeated heading into the first and move an out-of-order block, then check + nothing was lost: ```bash awk '/^## \[\]/{f=1;next} /^## \[/{f=0} f' CHANGELOG.md | grep '^### ' @@ -126,9 +140,9 @@ rise. Touch no other version: plugins and PluginKit pin `1.0` in `project.yml`, grep -n '^## \[' CHANGELOG.md | head -5 ``` -3. Write the lead block: two or three lines under the version heading, before the first `###`, naming - the `### Added` items a reader would change their behavior over, in the reader's words. The update - window and the Sparkle feed show only this block. A release of pure fixes can skip it. +3. Write the lead block: two or three lines under the version heading, before the first `###`, + naming the `### Added` items a reader would change their behavior over, in the reader's words. + The update window and the Sparkle feed show only this block. A release of pure fixes can skip it. 4. Regenerate the in-app notes so **Help > What's New** matches the feed: `scripts/generate-whats-new.sh `. @@ -143,46 +157,39 @@ docs are English only, so there is no translated copy. ### New Features - **Feature Name**: what it does for the reader - - ### Bug Fixes - - - Description ``` -The headings are New Features, Improvements, Removed, Security and Bug Fixes, in that order, each only -when the version has one. Each line is one CHANGELOG entry written for a reader, and a New Features +The headings are New Features, Improvements, Removed, Security and Bug Fixes, in that order, each +only when the version has one. Each line is one CHANGELOG entry written for a reader; a New Features line may add one sentence on what the reader does with it. An outside contributor's line ends `, by [@user](https://github.com/user)`; a maintainer's carries no credit. ### Critical update flag `.github/release-flags.json` holds `criticalUpdate`. Leave it `false`: updates install in the -background and apply on quit, and the flag is what lets Sparkle interrupt the user. Set it `true` only -when the release fixes one of these, and say which: +background and apply on quit, and the flag is what lets Sparkle interrupt the user. Set it `true` +only when the release fixes one of these, and say which: - Data loss or corruption of a database, saved connections or persisted tabs. - A security issue with a `### Security` entry that users of the old build are exposed to. - A crash or hang on a common path, or a failure to launch, connect or update. - A regression from the previous release with no workaround. -Nothing else, a handful a year at most. Set it in the release commit and back to `false` in the next -one: the release job fails when it is `true` and the file is unchanged since the previous tag. A retag -of the same release never gets it. A bad build is withdrawn (below) and superseded by a corrective -release with the flag. +Nothing else; a handful a year at most. Set it in the release commit and back to `false` in the +next. A retag of the same release never gets it. A bad build is withdrawn (Withdraw a release) and +superseded by a corrective release with the flag. What the flag makes Sparkle do, and the release +job's check for a leftover flag: `RELEASING.md`, Staged rollout and critical updates. ### Commit, tag, push -`main` takes no direct push. The release commit is the one exception: it goes up with the release -deploy key `~/.ssh/tablepro_release`, the only bypass in main's ruleset. The key has a passphrase and -is never added to ssh-agent, so you cannot run that push: ask the user to run it in their own -terminal, wait for them to confirm it landed, then push the tag. The key skips every check, so -before you hand it over, prove the push carries the release commit and nothing else. Stop if the -check prints nothing. +The release key skips every check, so before the user pushes with it, prove the push carries the +release commit and nothing else; stop if the check prints nothing. Push the docs changelog in the +same sitting: the docs CI fails while the app's `CHANGELOG.md` carries a version the page does not. +It goes straight to the docs `main` under the admin bypass, and Mintlify deploys it. ```bash -git add Configs/Version.xcconfig CHANGELOG.md .github/release-flags.json \ - TablePro/Resources/WhatsNew.md +git add Configs/Version.xcconfig CHANGELOG.md .github/release-flags.json TablePro/Resources/WhatsNew.md scripts/check-banned-words.sh --staged git commit -m "release: v" git fetch origin main @@ -194,60 +201,37 @@ git tag -a v -m "v" # The user runs this one: GIT_SSH_COMMAND='ssh -i ~/.ssh/tablepro_release -o IdentitiesOnly=yes' git push origin main git push origin v -``` - -Then publish the docs changelog. It goes straight to the docs `main` under the admin bypass, and -Mintlify deploys it: - -```bash +# Then the docs changelog: git -C ../tablepro-docs add changelog.mdx git -C ../tablepro-docs commit -m "docs(changelog): v" git -C ../tablepro-docs push origin main ``` -The docs CI fails while the app's `CHANGELOG.md` carries a version the page does not, so push it in -the same sitting as the tag. - - Keep the subject exactly `release: v`. -- Always `-a -m`: the owner's git has `tag.gpgSign=true`, and a plain `git tag ` fails with - "no tag message?". -- Keep the release commit to the four files above. No suite reads them, so its push to `main` skips - both suites and the tag takes the verdict of the code it ships. Any other file runs the suites on - that push, and the tag waits for them. A lint fix made on the way lands first, through its own - pull request. - -The tag runs `.github/workflows/build.yml`: arm64 and x86_64 builds, DMG and tar.xz, Sparkle signatures, -deltas from the previous five releases, the GitHub Release with notes from `CHANGELOG.md`, then -`appcast.xml`, pushed with `APPCAST_DEPLOY_KEY` from the `release` environment. The release is created -as a draft holding every asset and published in one step, only when `Tested on main`, both builds and -`Registry Readiness` pass. The job fails when the tag disagrees with `MARKETING_VERSION`. - -A published release is immutable: its assets and its tag are locked. When a run fails: - -- Before **Publish the release**, only a draft exists. Fix forward on `main` through a pull request, - then move the tag to the commit it lands as: `git fetch origin main`, - `git tag -f -a v -m "v" origin/main`, then `git push origin v --force`. - The next run reuses the draft. -- After it, at the `appcast.xml` push for instance, use **Re-run failed jobs**. The release job finds - the same files already published and carries on. **Re-run all jobs** rebuilds, and the release job - refuses the new files. -- A published build that is wrong ships as a new version. +- Keep the release commit to the four files above. All four are on the change detector's + release-only list (`.claude/rules/ci.md`, Change detector), so the push skips both suites and the + tag takes the verdict of the code it ships. Any other file runs the suites on that push, and the tag waits for + them. A lint fix made on the way lands first, through its own PR. -### Withdraw a release +### Confirm it shipped -Only when a build loses data, fails to launch or breaks connecting, and the fix is more than a few -minutes away. Removing the version from `appcast.xml` is the only way to un-ship a Sparkle release: +A pushed tag is not a build: the workflow publishes from its last job. Check the run, the release +assets and the feed: ```bash -python3 scripts/ci/pull-release.py --dry-run -# The user runs this one, for the same reason as the release push: -GIT_SSH_COMMAND='ssh -i ~/.ssh/tablepro_release -o IdentitiesOnly=yes' \ - python3 scripts/ci/pull-release.py +gh run list --workflow build.yml --limit 1 +gh release view v --json tagName,assets -q '"\(.tagName) assets=\(.assets|length)"' +curl -s https://raw.githubusercontent.com/TableProApp/TablePro/main/appcast.xml | grep -o '[^<]*' | head -2 ``` -It commits and pushes to `main` with the release key. Leave the GitHub Release so direct DMG links -resolve. Users who already installed the build are reached by the corrective release with -`criticalUpdate` set. +That raw URL is the app's `SUFeedURL`; `tablepro.app/appcast.xml` does not exist. `shortVersionString` +is an element, not an attribute. A failed run: `RELEASING.md`, When a release run fails. + +### Withdraw a release + +Only when a build loses data, fails to launch or breaks connecting, and the fix is more than a few +minutes away. Commands and effects: `RELEASING.md`, Withdrawing a release; the user runs its push. +Users already on the build are reached by the corrective release with `criticalUpdate` set. ## Stage 3: Plugins @@ -259,56 +243,50 @@ python3 .claude/skills/release/scripts/changed-plugins.py Slugs, tag names and targets come from `.github/plugin-registry.json`, so nothing is mapped by hand. `own` counts commits in the plugin's source paths (its folder, plus a package or `Native/` bridge it -builds from), `kit` counts PluginKit commits. Skip `bundled` rows: their changes ship inside the app, -which never applies a registry update to a bundled plugin. A row with only `kit` commits -needs a release only after a breaking bump, through `release-all-plugins.sh`. +builds from), `kit` counts PluginKit commits. Skip `bundled` rows: their changes ship inside the +app, which never applies a registry update to a bundled plugin. A row with only `kit` commits needs +a release only after a breaking bump, through `release-all-plugins.sh`. Show the user what changed and ask before tagging, suggesting a patch bump from the last tag. Tag -each approved plugin annotated, for the same signing reason as the app. The tag is the version, so a -plugin needs no version bump or changelog edit. +each approved plugin annotated (top of this file). The tag is the version, so a plugin needs no +version bump or changelog edit. ```bash git tag -a plugin--v -m "plugin--v" git push origin plugin--v ``` -- One tag per push: GitHub sends no push event when one push carries more than three tags, so no build - starts. If several went at once, run `gh workflow run build-plugin.yml -f tags=","`. -- Wait for the app build first (`gh run list --workflow build.yml --limit 1`). Five macOS jobs run at - a time, and plugin builds would queue the release's own tests. -- Confirm a run started for each tag: `gh run list --workflow build-plugin.yml --limit 10`. -- A published plugin release cannot take a rebuilt binary, and a rebuild never matches the published - bytes. A plugin run that fails after its release is published ships as the next patch version. +- One tag per push: GitHub sends no push event for a push carrying more than three tags, so no + build starts. If several went at once, run `gh workflow run build-plugin.yml -f tags=","`. +- Wait for the app build first (`gh run list --workflow build.yml --limit 1`): plugin builds compete + with its `Build` jobs and `Create GitHub Release` for the same macOS runner slots + (`.claude/rules/ci.md`, Caches and runner budget). Then confirm a run started for each tag with + `gh run list --workflow build-plugin.yml --limit 10`. +- A published plugin release cannot take a rebuilt binary, and a rebuild never matches the + published bytes. A plugin run that fails after its release is published ships as the next patch. ### PluginKit -If the release touched `Plugins/TableProPluginKit/`, run `scripts/check-pluginkit-abi.sh -v` before publishing anything, and act on the diff as -`.claude/rules/plugin-system.md` says. Any diff bumps `currentPluginKitVersion`, once per release -cycle. A breaking one also raises `minimumCompatiblePluginKitVersion`, and -`scripts/release-all-plugins.sh ` must publish before the app tag: `registry-readiness` -blocks the release otherwise, and a silent install would hide drivers that stopped loading. Run the -bulk release for a breaking bump only: an additive bump leaves published binaries valid, and each -bulk run uses up a registry retention slot for every plugin. - -To get one driver fix to users who have not updated, build it against their release: +If the release touched `Plugins/TableProPluginKit/`, run `scripts/check-pluginkit-abi.sh v` +before publishing anything, and act on the diff as `.claude/rules/plugin-system.md` (ABI) says. +`scripts/release-all-plugins.sh` runs for a breaking bump only: each run uses a registry retention +slot for every plugin. One driver fix reaches users who have not updated through `scripts/release-plugin-for-shipped-app.sh plugin--v v`. ## Stages 4 to 6: Announce (big only) 1. **Blog post**, by `references/blog-post.md`: one file in `resources/blog/` of the marketing site - repo (`../tablepro-web` by default; confirm the path). It goes live first, because the newsletter - and the X posts link to it. Once it is live, link it from the version's `` block in - `changelog.mdx` of the docs repository, on its own line before the first `###`, and push it as - `docs(changelog): link the announcement`: - `[Read the announcement](https://tablepro.app/blog/)`. + repo (`../tablepro-web` by default; confirm the path). It goes live first, because the + newsletter and the X posts link to it. Once live, link it from the version's `` block in + the docs `changelog.mdx` with `[Read the announcement](https://tablepro.app/blog/)` on its + own line before the first `###`, pushed as `docs(changelog): link the announcement`. 2. **Newsletter**, by `references/newsletter.md`: 150 to 250 words that name the headline items and link the post. -3. **X posts**, by `references/newsletter.md`: four to six posts plus one standalone, written from the - finished post and newsletter. +3. **X posts**, by `references/newsletter.md`: four to six posts plus one standalone, written from + the finished post and newsletter. -Lint each draft, then work `references/fact-checks.md` top to bottom. Its closing blocker list gates -all three; report blockers separately from the drafts. +Lint each draft, then work `references/fact-checks.md` top to bottom. Its closing blocker list +gates all three; report blockers apart from the drafts. ```bash python3 .claude/skills/release/scripts/lint-draft.py --blog /resources/blog/.md @@ -317,5 +295,5 @@ python3 .claude/skills/release/scripts/lint-draft.py Newsletter and X drafts go in the session scratchpad, named by version (`newsletter-v0.77.0.md`, `x-posts-v0.77.0.md`). Sending is out of scope. If the user asks to send the newsletter, warn them -first: in the license backend `all` means verified subscribers only, a small fraction of the list, and -`everyone` is the real list. +first: in the license backend `all` means verified subscribers only, a small fraction of the list, +and `everyone` is the real list. diff --git a/.claude/skills/release/references/fact-checks.md b/.claude/skills/release/references/fact-checks.md index c16d163f09..d137919b8a 100644 --- a/.claude/skills/release/references/fact-checks.md +++ b/.claude/skills/release/references/fact-checks.md @@ -1,6 +1,6 @@ # Fact checks -`lint-draft.py` checks how a sentence reads. This checks whether it is true. Work it top to +`lint-draft.py` checks how a sentence reads; this file checks whether it is true. Work it top to bottom on the blog post first, then on the newsletter and the X posts written from it. ## 1. A fix is tied to a version only when git agrees @@ -10,17 +10,17 @@ git log --oneline -S "" -- git show : | grep -n "" ``` -If the broken code is also at the tag before that, it was never a regression. Say what was -wrong without naming a version. +If the broken code is also at the tag before that, it was never a regression. Say what was wrong +without naming a version. ## 2. A behavior change matches what the old code did -Before grouping several things under one verb ("joins, unions and CTEs are read-only now"), -read the diff and confirm each one changed. A summary easily drops the changelog's scope. +Before grouping several things under one verb ("joins, unions and CTEs are read-only now"), read the +diff and confirm each one changed. A summary easily drops the changelog's scope. ## 3. A plugin feature needs a published binary -Not installable until its tag exists, contains the commit, and the registry lists it: +It is not installable until its tag exists, the tag contains the commit, and the registry lists it: ```bash git tag -l "plugin--*" | tail -3 @@ -29,8 +29,8 @@ curl -s https://raw.githubusercontent.com/TableProApp/plugins/main/plugins.json 'import json,sys; [print(p["id"], p["version"]) for p in json.load(sys.stdin)["plugins"] if "" in p["id"]]' ``` -The registry `id` is the `bundleId` in `.github/plugin-registry.json`. If a check fails, tag -the plugin with the release or cut the section. +The registry `id` is the `bundleId` in `.github/plugin-registry.json`. If a check fails, tag the +plugin with the release or cut the section. ## 4. Images are real and current @@ -47,7 +47,7 @@ Never embed a placeholder or a stale shot. Mark the gap in the draft and list it ## 5. Menu labels come from the app -The docs paraphrase. Take the string from the menu builder, and watch for labels that change by +The docs paraphrase, so take the string from the menu builder. Watch for labels that change by engine: "Close Tabs for Other Databases" reads "Other Schemas" on Oracle, Dameng and BigQuery. ```bash @@ -60,10 +60,11 @@ grep -rn "String(localized:" TablePro/Core/Menu/ | grep -i "" grep -n -A3 "case " TablePro/Models/Settings/ProFeature.swift ``` -`requiredTier` is the lowest tier that includes a feature, and Team includes Starter, so write -"needs a license, and both tiers include it". Describe a gate as it behaves: a locked overlay -is not a disabled menu item. Quote the number that decides a behavior, or none, after reading -every constant in the model file, including the default range a comparison runs over. +- `requiredTier` is the lowest tier that includes a feature, and Team includes Starter, so write + "needs a license, and both tiers include it". +- Describe a gate as it behaves: a locked overlay is not a disabled menu item. +- Quote the number that decides a behavior, or none, after reading every constant in the model + file, including the default range a comparison runs over. ## 7. Every URL resolves @@ -72,31 +73,23 @@ ls ../tablepro-docs/features/.mdx ../tablepro-docs/databases/.mdx grep -n "^## " ../tablepro-docs/databases/.mdx ``` -The docs `changelog.mdx` carries the version only after the release-day docs deploy, so that link -is a blocker until then, not a broken link. +The docs `changelog.mdx` carries the version only after the release-day docs deploy, so that link is +a blocker until then, not a broken link. ## 8. Counts and references -Count the bullets under `### Fixed` in the shipped section and write "N fixes", never "N bugs -fixed": one bullet can cover several issues. Never inflate it with issue or commit counts. -Leave issue numbers out of every announcement; changelog references are sometimes wrong. +- Count the bullets under `### Fixed` in the shipped section and write "N fixes", never "N bugs + fixed": one bullet can cover several issues. +- Never inflate the count with issue or commit counts. +- Leave issue numbers out of every announcement; changelog references are sometimes wrong. ## Blockers Report these apart from any draft. Nothing goes out until every one holds. -- The GitHub Release has its assets and the appcast carries the version. A pushed tag is not a - build: the workflow takes about 45 minutes and publishes from its last job. - - ```bash - gh run list --workflow build.yml --limit 1 - gh release view v --json tagName,assets -q '"\(.tagName) assets=\(.assets|length)"' - curl -s https://raw.githubusercontent.com/TableProApp/TablePro/main/appcast.xml \ - | grep -o '[^<]*' | head -2 - ``` - - That raw URL is the `SUFeedURL`. `tablepro.app/appcast.xml` does not exist, and - `shortVersionString` is an element, not an attribute. +- The release is live: the run finished, the GitHub release has its assets, and the appcast carries + the version. The commands and the `SUFeedURL` facts are in the release skill's Confirm it shipped + section (`.claude/skills/release/SKILL.md`). - Every plugin an announcement names is tagged, built and in the registry (check 3). - Screenshots are captured, light and dark, on a build that contains the change. - The docs `changelog.mdx` is deployed with the version. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 006676846f..7e1a85a819 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,7 +1,11 @@ ## Why @@ -14,7 +18,7 @@ Leave out review history, test lists and counts, the investigation story and fil ## Verified - + ## Screenshots diff --git a/.github/release-flags.json b/.github/release-flags.json index 8a9b9f0b81..61784166a7 100644 --- a/.github/release-flags.json +++ b/.github/release-flags.json @@ -1,4 +1,4 @@ { - "$comment": "Per-release flags read by .github/workflows/build.yml. criticalUpdate marks the release as one Sparkle may interrupt a user about, which bypasses phased rollout, hides Skip and Remind Me Later, and reaches a user who has already downloaded it sooner. The bar for setting it is written in .claude/skills/release/SKILL.md and RELEASING.md: data loss, an exposed security issue, a crash or hang on a common path, or a regression with no workaround. Set it in the release commit and set it back in the next one. The release job fails if it is true and this file did not change since the previous tag, which is what a leftover flag looks like.", + "$comment": "Per-release flags read by .github/workflows/build.yml. criticalUpdate: when to set it is the Critical update flag section of .claude/skills/release/SKILL.md; what it does is the Staged rollout and critical updates section of RELEASING.md.", "criticalUpdate": false } diff --git a/AGENTS.md b/AGENTS.md index 52b6512b5f..9b1503b92f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,151 +1,153 @@ # TablePro -A native macOS database client (SwiftUI + AppKit), a lightweight alternative to TablePlus. macOS 13.0+, Swift 6 language mode for the app targets (`Configs/Base.xcconfig`), Universal Binary. `TableProMobile/` is the iOS app. +A native macOS database client (SwiftUI and AppKit), a lightweight alternative to TablePlus. macOS 13.0+, Swift 6 language mode for the app targets (`Configs/Base.xcconfig`), Universal Binary. `TableProMobile/` is the iOS app. -This file is the project guide for every agent, and it covers the macOS and iOS apps. An app for another platform keeps its own guide beside its code, and that guide is the one to follow there. Claude Code loads this file through `CLAUDE.md`; Codex loads it directly. Area rules live in `.claude/rules/` (index at the end), and Claude Code loads each one automatically when you touch a file its `paths:` names. Codex does not, so read the matching rule before editing those paths. +This guide covers the macOS and iOS apps; another platform's app keeps its own guide beside its code. Claude Code loads it through `CLAUDE.md`, Codex directly. ## Principles 1. **Security first.** Validate at system boundaries; never introduce injection, credential exposure or a widened permission by accident. -2. **Native only.** AppKit, SwiftUI and system frameworks. No cross-platform layers, no web views for native UI. Follow the documented Apple API and the HIG, not a hand-rolled equivalent. -3. **Fix the cause, not the symptom.** Diagnose (add OSLog if needed), then fix the actual cause. No temporary workarounds left in place. -4. **Clean architecture.** Separation of concerns, protocol-oriented design, dependency injection where it earns its place. Consider the effect of every change on the design, not just the immediate problem. -5. **Tests define behavior.** Every testable change has a test. When a test fails, fix the code, never the test. -6. **Open plugin domain.** `DatabaseType` is a string-backed struct, so unknown types from future plugins round-trip. A `switch` over an open type keeps a `default:`; a `switch` over a closed enum stays exhaustive so the compiler flags new cases. +2. **Native only.** AppKit, SwiftUI and system frameworks; no cross-platform layer, no web view for native UI. Follow the documented Apple API and the HIG. +3. **Fix the cause, not the symptom.** Diagnose (add OSLog if needed), then fix the cause. No workaround left behind. +4. **Clean architecture.** Separation of concerns, protocol-oriented design, dependency injection where it earns its place. Weigh each change's effect on the design. +5. **Tests define behavior.** Every testable change has a test. When a test fails, fix the code, never the test. A test that pins behavior the PR changes on purpose is updated in the same PR and named in its description. +6. **Open plugin domain.** `DatabaseType` is a string-backed struct (static constants, `allKnownTypes`), so a future plugin's type round-trips. A `switch` over an open type keeps a `default:`; one over a closed enum stays exhaustive. ## Scope Every capability is core, a module or a plugin. The first rule that fits decides: -1. Used weekly by an app or backend developer or an AI-first user, or only a row that runs nothing until clicked: **core**, with no toggle. -2. UI for one engine: core, shown only for the drivers that support it. -3. A database driver or a heavy native dependency: **plugin**, shipped through the plugin registry and signed by TablePro. -4. Any other advanced tool that needs a session, the grid, the editor or native UI: **module**, compiled in and turned on or off as a unit. -5. A duplicate, or not worth maintaining: delete it. Never move it to the registry as a halfway step. +1. Used weekly by app or backend developers or AI-first users, or only a row that runs nothing until clicked: **core**, no toggle. +2. UI for one engine: core, shown only for drivers that support it. +3. A database driver or a heavy native dependency: **plugin**, registry-shipped, signed by TablePro. +4. Any other advanced tool needing a session, the grid, the editor or native UI: **module**, compiled in, toggled as a unit. +5. A duplicate, or not worth maintaining: delete it, never park it in the registry. -- A new database engine ships only as a registry plugin. Engine-specific code never goes into `TablePro/`; engine UI goes through a PluginKit capability or plugin metadata. +- A new engine ships only as a registry plugin: no engine-specific code in `TablePro/`; engine UI uses a PluginKit capability or plugin metadata. - A new advanced tool ships as a module, off by default. -- A module PR declares its module id, its default, the sink that refuses it while it is off, its residue test and its docs page. -- An off module leaves nothing behind: no menu item, key equivalent, context item, Settings section or Keyboard row; no launch work, observer, timer, listener or child process; no Keychain read or network call. A URL, AppleScript or MCP request for it gets an error that says where to turn it on, and only a click in TablePro turns it on. +- A module PR declares its id, default, off-state sink, residue test (a behavior test) and docs page. +- An off module leaves nothing behind: no menu item, key equivalent, context item, Settings section or Keyboard row; no launch work, observer, timer, listener or child process; no Keychain read or network call. A URL, AppleScript or MCP request for it gets an error naming where to enable it; only a click in TablePro enables it. ## Repository map -- `TablePro/`: the app. `Core/` (services, business logic), `Views/`, `Models/`, `ViewModels/`, `Extensions/`, `Theme/`. -- `Plugins/`: every database driver and import/export format is a `.tableplugin` bundle, plus `TableProPluginKit`, the shared framework. Bundled plugins are the targets in the app's `copy: { destination: plugins }` phase in `project.yml`; the rest are registry-only and ship through [TableProApp/plugins](https://github.com/TableProApp/plugins). Some bundled plugins also have a registry entry (`"bundled": true` in `.github/plugin-registry.json`), but the app never installs or offers a registry update over a bundled plugin; a published build reaches a user only through a manual install of its zip. -- `Packages/`: local SwiftPM packages (`TableProCore`, `TableProOracle`, `TableProEditor`, `TableProGrammars`). Each sets its own Swift language mode in its manifest; `SWIFT_VERSION` in the xcconfig does not reach them, and passing `SWIFT_VERSION=` to `xcodebuild` reaches every package and reports their errors as yours. `Packages/TableProCore/Sources/TableProPluginKit` is a symlink: edit `Plugins/TableProPluginKit/` only. -- `Native/`: first-party Rust and Go bridges (see `.claude/rules/native-bridges.md`). -- `Libs/`: prebuilt static libraries and iOS xcframeworks, downloaded by `scripts/download-libs.sh`, not in git. -- The docs site (docs.tablepro.app) is its own repository, [TableProApp/docs](https://github.com/TableProApp/docs), cloned beside this one as `../tablepro-docs`. Its `AGENTS.md` and `STYLE.md` govern its pages. -- `TablePro.xcodeproj` and `TableProMobile/TableProMobile.xcodeproj` are generated from `project.yml` and `TableProMobile/project.yml` by XcodeGen. They are gitignored, except each one's tracked `Package.resolved`. Never hand-edit them. Signing overrides and secrets go in `Configs/Secrets.xcconfig` (template: `Secrets.xcconfig.example`). +- `TablePro/`: the app (`Core/`, `Views/`, `Models/`, `ViewModels/`, `Extensions/`, `Theme/`). +- `Plugins/`: drivers and import/export formats as `.tableplugin` bundles, plus `TableProPluginKit`. Bundled plugins are the app's `copy: { destination: plugins }` targets in `project.yml`; the rest ship through TableProApp/plugins, and the app never offers a registry install or update over a bundled one. +- `Packages/`: `TableProCore` (both apps), `TableProMacCore` (macOS-only modules; never in the iOS project), `TableProOracle`, `TableProEditor`, `TableProGrammars`. `TableProCore`'s PluginKit source is a symlink: edit `Plugins/TableProPluginKit/` only. Each manifest sets its own Swift language mode, so never pass `SWIFT_VERSION=` to `xcodebuild`. +- Tests: `TableProTests/`, `TableProUITests/`, `Plugins/*Tests/`, `Packages/*/Tests/`, `TableProMobile/TableProMobileTests/`; plans in `TestPlans/`. Which holds what: `.claude/rules/tests.md`. +- `Native/`: first-party Rust and Go bridges (`.claude/rules/native-bridges.md`). +- `Libs/`: static libraries and iOS xcframeworks from `scripts/download-libs.sh`, not in git. +- Docs site: TableProApp/docs, cloned beside this repo as `../tablepro-docs`, under its own `AGENTS.md` and `STYLE.md`. +- `TablePro.xcodeproj` and `TableProMobile/TableProMobile.xcodeproj` are generated by XcodeGen from their `project.yml`, gitignored except their `Package.resolved`; never hand-edit them. Signing overrides go in `Configs/Secrets.xcconfig`. -## Commands +## Architecture -```bash -scripts/download-libs.sh # first time, and after a libs update -scripts/generate-project.sh # after editing project.yml or Configs/, or adding, moving or deleting a source file -``` +- **Plugins.** `PluginManager` loads `DriverPlugin` bundles; `DatabaseDriverFactory` finds one by `pluginTypeId`; `PluginDriverAdapter` bridges its `PluginDatabaseDriver` to `DatabaseDriver`; `DatabaseManager` owns sessions. A new requirement gets a default implementation, then the adapter (`.claude/rules/plugin-system.md`). +- **Main window.** `MainContentCoordinator`, split into `Views/Main/Extensions/`; one `ConnectionWorkspace` per connection (`.claude/rules/connection-window.md`). +- **Change tracking.** `DataChangeManager` holds edits behind `AnyChangeManager`; `SQLStatementGenerator` writes them on Save. Undo: `StructureChangeManager`'s `UndoManager`, `ConnectionWorkspace.undoManager`. +- **Editor and theme.** `ThemeCatalog` lists themes, `ThemeResolver` picks the light and dark pair, `ThemeEngine` holds the palette and font caches; fonts come from `TypographySettings`. A theme colors content and window backdrop, never chrome (`.claude/rules/theme.md`). `QueryCompletionAdapter` bridges `CompletionEngine`; `EditorTabStrip` draws tabs (`.claude/rules/editor.md`). +- **Storage.** Passwords in the Keychain, preferences in UserDefaults, query history in SQLite FTS5, tab state as JSON (`.claude/rules/data-sync-storage.md`). -Build, test and lint through the wrapper, which keeps the full log on disk and prints a short verdict (`PASS`, `FAIL`, `INCONCLUSIVE`): +## Commands ```bash +scripts/download-libs.sh # first time, and after a libs update +scripts/generate-project.sh [both|macos|ios] # needs the libs; rerun after editing a project.yml or Configs/, or adding, moving or deleting a source file .claude/skills/fix-issue/scripts/verify.sh build -.claude/skills/fix-issue/scripts/verify.sh test [SuiteType...] -.claude/skills/fix-issue/scripts/verify.sh lint [...] +.claude/skills/fix-issue/scripts/verify.sh test [Test...] # no Test: the whole Unit plan +.claude/skills/fix-issue/scripts/verify.sh lint ... ``` -It also has `generate`, `uitest`, `package`, `ios`, `plugins`, `abi`, `l10n`, `docs` and `agent-docs`; `verify.sh --help` lists them. The steps are the platform's: on a Mac they are these, and `--platform ` picks another app's. The raw equivalents always pass `-skipPackagePluginValidation`: - -```bash -xcodebuild -project TablePro.xcodeproj -scheme TablePro -configuration Debug build -skipPackagePluginValidation -xcodebuild -project TablePro.xcodeproj -scheme TablePro test -skipPackagePluginValidation -only-testing:TableProTests/ -swiftlint lint --strict -``` - -- Tests are Swift Testing. `-only-testing` matches the Swift type name of a suite (one case: `/()`), and a filter that matches nothing still prints `TEST SUCCEEDED`, so check the executed count. -- A full `TableProTests` run is not a gate: run the suites that own the types you changed. -- Release builds: `scripts/build-release.sh arm64|x86_64|both` and `scripts/create-dmg.sh`; the `release` skill runs the whole release. -- SwiftLint (`.swiftlint.yml`) is the enforced style and runs on every PR. Pass it file paths: a directory outside its `included:` scope lints nothing and reports zero violations. - -## Architecture - -**Plugins.** `PluginManager` (`Core/Plugins/`) loads bundles at runtime. `PluginDatabaseDriver` and `DriverPlugin` live in TableProPluginKit; `PluginDriverAdapter` bridges a plugin driver to the app's `DatabaseDriver`; `DatabaseDriverFactory` finds a plugin by `DatabaseType.pluginTypeId`; `DatabaseManager` owns sessions and is what views and coordinators talk to. A new protocol method goes on `PluginDatabaseDriver` with a default implementation, then into `PluginDriverAdapter`. PluginKit is ABI-resilient and its compatibility rules are in `.claude/rules/plugin-system.md`; read them before touching it. - -**DatabaseType.** A string-backed struct, not an enum. Use the static constants (`.mysql`, `.postgresql`) for known types and `DatabaseType.allKnownTypes` for the canonical list. - -**Main window.** `MainContentCoordinator` is the central coordinator, split into extension files in `Views/Main/Extensions/`. Each connection a window hosts is a `ConnectionWorkspace` with its own phase; see `.claude/rules/connection-window.md`. - -**Change tracking.** A cell edit goes to `DataChangeManager`; Save turns it into statements through `SQLStatementGenerator`; `AnyChangeManager` abstracts the concrete managers. Undo comes from `StructureChangeManager`'s private `UndoManager` and `ConnectionWorkspace.undoManager`. - -**Editor and theme.** `ThemeCatalog` owns the theme list, `ThemeResolver` picks the light and dark pair, and `ThemeEngine` holds the palette and the font caches; the fonts themselves are `TypographySettings`. A theme colors content and the window backdrop, never chrome or controls; see `.claude/rules/theme.md`. `CompletionEngine` is framework-agnostic and `QueryCompletionAdapter` bridges it to the editor. Editor tabs are drawn by `EditorTabStrip`, not native window tabs. Details: `.claude/rules/editor.md`. - -**Storage.** Passwords in the Keychain, preferences in UserDefaults, query history in SQLite FTS5, tab state as JSON. The full table of which store owns what is in `.claude/rules/data-sync-storage.md`. +- `verify.sh` logs to disk and prints `PASS`, `FAIL` or `INCONCLUSIVE`, exit 0, 1 or 2; 3 is a usage error that ran nothing. +- A Test is `` (a Swift type name), `/()`, or either behind its target, `/`. +- `INCONCLUSIVE` is never a pass. +- A run that executed nothing is `FAIL`, unless a named environment cause makes it `INCONCLUSIVE`. +- `verify.sh --help` lists every step. Steps per change: `.claude/skills/fix-issue/references/platform-apple.md`, Verify. Test names, verdicts, remedies, red CI checks: `.claude/skills/fix-issue/references/verification-apple.md`. +- A raw `xcodebuild` passes `-skipPackagePluginValidation`, plus `-testPlan Unit` or `UI` for macOS tests. Raw commands: `CONTRIBUTING.md`, Tests. +- The macOS plans own their run settings: never add `-parallel-testing-enabled`, `-test-iterations` or `-retry-tests-on-failure`, except to reproduce a flake (`.claude/rules/tests.md`). iOS tests have no plan and pass `-parallel-testing-enabled NO` (`.claude/rules/ci.md`, iOS and Package Tests). +- Releases: `RELEASING.md` and the `release` skill. ## Invariants that apply everywhere -- **The app runs the AppKit lifecycle.** `main.swift` starts it, `MainMenuBuilder.install` builds the menu bar in `applicationWillFinishLaunching`, and every window is an `NSWindowController`. Never add a SwiftUI `App`: it rewrites `NSApp.mainMenu` after launch. -- **A refresh never clears the cache it refreshes.** Fetch, then commit over the old value. Enter `.loading` only when nothing is loaded, keep the good data when a refresh fails, and use `prepareForReload` for a reload, keeping `invalidate` for disconnect. -- **Canceling a connect does not stop the driver.** `Task.cancel()` cannot interrupt a blocking C call, so a connect must be abortable (poll it, or resume through `runCancellableBlocking` and let the late call close its own handle), and every attempt checks its `ConnectionAttemptRegistry` generation before adopting a driver. +- **AppKit lifecycle.** `main.swift` starts the app; `MainMenuBuilder.install` builds the menus in `applicationWillFinishLaunching`; windows are `NSWindowController`s. A SwiftUI `App` rewrites `NSApp.mainMenu`: never add one. +- **A refresh never clears what it refreshes.** Fetch, then commit over the old value; `.loading` only when nothing is loaded; keep good data on failure; `prepareForReload` to reload, `invalidate` on disconnect. +- **A canceled connect keeps running.** `Task.cancel()` cannot interrupt a blocking C call: poll the connect or resume through `runCancellableBlocking` (the late call closes its handle), then check the `ConnectionAttemptRegistry` generation before adopting a driver. +- **Storage resolves through `AppStorageEnvironment.shared`** (stores, defaults suite, keychain); the `storage_environment_*` SwiftLint rules fail other Application Support or standard-defaults use. +- **An AppKit autosave name** (split view, window frame, table) **goes through `SplitViewAutosaveName.current(_:)`**: AppKit writes the record to the real defaults domain, which the sandbox does not redirect. +- **No new test-only branch in production code**: tests use injection or `TABLEPRO_UI_TEST_SANDBOX`. +- **macOS files the iOS app compiles** (listed in `TableProMobile/project.yml`) give each top-level declaration explicit isolation, usually `nonisolated`, since iOS defaults to `MainActor` (`scripts/ci/check-ios-shared-isolation.py`). +- **No package product in a macOS binary depends on the `TableProPluginKit` package**: a plugin would carry its own PluginKit types (`.claude/rules/plugin-system.md`). +- **A source-text rule is a SwiftLint rule or a `scripts/ci` check**, never a test reading source. ## Code style -`.swiftlint.yml` is the source of truth. Beyond it: - -- **Comments say why, never what.** A short `///` (one or two lines) belongs where the code would surprise a careful reader: a platform quirk, an invariant, a workaround and the reason for it. Never narrate what the code does, cite tickets, or describe callers. Remove a what-comment when you touch its code. -- Early returns with `guard`; small focused functions; self-explanatory names. -- Explicit access control, declared on the extension rather than on each member. -- No force unwrapping or force casting. -- OSLog (`Logger(subsystem: "com.TablePro", category: "...")`), never `print()`. -- Imports: system frameworks alphabetically, then third-party, then local. -- Approaching a SwiftLint size limit: extract into `TypeName+Category.swift`, grouped by domain. +- `.swiftlint.yml` is the source of truth. Never run `swiftlint --fix`. +- **Comments say why, never what.** A short `///` only where code would surprise a reader (a platform quirk, an invariant, a workaround's reason). Delete what-comments in code you touch. +- Early returns with `guard`, small functions, self-explanatory names. +- Explicit access control, declared on the extension. +- No force unwrap or force cast. +- Log with `Logger(subsystem: "com.TablePro", category: ...)`, never `print()`. +- Imports: one alphabetical block (`sorted_imports`). +- Booleans start with `is`, `has`, `can` or `should`; factories with `make`; acronyms all caps (`SQLStatementGenerator`). +- Near a size limit, split into `TypeName+Category.swift` by domain. `file_length` skips comment-only lines: trust `swiftlint lint --strict --no-cache `, not `wc -l`. +- Formatting beyond SwiftLint: `CONTRIBUTING.md`, Code style. ## Performance pitfalls - Never `ForEach($bindable.array) { $item in }` on an `@Observable` array that can shrink: index bindings crash. Use `ForEach(array)` and a manual binding. -- On large strings, use `(string as NSString).length` and `character(at:)`, never `.count` or `index(_:offsetBy:)` in a loop. -- Never call `ensureLayout(forCharacterRange:)`; it defeats non-contiguous layout. -- A SQL dump can have one line of millions of characters: cap regex and highlight ranges at 10k characters. +- On large strings use `(string as NSString).length` and `character(at:)`, never `.count` or `index(_:offsetBy:)` in a loop. +- Never call `ensureLayout(forCharacterRange:)`: it defeats non-contiguous layout. +- A SQL dump line can hold millions of characters: cap regex and highlight ranges at 10k. ## Every change -1. **Tests.** Unit tests for testable behavior; `TableProUITests` automation for a user flow that runs deterministically, or the reason in the PR. UI suites subclass `UITestCase`. Details: `.claude/rules/tests.md`. -2. **CHANGELOG.md.** A user-visible change gets one fragment under `[Unreleased]`, in the existing canonical section. Format: `.claude/rules/changelog.md`. -3. **Localization.** `String(localized:)` for user-facing strings outside SwiftUI literals, never with interpolation (use `String(format: String(localized: "Preview %@"), name)`). Do not localize technical terms. Plugin and package messages must be in the app catalog, managed manually: `python3 scripts/localization.py plugins --add`. Never hand-edit a `.xcstrings`; translations go through `xcodebuild -exportLocalizations` and `-importLocalizations`. -4. **Docs.** A new shortcut, UI or settings change, or driver change gets a paired draft pull request in TableProApp/docs that names this one. Its CI checks the app's `main`, so it merges after this one does; `verify.sh docs` runs the same checks against this tree first. -5. **Lint** the changed Swift files with `swiftlint lint --strict`. +1. **Tests.** Every testable change has a test in the target owning the code. Read `.claude/rules/tests.md` before creating a test file: a path rule loads only once a matching file is open. A UI flow only for a new or changed end-to-end flow (`.claude/rules/ui-tests.md`). +2. **CHANGELOG.md.** A user-visible change gets one fragment under `[Unreleased]`; format: `.claude/rules/changelog.md`. +3. **Localization.** `String(localized:)` outside SwiftUI literals, never interpolated: `String(format: String(localized: "Preview %@"), name)`. Technical terms stay unlocalized. A plugin or package string lives in the host app's catalog: add it with `python3 scripts/localization.py plugins --add`, plus `--target ios` for a package the iOS app links. Never hand-edit a `.xcstrings`. +4. **Docs.** A new shortcut, UI or settings change, or driver change gets a paired draft PR in TableProApp/docs naming this one; it merges after this one. `verify.sh docs` runs its checks here first. +5. **Lint** each changed Swift file with `verify.sh lint`. Run `verify.sh agent-docs` when `CLAUDE.md`, `AGENTS.md`, `CONTRIBUTING.md`, `RELEASING.md`, `.github/pull_request_template.md` or anything under `.claude/` changed. 6. **Atomic API changes.** A rename or signature change updates every caller and test in the same commit. -7. **Commits and pull requests.** Squash merges use the PR title as the commit subject, so the title is the record: Conventional Commits (`(): `, `!` for breaking), at most 72 characters, a canonical scope when one fits. CI checks it. Branch commits follow the same format; put the explanation in the PR description, which follows `.github/pull_request_template.md`. - - Types: `feat`, `fix`, `refactor`, `perf`, `test`, `docs`, `build`, `ci`, `chore`, `style`, `revert`. The `release` skill alone writes `release: v`. - - Scopes: `ai-chat`, `ai-providers`, `mcp`, `copilot`, `inline-suggest`, `editor`, `datagrid`, `tabs`, `coordinator`, `sidebar`, `connections`, `connection-form`, `welcome`, `settings`, `toolbar`, `hig`, `ssh`, `ios`, `windowing`, `perf`, `launch`, `plugins`, `plugin-`, `changelog`, `claude-md`, `docs`, `ci`, `release`. - - Platform scopes: `ios` for the iOS app, `linux` or `linux-` for the Linux app. `windows` is kept for a Windows app, which is why window management is `windowing`. +7. **Commits and PRs.** The PR title is the squash commit subject; branch commits use the same format. The body follows `.github/pull_request_template.md`, whose top comment holds the format, types and scopes. The `PR title` check (`scripts/ci/check-pr-title.py`) enforces the format and 72 characters, not scopes. -## Writing style +## Writing style and docs -For everything: docs, commits, CHANGELOG, UI strings, errors, PR descriptions. Short sentences, plain words, specific numbers and names. No em dashes, no filler. `scripts/banned-words.txt` is the list, and `scripts/check-banned-words.sh --staged` checks the added lines of a staged change. +- All writing: short sentences, plain words, specific numbers and names, no em dash, no filler. +- `scripts/banned-words.txt` is the list; never copy its terms. `scripts/check-banned-words.sh --staged` checks a staged change; the `PR title` job checks a PR's added lines and body. +- One home per fact; others point to it by repo path and section. No drifting counts or timings; a plan is not a fact. +- Budgets: this file 16 KiB, a rule 200 lines, a `SKILL.md` 500, a skill description 1,536 characters. +- `verify.sh agent-docs` checks backticked names in the Every change 5 files and each Area rules row against its rule's `paths:`. Never state a plugin count in prose. ## CI -- **PRs:** `macos-tests.yml`, `ios-tests.yml`, `swiftlint.yml` (pinned, `--strict`), `pr-title.yml` (Conventional Commits, at most 72 characters) and `repo-hygiene.yml` (actionlint, shellcheck and the repo's source-scanning checks). -- **Releases:** `build.yml` runs on `v*` tags and produces the DMG, the tar.xz update archive, its deltas and the Sparkle feed, with notes taken from `CHANGELOG.md`. -- **Plugins:** `build-plugin.yml` runs on `plugin--v*` tags, where the slug is a key in `.github/plugin-registry.json`. +- A PR merges on five green checks: macOS Tests, iOS and Package Tests, SwiftLint, Repo Hygiene and PR title. Only release deploy keys bypass them (`RELEASING.md`, Keys and rulesets). +- Read why a CI check failed before any re-run. +- Workflows: `.claude/rules/ci.md`. Red checks: `.claude/skills/fix-issue/references/verification-apple.md`, Reading CI. ## Area rules -| Rule | Covers | +Claude Code loads a rule when it reads or edits a file the rule covers. Under Codex, before editing a file, read every rule whose row matches it. Each row is the rule's `paths:` with a trailing `/**/*` dropped; `verify.sh agent-docs` keeps them equal. + +| Rule | Read before editing | | --- | --- | -| `ai-mcp-security.md` | `TablePro/Core/AI`, `TablePro/Core/MCP` | +| `ai-mcp-security.md` | `TablePro/Core/AI`, `TablePro/Core/MCP`, `Packages/TableProMacCore/Sources/TableProMCP` | | `changelog.md` | `CHANGELOG.md` | -| `connection-window.md` | connection windows, workspaces, sessions, `DatabaseManager` | -| `data-grid.md` | the data grid under `TablePro/Views/Results` | -| `data-sync-storage.md` | storage, CloudKit sync, the database core | -| `editor.md` | `Packages/TableProEditor`, editor views, fonts | -| `libs.md` | `Libs/` and the libs scripts | -| `mongodb-driver.md`, `mysql-driver.md`, `redis-driver.md` | those drivers | -| `native-bridges.md` | `Native/` and the Dameng and HANA plugins | -| `plugin-system.md` | `Plugins/`, TableProPluginKit, `project.yml`, plugin CI | -| `split-views.md` | split view controllers and workspace panes | -| `tests.md` | `TableProTests`, `TableProUITests` | -| `theme.md` | `TablePro/Theme`, every view a theme reaches, content windows | -| `ui-lifecycle.md` | views, view models, UI tests | -| `welcome-library.md` | the welcome window's connection list | - -Skills live in `.claude/skills/`; Codex sees them through `.agents/skills/`. `fix-issue` takes an issue to a pull request; `release` ships a version. +| `ci.md` | `.github/workflows`, `.github/actions`, `.github/scripts`, `.github/actionlint.yaml`, `.github/*baseline*`, `scripts/ci`, `.swiftlint.yml`, `**/.swiftlint.yml` | +| `connection-window.md` | `TablePro/Core/Services/Infrastructure`, `TablePro/Models/Connection`, `TablePro/Core/Database/DatabaseManager*.swift`, `Packages/TableProCore/Sources/TableProDatabase`, `TablePro/Views/Main` | +| `data-grid.md` | `TablePro/Views/Results`, `TablePro/Core/DataGrid`, `TableProTests/Views/Results` | +| `data-sync-storage.md` | `TablePro/Core/**/*Storage*.swift`, `TablePro/Core/**/*Sync*.swift`, `TablePro/Core/Database`, `TablePro/Core/Plugins/PluginMetadataRegistry*.swift`, `TablePro/Core/Services/Query`, `Packages/TableProCore/Sources/TableProSyncTransport`, `CloudKit` | +| `editor.md` | `Packages/TableProEditor`, `TablePro/Theme`, `TablePro/Views/Editor`, `TablePro/Views/RowInspector` | +| `libs.md` | `Libs`, `scripts/download-libs.sh`, `scripts/publish-libs.sh`, `scripts/publish-ios-libs.sh` | +| `mongodb-driver.md` | `Plugins/MongoDBDriverPlugin`, `Plugins/MQLExportPlugin` | +| `mysql-driver.md` | `Plugins/MySQLDriverPlugin`, `Packages/TableProCore/Sources/TableProMySQLCore`, `TableProMobile/**/*MySQL*`, `TableProMobile/**/*MariaDB*` | +| `native-bridges.md` | `Native`, `Plugins/DamengDriverPlugin`, `Plugins/HanaDriverPlugin`, `scripts/build-dameng.sh`, `scripts/build-hana.sh` | +| `plugin-system.md` | `Plugins`, `Packages/TableProCore/Sources/TableProPluginKit`, `Packages/TableProCore/Sources/TableProMySQLCore`, `Packages/TableProCore/Sources/TableProPostgreSQLCore`, `Packages/TableProCore/Sources/TableProRedisCore`, `Packages/*/Package.swift`, `project.yml`, `TableProMobile/project.yml`, `.github/workflows/build-plugin.yml`, `.github/plugin-registry.json`, `scripts/*plugin*` | +| `redis-driver.md` | `Plugins/RedisDriverPlugin`, `Packages/TableProCore/Sources/TableProRedisCore` | +| `split-views.md` | `TablePro/**/*Split*.swift`, `TablePro/Core/Services/Infrastructure/WorkspacePane*.swift` | +| `tests.md` | `TableProTests`, `TableProUITests`, `Plugins/*Tests`, `Packages/*/Tests`, `TableProMobile/TableProMobileTests`, `TestPlans`, `scripts/ci/check-test-plans.py`, `scripts/clean-test-residue.sh` | +| `theme.md` | `TablePro/Theme`, `TablePro/Views`, `TablePro/Core/Services/Infrastructure/*Window*` | +| `ui-lifecycle.md` | `TablePro/Views`, `TablePro/ViewModels`, `TablePro/Core/Services/Infrastructure`, `Packages/TableProEditor` | +| `ui-tests.md` | `TableProUITests`, `TestPlans/UI.xctestplan` | +| `welcome-library.md` | `TablePro/Views/Welcome`, `TablePro/ViewModels/WelcomeViewModel*.swift`, `Packages/TableProCore/Sources/TableProConnectionLibrary` | + +## Skills + +`fix-issue` takes a problem to one green PR and holds `verify.sh` and its references; `release` ships a version. Both live in `.claude/skills/`; Codex sees them through `.agents/skills/`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5eae5f26cd..27f14b5b91 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,15 +1,14 @@ # Contributing to TablePro -[`AGENTS.md`](AGENTS.md) holds the architecture invariants and the rules every change has to meet; where this file and it disagree, it wins. The docs site lives in [TableProApp/docs](https://github.com/TableProApp/docs), and releasing is in [`RELEASING.md`](RELEASING.md). +[`AGENTS.md`](AGENTS.md) holds the invariants and the rules every change meets; where this file and it disagree, it wins. The docs site lives in [TableProApp/docs](https://github.com/TableProApp/docs), and releasing is in [`RELEASING.md`](RELEASING.md). ## Setup | Software | Version | Notes | -|----------|---------|-------| -| Xcode | 26.0+ | The app calls macOS 26 SDK symbols behind `#available`. The deployment target stays macOS 13.0. Every CI job pins 26.4.1 through `.github/actions/setup-xcode` | -| XcodeGen | 2.46.0 exactly | Generated `pbxproj` output differs between versions, so `scripts/generate-project.sh` warns on any other | -| SwiftLint | any | The pull request gate runs `swiftlint lint --strict` | -| SwiftFormat | any | Not run in CI | +|---|---|---| +| Xcode | 27.1 | What CI selects in `.github/actions/setup-xcode`. Code that needs the macOS 27 SDK sits behind `SDK_MACOS_27` (`Configs/Base.xcconfig`) so Xcode 26 can still build, though nothing on CI builds with 26. Newer SDK symbols sit behind `#available`; the deployment target is macOS 13.0 | +| XcodeGen | 2.46.0 exactly | The generated project differs between versions, so `scripts/generate-project.sh` warns on any other | +| SwiftLint | 0.65.1 | The version CI runs | | GitHub CLI | any | Optional: `scripts/download-libs.sh` falls back to `curl` | | Go | 1.27.1 | SAP HANA plugin only: run `scripts/build-hana.sh` with no argument before building `HanaDriver` or `AllPlugins`. The script fetches 1.27.1 when the installed `go` is another release | | Rust | 1.91.1 | Dameng plugin only, through [rustup](https://rustup.rs): run `scripts/build-dameng.sh arm64` before building `DamengDriver` or `AllPlugins` | @@ -18,7 +17,7 @@ Fork the repo on GitHub, then: ```bash git clone https://github.com//TablePro.git && cd TablePro -brew install xcodegen swiftlint swiftformat +brew install xcodegen swiftlint scripts/download-libs.sh cp Configs/Secrets.xcconfig.example Configs/Secrets.xcconfig scripts/generate-project.sh @@ -29,26 +28,18 @@ Select the **TablePro** scheme and **My Mac**, then press `Cmd+R`. ### Static libraries -`scripts/download-libs.sh` fetches two archives from the `libs-v1` GitHub Release: the macOS `.a` files (libmariadb, libpq, libduckdb, libmongoc and the rest) and the iOS xcframeworks, 257 MB downloaded and 621 MB on disk. None of it is in git, so a build that skips this step fails at link time. Every run, including one that downloads nothing, verifies `Libs/` against the checksums committed at `HEAD`. +`scripts/download-libs.sh` fetches two archives from the `libs-v1` GitHub Release: the macOS `.a` files (libmariadb, libpq, libduckdb, libmongoc and the rest) and the iOS xcframeworks. None of it is in git, so a build that skips this step fails at link time. Every run, including one that downloads nothing, verifies `Libs/` against the checksums committed at `HEAD`. Publishing a rebuilt library is in `RELEASING.md`, Static libraries. -### Signing with your own team +## Building with a personal Apple team Edit `Configs/Secrets.xcconfig`. The template ships the literal `YOUR_TEAM_ID`, which fails to sign the same way a missing file does: ``` -TABLEPRO_DEVELOPMENT_TEAM = YOUR_TEAM_ID +TABLEPRO_DEVELOPMENT_TEAM = TABLEPRO_APP_BUNDLE_IDENTIFIER = com..TablePro ``` -A free personal team works. Debug builds sign against `TablePro/TablePro.Debug.entitlements`, which drops iCloud, so sync turns itself off at runtime. Plugin bundles and the MCP helper sign ad-hoc. The file is gitignored, so these settings never reach a commit and survive regenerating the project. Signing changed in the Xcode UI does not last: the next `scripts/generate-project.sh` discards it. - -To check it works, save a connection password, relaunch, and reopen the connection. The password should still be there. - -### The generated project - -`scripts/generate-project.sh` writes `TablePro.xcodeproj` and `TableProMobile/TableProMobile.xcodeproj` from their `project.yml` specs; pass `macos` or `ios` for one. Neither project is in git. Run it again after editing a `project.yml` or anything in `Configs/`, and after adding, moving or deleting a source file: XcodeGen collects sources at generation time. Never hand-edit a generated project. - -The Swift language mode comes from `SWIFT_VERSION` in `Configs/Base.xcconfig`, and each package under `Packages/` sets its own. Never pass `SWIFT_VERSION=` to `xcodebuild`: it reaches the packages too and reports their errors as yours. +A free personal team works. Debug builds sign against `TablePro/TablePro.Debug.entitlements`, which drops iCloud, so sync turns itself off. Plugin bundles and the MCP helper sign ad hoc. The file is gitignored, so these settings never reach a commit and survive regenerating the project. Signing changed in the Xcode UI does not: the next `scripts/generate-project.sh` discards it. To check it, save a connection password, relaunch, and reopen the connection: the password is still there. ## Building @@ -56,13 +47,11 @@ The Swift language mode comes from `SWIFT_VERSION` in `Configs/Base.xcconfig`, a xcodebuild -project TablePro.xcodeproj -scheme TablePro -configuration Debug build -skipPackagePluginValidation ``` -`-skipPackagePluginValidation` belongs on every `xcodebuild` invocation. Xcode writes the app to DerivedData; the command line writes it to `build/Debug/` inside the repo. - -Debug builds sign with the team from `TABLEPRO_DEVELOPMENT_TEAM`, and that matters locally too: a plugin in the user plugins directory has to carry the same team as the running app. +Xcode and the command line both write the app to `~/Library/Developer/Xcode/DerivedData/TablePro-*/Build/Products/Debug/TablePro.app`. When to regenerate the project, and why never to pass `SWIFT_VERSION=`: [`AGENTS.md`](AGENTS.md), Commands and Repository map. Debug builds sign with the team from `TABLEPRO_DEVELOPMENT_TEAM`, and a plugin in the user plugins folder must carry the same team as the running app. ### Every plugin -The app scheme compiles only the plugins the app embeds. The `AllPlugins` aggregate compiles all of them, and is the check to run after changing anything under `Plugins/TableProPluginKit/`: +The app scheme compiles only the plugins the app embeds. The `AllPlugins` aggregate compiles all of them; when to run it is in `.claude/rules/plugin-system.md`, Building. ```bash xcodebuild build -project TablePro.xcodeproj -scheme AllPlugins -destination 'platform=macOS' -skipPackagePluginValidation CODE_SIGNING_ALLOWED=NO @@ -80,128 +69,67 @@ xcodebuild build -project TablePro.xcodeproj -scheme AllPlugins -destination 'pl ## Tests -```bash -xcodebuild -project TablePro.xcodeproj -scheme TablePro test -skipPackagePluginValidation -only-testing:TableProTests/ -``` - -Tests are Swift Testing. `-only-testing` matches the suite's Swift type name, or `/()` for one case. A filter that matches nothing still reports success, so check how many tests ran. `-only-testing:TableProUITests` runs the UI automation, whose suites subclass `UITestCase`. Run the suites that own the types you changed; a full run is not a gate. - -## Code style +Prefer `.claude/skills/fix-issue/scripts/verify.sh test ` ([`AGENTS.md`](AGENTS.md), Commands): it finds the target that declares the suite and fails a run that executed nothing. Where a test goes and how to write one: `.claude/rules/tests.md`; UI flows: `.claude/rules/ui-tests.md`. -`.swiftlint.yml` is the source of truth, and CI runs it on every pull request: +The raw equivalents, in order: a suite in the Unit plan, a UI flow (needs Automation Mode, see Troubleshooting), a TableProCore or TableProOracle suite (no Xcode plan lists those two packages), and the iOS app: ```bash -swiftlint lint --strict +xcodebuild -project TablePro.xcodeproj -scheme TablePro test -testPlan Unit -skipPackagePluginValidation -only-testing:/ +xcodebuild -project TablePro.xcodeproj -scheme TablePro test -testPlan UI -skipPackagePluginValidation -only-testing:TableProUITests/ +swift test --package-path Packages/TableProCore --filter '.' +xcodebuild -project TableProMobile/TableProMobile.xcodeproj -scheme TableProMobile test -only-testing:TableProMobileTests/ \ + -destination 'platform=iOS Simulator,name=' -parallel-testing-enabled NO -skipPackagePluginValidation CODE_SIGNING_ALLOWED=NO ARCHS=arm64 ``` -`--strict` turns warnings into errors, and several rules are warnings on purpose, so a plain `swiftlint lint` passes code CI rejects. Nothing lints during an Xcode build. The scope is `included: [TablePro]`, and that key beats a path on the command line: `swiftlint lint Plugins/` lints `TablePro/` and exits 0. Do not run `swiftlint --fix` unreviewed; check its diff. +## Code style + +`.swiftlint.yml` is the source of truth. Run `swiftlint lint --strict `; CI lints the whole repo the same way on every pull request, and nothing lints during an Xcode build. Formatting the linter leaves to you: | Rule | Value | |---|---| -| Indentation | 4 spaces; `case` not indented inside `switch` | -| Line length | Wrap at 120; SwiftLint warns at 180 and errors at 300 | -| Braces | K&R, `else` on the closing brace line | +| Indentation | 4 spaces | +| Line length | Wrap at 120 | +| Braces | K&R | | Wrapped arguments | One per line | -| Trailing commas, semicolons | None | +| Trailing commas | None | | `self.` | Only where the compiler needs it | -| Booleans | `is`, `has`, `can` or `should` prefix | -| Factory methods | `make` prefix | -| Acronyms | All caps, as in Apple's APIs: `SQLStatementGenerator`, `MCPAuditLogger` | -| Imports | One alphabetical block, no blank lines inside it | - -Beyond the linter: -- Explicit access control, declared on the extension rather than on each member. -- No force unwraps (`!`) or force casts (`as!`). `force_try` is disabled in SwiftLint; the rule still applies. -- OSLog, never `print()`. -- Comments say why, never what. -- Storage goes through `AppStorageEnvironment.shared`: two custom rules fail on `applicationSupportDirectory` and `UserDefaults.standard` anywhere else, because a UI test would otherwise write into your own store. -- A type nearing a size limit splits into `TypeName+Category.swift` files under an `Extensions/` folder beside it, grouped by domain. `MainContentCoordinator` is the worked example. -- User-facing strings use `String(localized:)`, never with interpolation: `String(localized: "Preview \(name)")` builds a new key on every call and ships untranslated. Use `String(format: String(localized: "Preview %@"), name)`. Font names, database types and SQL keywords stay unlocalized. +Naming, imports, access control, force unwrapping, logging, comments, storage, file size and localization: [`AGENTS.md`](AGENTS.md), Code style, Invariants that apply everywhere, and Every change. ## Architecture -Every database driver is a bundle the app loads at runtime, so nothing switches on database type and `DatabaseType` is a string-backed struct, not an enum. The UI is SwiftUI on an AppKit lifecycle, async work is Swift concurrency, and database connectivity is native C libraries linked from `Libs/`. - -| Component | Location | Role | -|-----------|----------|------| -| TableProPluginKit | `Plugins/TableProPluginKit/` | The plugin protocols and transfer types, ABI-resilient | -| PluginManager | `TablePro/Core/Plugins/` | Discovers, validates and loads plugin bundles | -| PluginDriverAdapter | `TablePro/Core/Plugins/` | Bridges a plugin driver to the app's `DatabaseDriver` | -| DatabaseManager | `TablePro/Core/Database/` | Owns connections and sessions; what views and coordinators talk to | -| MainContentCoordinator | `TablePro/Views/Main/` | One per connection session, split across `Views/Main/Extensions/` | -| DataChangeManager, SQLStatementGenerator | `TablePro/Core/ChangeTracking/` | Pending cell edits, and the statements Save turns them into | -| CompletionEngine | `TablePro/Core/Autocomplete/` | Ranks completions; `QueryCompletionAdapter` in `Views/Editor/` bridges it to the editor | -| MCP server | `TablePro/Core/MCP/` | Loopback-only HTTP; its contract is documented under [developers](https://docs.tablepro.app/developers/mcp-protocol) | - -| Store | Holds | -|---|---| -| Keychain | Connection passwords | -| UserDefaults | Preferences, favorites, recents | -| SQLite FTS5 | Query history | -| JSON files | Tab state, per-table filters | +The architecture and its invariants: [`AGENTS.md`](AGENTS.md), Architecture and Invariants that apply everywhere. | Dependency | Source | Purpose | |---|---|---| -| TableProEditor | `Packages/`, a fork of CodeEditSourceEditor and CodeEditTextView | The SQL editor | +| TableProEditor | `Packages/`, vendored from CodeEditSourceEditor and CodeEditTextView | The SQL editor | | TableProGrammars | `Packages/` | Tree-sitter grammars | | TableProCore | `Packages/` | Modules the macOS and iOS apps share | +| TableProMacCore | `Packages/` | macOS-only modules the app links: utilities, autocomplete, SSH config, MCP | | TableProOracle | `Packages/`, wraps a pinned fork of OracleNIO | Oracle wire protocol | | Sparkle | SPM | Updates, EdDSA-signed | -| swift-certificates | SPM | X.509 types | | Yams | SPM | YAML for project folder import | - -`Plugins/DamengDriverPlugin` links a Rust bridge from `Native/DamengBridge`. `Plugins/HanaDriverPlugin` runs `tablepro-hana-helper`, a Go executable from `Native/HanaBridge`, as one process per session and talks JSON over stdin and stdout. - -## Project layout - -``` -project.yml Xcode project definition (XcodeGen); the .xcodeproj is generated, not in git -Configs/ Shared build settings (.xcconfig), app version, signing template -TablePro/ App source: Core/, Views/, Models/, ViewModels/, Extensions/, Theme/ -Plugins/ .tableplugin bundles and the TableProPluginKit framework -Packages/ Local SwiftPM packages -Native/ Rust and Go bridges for the Dameng and SAP HANA plugins -TableProMobile/ iOS app, widget extension, and its own project.yml -Libs/ Prebuilt static libraries, downloaded by script, not in git -TableProTests/ Unit tests -TableProUITests/ UI automation -scripts/ Build, release and library scripts -``` - -`Packages/TableProCore/Sources/TableProPluginKit` is a symlink to `Plugins/TableProPluginKit`. Edit the files under `Plugins/` only. - -## Commits - -[Conventional Commits](https://www.conventionalcommits.org/). Pull requests are squash-merged with the PR title as the commit subject, so the title is what lands on `main`: at most 72 characters, checked by CI. Explain the change in the PR description, not in commit bodies. - -``` -feat: add CSV export for query results -fix: prevent crash on empty query result -``` +| swift-nio, swift-nio-ssl, zstd | SPM | The Kafka plugin's transport, TLS and zstd codec | ## Branch naming -Branch off `main`: - -- `feat/add-cassandra-support` -- `fix/query-editor-crash` +Branch off `main`: `feat/add-cassandra-support`, `fix/query-editor-crash`. ## Pull requests One logical change per PR, described with the template the PR form opens with. Before you open it: - Tests added or updated. -- `CHANGELOG.md` updated under `[Unreleased]` (skip for a fix to something still unreleased). Leave the credit off: the release adds `(#123 by @you)` to every entry from your pull request. -- User-facing behavior documented in a paired pull request in [TableProApp/docs](https://github.com/TableProApp/docs) that links this one. It merges after this one does, because its CI checks the app's `main`. +- `CHANGELOG.md` updated under `[Unreleased]`, with the credit left off: the release adds it (`.claude/rules/changelog.md`). +- User-facing behavior documented in a paired pull request in [TableProApp/docs](https://github.com/TableProApp/docs) that links this one. - User-facing strings localized. - SwiftLint clean. -On every pull request, `macos-tests.yml` builds every plugin, loads each built bundle, then runs the package tests, `TableProTests` and the UI tests in three shards; its first job skips the suites when nothing relevant changed. `ios-tests.yml` covers the iOS app, `swiftlint.yml` the linter, `pr-title.yml` the title, and `repo-hygiene.yml` runs actionlint, shellcheck and the repo's source-scanning checks. +The title format, types and scopes are in the top comment of `.github/pull_request_template.md`. A PR merges when the required checks in [`AGENTS.md`](AGENTS.md), CI, pass; what each workflow runs is in `.claude/rules/ci.md`. ## Adding a database driver -Drivers are `.tableplugin` bundles loaded at runtime. Create a new bundle under `Plugins/`, implement `DriverPlugin` and `PluginDatabaseDriver` from `TableProPluginKit`, and add the target to `project.yml`. Guides: [Plugin Development](https://docs.tablepro.app/development/plugin-development) and [Plugin Registry](https://docs.tablepro.app/development/plugin-registry). +A new engine ships as a registry plugin: a `.tableplugin` bundle under `Plugins/` that implements `DriverPlugin` and `PluginDatabaseDriver` from `TableProPluginKit`, with a target in `project.yml`. The checklist is in `.claude/rules/plugin-system.md`, Structure. Guides: [Plugin Development](https://docs.tablepro.app/development/plugin-development) and [Plugin Registry](https://docs.tablepro.app/development/plugin-registry). ## Translating @@ -215,9 +143,7 @@ xcodebuild -importLocalizations -project TablePro.xcodeproj -localizationPath Lo scripts/localization.py status # what is translated, per language ``` -The export builds the app first, so run `scripts/download-libs.sh` and `scripts/generate-project.sh` once before it. Pass an existing language code to fill gaps, or a new one to start a language. Commit the catalog, not the export: `Localization/` is ignored. The iPhone and iPad app has a catalog of its own: run the same commands on `TableProMobile/TableProMobile.xcodeproj` with the `TableProMobile` scheme. - -Strings from plugins and from packages under `Packages/` live in the app's catalog too, marked "Managed Manually" so that Xcode keeps them. `scripts/localization.py plugins --add` adds new ones. +The export builds the app first, so run `scripts/download-libs.sh` and `scripts/generate-project.sh` once before it. Pass an existing language code to fill gaps, or a new one to start a language. Commit the catalog, not the export: `Localization/` is ignored. The iPhone and iPad app has its own catalog: run the same commands on `TableProMobile/TableProMobile.xcodeproj` with the `TableProMobile` scheme. Plugin and package strings: [`AGENTS.md`](AGENTS.md), Every change 3. ## Troubleshooting @@ -231,15 +157,12 @@ Strings from plugins and from packages under `Packages/` live in the app's catal | `Build input file cannot be found: '…/tablepro-hana-helper'`, or the helper has no slice for your architecture | Run `scripts/build-hana.sh` with no argument, which builds both architectures | | Signing errors after a fresh clone | `Configs/Base.xcconfig` defaults to the maintainer's team and includes `Secrets.xcconfig` optionally, so a build without it still runs and fails to sign. Create the file with your team | | Package resolution fails | `Cmd+Shift+K` and reopen Xcode; then `rm -rf ~/Library/Developer/Xcode/DerivedData` | +| `Timed out while enabling automation mode`, and UI tests run nothing | Answer the Enable UI Automation prompt with an admin password, or run `sudo automationmodetool enable-automationmode-without-authentication` once | +| Test runs left defaults domains (`com.TablePro.tests.*`, `com.TablePro.uitest.*`) or temp folders behind | Quit TablePro, then run `scripts/clean-test-residue.sh`. It only reports until you pass `--apply` | ## Reporting bugs -Open a [GitHub issue](https://github.com/TableProApp/TablePro/issues) with: - -- macOS version -- TablePro version -- Reproduction steps -- Database type and version (for database-specific bugs) +Open a [GitHub issue](https://github.com/TableProApp/TablePro/issues) with your macOS version, TablePro version, steps to reproduce, and, for a database-specific bug, the database type and version. ## License diff --git a/Plugins/DamengDriverPlugin/README.md b/Plugins/DamengDriverPlugin/README.md index 600710c6d7..ba140a4b52 100644 --- a/Plugins/DamengDriverPlugin/README.md +++ b/Plugins/DamengDriverPlugin/README.md @@ -25,18 +25,6 @@ xcodebuild -project TablePro.xcodeproj -scheme DamengDriver \ Use `scripts/build-dameng.sh arm64` or `x86_64` only for architecture-specific diagnostics. Do not commit generated files under `Libs/` or `target/`. -## Tests - -Run protocol, bridge, and Swift tests before submitting a change: - -```bash -(cd Native/DamengBridge && cargo test --locked) -xcodebuild -project TablePro.xcodeproj -scheme DamengDriverTests \ - -configuration Debug build-for-testing CODE_SIGNING_ALLOWED=NO -``` - -Integration tests require an isolated DM8 server, such as a container running in OrbStack. Set `TABLEPRO_DM8_INTEGRATION=1`, `DM_HOST`, `DM_PORT`, `DM_USER`, and `DM_PASSWORD`, then run the built `DamengDriverTests.xctest` bundle with `xcrun xctest`. Tests create a unique temporary schema and remove it afterward. - ## Security and Limitations The parameter binder recognizes placeholders only in SQL code, escapes text literals, and encodes bytes with `HEXTORAW`. Response bodies and LOB content are capped at 64 MiB. Native TLS is not available; use SSH, SOCKS, or Cloudflare tunneling for untrusted networks. Native binary and off-row LOB reads remain unsupported by the transport; use `RAWTOHEX` for binary values and cast CLOB values to `VARCHAR`. diff --git a/Plugins/HanaDriverPlugin/README.md b/Plugins/HanaDriverPlugin/README.md index f2c7d3f677..f78fc008df 100644 --- a/Plugins/HanaDriverPlugin/README.md +++ b/Plugins/HanaDriverPlugin/README.md @@ -4,7 +4,7 @@ This plugin connects TablePro to SAP HANA Cloud and SAP HANA 2.0 through [SAP/go The Go bridge in `Native/HanaBridge` is built by `scripts/build-hana.sh` into `tablepro-hana-helper`, an executable with no cgo. The plugin carries it in `Contents/MacOS`, starts one helper for each session, stops it when the session closes, and talks to it over stdin and stdout in length-prefixed JSON frames. The metadata connection pool opens drivers of its own, so one connection can run several helpers. A crash in go-hdb ends that driver's session, not TablePro. The helper owns the HANA session and returns JSON, and the Swift side writes every message the user reads. -Xcode copies the helper into the bundle and does not build it. Run `scripts/build-hana.sh` with no argument, which builds a universal arm64 and x86_64 helper, before building `HanaDriver`, `HanaDriverTests` or `AllPlugins`. A thin build, `scripts/build-hana.sh arm64` or `x86_64`, only works in a build of that one architecture. `scripts/build-plugin.sh` signs the helper with Developer ID, the hardened runtime and a timestamp before it signs the plugin. +Xcode copies the helper into the bundle and does not build it. Run `scripts/build-hana.sh` with no argument, which builds a universal arm64 and x86_64 helper, before building `HanaDriver` or `AllPlugins`. A thin build, `scripts/build-hana.sh arm64` or `x86_64`, only works in a build of that one architecture. `scripts/build-plugin.sh` signs the helper with Developer ID, the hardened runtime and a timestamp before it signs the plugin. What the plugin does: diff --git a/RELEASING.md b/RELEASING.md index 2f8c4d48de..d5b16bcb55 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -1,179 +1,189 @@ # Releasing -`scripts/build-release.sh` is the whole release build. It signs every binary in the bundle itself, -each plugin, helper and framework first and the app bundle last, so signing again by hand -afterwards undoes its work. +`scripts/build-release.sh` is the whole release build: it signs every nested binary (plugins, +helpers, frameworks) and the app bundle last, so signing again by hand undoes its work. -Everything here needs credentials the maintainer holds. A contributor building from source wants -[CONTRIBUTING.md](CONTRIBUTING.md#building) instead. The `release` skill in `.claude/skills/release` -runs a whole release, from the version bump to the announcement, on top of what this file describes. +Everything here needs the maintainer's credentials; a contributor wants `CONTRIBUTING.md`, Building. +The `release` skill in `.claude/skills/release` runs a release on top of this file and owns the +procedure and its decisions. ## Release builds ```bash -scripts/build-release.sh arm64 # Apple silicon -scripts/build-release.sh x86_64 # Intel -scripts/build-release.sh both # both, the default with no argument +scripts/build-release.sh arm64 # or x86_64, or both (the default with no argument) ``` Output lands in `build/Release/TablePro-arm64.app` and `build/Release/TablePro-x86_64.app`. -The script extracts the matching slice from the universal static libraries, copies every -non-system dynamic library the binaries reference into `Contents/Frameworks` and rewrites their -install names, strips the main binary and the helpers, then signs with hardened runtime and -`TablePro/TablePro.entitlements`. +The script extracts the matching slice from the universal static libraries, copies every non-system +dynamic library the binaries reference into `Contents/Frameworks` and rewrites their install names, +strips the binaries, then signs with hardened runtime and `TablePro/TablePro.entitlements`. | Variable | Default | Purpose | |---|---|---| -| `SIGN_IDENTITY` | The maintainer's Developer ID Application identity | Codesign identity for every binary | -| `NOTARIZE` | `false` | Set `true` to notarize and staple after building | +| `SIGN_IDENTITY` | the maintainer's Developer ID Application | codesign identity for every binary | +| `NOTARIZE` | `false` | `true` notarizes and staples after the build | | `NOTARY_PROFILE` | `TablePro` | `notarytool` keychain profile holding the credentials | -Create the notarization profile once: - -```bash -xcrun notarytool store-credentials "TablePro" --apple-id … --team-id … --password … -``` - -Notarization zips each app, submits it, waits, and staples the ticket. `scripts/lib/notarize.sh` -reads both the exit status and the reported status: `notarytool` has exited 0 on a rejected -submission. +Create the notary profile once with `xcrun notarytool store-credentials "TablePro"` plus +`--apple-id`, `--team-id` and `--password`. `scripts/lib/notarize.sh` zips, submits, waits and +staples. It reads both the exit status and the reported status, because `notarytool` has exited 0 +on a rejected submission. ## Packaging a DMG -`scripts/create-dmg.sh` takes a version, an architecture (default `universal`) and a source app -(default `build/Release/TablePro.app`). With no version it reads `MARKETING_VERSION` from -`Configs/Version.xcconfig`, which is the only place the app version is declared. +`scripts/create-dmg.sh [source app]` packages one architecture. The source +app defaults to the one `build-release.sh` wrote for that architecture. An empty version reads +`MARKETING_VERSION` from `Configs/Version.xcconfig`, the only place the app version is declared. ```bash -scripts/build-release.sh arm64 -scripts/create-dmg.sh 0.67.0 arm64 build/Release/TablePro-arm64.app +scripts/create-dmg.sh 0.80.0 arm64 # writes build/Release/TablePro-0.80.0-arm64.dmg ``` -That writes `build/Release/TablePro-0.67.0-arm64.dmg`. The script prefers `create-dmg` -(`brew install create-dmg`) and falls back to `hdiutil` when it is missing. +It prefers `create-dmg` (`brew install create-dmg`) and falls back to `hdiutil`. `NOTARIZE=true` +notarizes the DMG too. ## Static libraries The `.a` files under `Libs/` and the xcframeworks under `Libs/ios/` live on the `libs-v1` GitHub -Release, not in git. Each has a checksum baseline committed beside it, and +release, not in git. Each set has a checksum baseline committed beside it, and `scripts/download-libs.sh` verifies the working copy against the baseline at `HEAD` on every run, -including runs that download nothing. - -That means an archive and its baseline have to move together. Publish through the scripts, which do -both: +including runs that download nothing. An archive and its baseline move together, so publish through +the scripts, which do both: ```bash scripts/publish-libs.sh libmongoc_arm64.a libmongoc_x86_64.a libmongoc_universal.a libmongoc.a git add Libs/checksums.sha256 && git commit -m "build: update static library checksums" ``` -Name every library you rebuilt. `publish-libs.sh` verifies each library you did *not* name against -the checksums at `HEAD`, regenerates `Libs/checksums.sha256`, and uploads the archive with -`--clobber`. `libs-v1` predates release immutability, so GitHub keeps it mutable. Both publish -scripts stop before touching a checksum file if it ever reports immutable. - -The iOS xcframeworks go through their own script, which takes `--dry-run` and refuses a publish -that changes nothing: - -```bash -scripts/publish-ios-libs.sh -git add Libs/ios/checksums.sha256 && git commit -m "build: update iOS xcframework checksums" -``` - -Never write `shasum -a 256 Libs/*.a > Libs/checksums.sha256` by hand. Regenerating from a stale -`Libs/` silently reverts every library you did not rebuild, which shipped a broken libmongoc and -rolled DuckDB back a version. Uploading the xcframeworks with a bare `tar` and `gh release upload` -fails from the other side: 299 MB published with no baseline behind it, and the next -`download-libs.sh` run on every machine and every CI job stops. - -## Shipping a version - -`.github/workflows/build.yml` fires on a `v*` tag push, and otherwise only on a manual dispatch. Bump -`Configs/Version.xcconfig` first: the release job compares the tag against `MARKETING_VERSION` and -fails with `tag v… does not match MARKETING_VERSION …` when they disagree. It also fails when -`CURRENT_PROJECT_VERSION` is not above the newest published release's. That number is what Sparkle -compares as ``, so a flat build number ships a release no installed app is offered. - -`main` takes no direct push. The release commit is the one exception: it touches only -`Configs/Version.xcconfig`, `CHANGELOG.md`, `TablePro/Resources/WhatsNew.md` and -`.github/release-flags.json`, and goes up with the release deploy key, the only bypass in main's -ruleset: - -```bash -GIT_SSH_COMMAND='ssh -i ~/.ssh/tablepro_release -o IdentitiesOnly=yes' git push origin main -``` - -The key has a passphrase and is never added to ssh-agent, so nothing that runs on this Mac without -you can push with it. - -The release job needs three jobs: Tested on main, Build (a matrix over `arm64` and `x86_64`) and -Registry Readiness. Tested on main runs no suite. For `macOS Tests` and `iOS and Package Tests` each, -it walks back the first-parent commits on `main` past those where the job skipped, waits for the -newest run when it is still going, and fails when that run is red or when files the suite reads -changed after it. The release commit changes no such file, so its own push skips both suites. A red -suite or a registry with no plugin binary compatible with the shipping app blocks the tag. - -The release job then verifies the artifacts, signs both update archives with the Sparkle key and -extracts the release notes from `CHANGELOG.md`. It creates the GitHub release as a draft with both -DMGs and both ZIPs on it, publishes the draft in one step, and only then commits `appcast.xml`, whose -download URLs 404 until the release is public. That push uses `APPCAST_DEPLOY_KEY`, a deploy key -held by the `release` environment, which only `v*` tags can use. A tag containing `-alpha`, `-beta` -or `-rc` publishes as a prerelease. +- Name every library you rebuilt. `publish-libs.sh` verifies each one you did not name against the + checksums at `HEAD`, regenerates `Libs/checksums.sha256` and uploads the archive with `--clobber`. +- The iOS xcframeworks go through `scripts/publish-ios-libs.sh`, then a commit of + `Libs/ios/checksums.sha256`. It takes `--dry-run` and refuses a publish that changes nothing. +- `libs-v1` predates release immutability, so GitHub keeps it mutable. Both scripts stop before + touching a checksum file if it ever reports immutable. +- Never write a checksum file by hand. Regenerating one from a stale `Libs/` reverts every library + you did not rebuild: that shipped a broken libmongoc and rolled DuckDB back a version. +- Never upload with a bare `tar` and `gh release upload`. That put 299 MB on the release with no + baseline behind it, and the next `download-libs.sh` run stopped on every machine and CI job. + +## The release pipeline + +`.github/workflows/build.yml` runs on a `v*` tag push or a manual dispatch, and a run is never +cancelled (concurrency group `build-tablepro-`). Every job except the two builds needs a `v*` +ref, so a dispatch from a branch only builds. The tag runs no test suite. + +- `Tested on main` (Ubuntu) fails when the tagged commit is not an ancestor of `origin/main`. +- For `macOS Tests` and `iOS and Package Tests` separately, it walks up to 50 first-parent commits + back from the tag. On each it takes the newest push run of that workflow and waits for it if it is + still going. A commit with no run, or whose job was skipped, is passed over; any conclusion other + than success fails the release. +- A passing run counts only when `scripts/ci/detect-changed-paths.sh --base ` + prints `false`, so nothing the suite reads changed between that commit and the tag. The step + summary names the covering run for each suite. +- The two suites may be covered by different commits. Requiring one commit for both would fail + whenever only one suite ran later. +- `Build arm64` and `Build x86_64` run on `xcode-27` with no fail-fast; x86_64 cross-compiles on an + arm64 runner. Each runs `scripts/build-release.sh` with notarization, then makes the DMG and the + tar.xz update archive. +- `Registry Readiness` (Ubuntu) runs `scripts/check-registry-readiness.py` with `--floor` and + `--current` set to the two kit versions in `TablePro/Core/Plugins/PluginManager.swift`. It fails + when a registry driver has no binary in that range, so the app never ships ahead of its plugins. +- `Create GitHub Release` needs all three and runs under the `release` environment. It fails when + the tag disagrees with `MARKETING_VERSION` (`tag v... does not match MARKETING_VERSION ...`). +- It also fails when `CURRENT_PROJECT_VERSION` is not above the newest published release's. Sparkle + compares that number as ``, so a flat one ships a release no install is offered. +- It signs both update archives with the Sparkle key, builds deltas from the previous five + releases' archives, and takes the notes from `CHANGELOG.md`. +- It checks that the appcast deploy key works before publishing anything. Then it puts both DMGs, + both update archives and the deltas on a draft release, publishes the draft in one step, and only + then commits `appcast.xml`, whose download URLs 404 until the release is public. +- The appcast is merged with the tag's own copy of `scripts/ci/merge-appcast.py` and pushed with + `APPCAST_DEPLOY_KEY`, up to 3 attempts, each re-merging onto the current `origin/main`. +- `generate_appcast` sees only the new release. The merge script splices its items in and moves + published items byte for byte, because Sparkle rewrites an existing item's download URL and drops + its notes when a staged archive matches it. +- A tag containing `-alpha`, `-beta` or `-rc` publishes as a prerelease. + +## Keys and rulesets + +- `main` takes no direct push. Its ruleset requires the checks `AGENTS.md` lists under CI, and + deploy keys bypass it, so the appcast push and the release commit push need no check run. +- The appcast push uses `APPCAST_DEPLOY_KEY`, a write deploy key held by the `release` environment, + which only `v*` tags can use. +- The release commit and a withdrawal push with the release key, `~/.ssh/tablepro_release`, a second + write deploy key. The release skill's pre-flight step 7 creates it the first time a release needs + it. +- The release key has a passphrase and is never added to ssh-agent: agents run on the maintainer's + Mac and the key skips every check, so only the maintainer runs a push with it. +- The push command is in `.claude/skills/release/SKILL.md`, Commit, tag, push. +- The `v*` tag ruleset lets only admins create, move or delete a release tag. + +## When a release run fails + +- A published release is immutable: GitHub refuses to add, replace or delete an asset, or to move + the tag. +- Before **Publish the release**, only a draft exists. Fix forward on `main` through a pull request, + then move the tag to the commit it lands as. The next run reuses the draft. -A published release is immutable: GitHub refuses to add, replace or delete an asset, or to move the -tag. When a run fails: + ```bash + git fetch origin main + git tag -f -a v -m "v" origin/main + git push origin v --force + ``` -- Before **Publish the release**, only a draft exists. Fix forward on `main` through a pull request, - move the tag to the commit it lands as and push it again. The next run reuses the draft. -- After it, at the `appcast.xml` push for instance, use **Re-run failed jobs**. The release job finds - the same files already published and carries on. **Re-run all jobs** rebuilds, and the release job +- After it (a failed `appcast.xml` push, say), use **Re-run failed jobs**: the release job finds the + same files already published and carries on. **Re-run all jobs** rebuilds, and the release job refuses the new files. - A published build that is wrong ships as a new version. -`generate_appcast` sees only the release being published, never the existing feed, and -`scripts/ci/merge-appcast.py` splices its two items into `appcast.xml` afterwards. Items already -published are moved byte for byte rather than regenerated, because Sparkle rewrites the download URL -and drops the release notes of any existing item a staged archive happens to match. - -## Staged rollout and critical releases - -Every release carries `21600`. Sparkle -spreads a release over seven cohorts keyed on a per-install random identifier, so six intervals is -the tail: the last cohort is offered the build 36 hours after the first. A user who runs **Check for -Updates…** is never phased and always gets the newest build. - -`.github/release-flags.json` decides whether a release may interrupt anyone: - -```json -{ "criticalUpdate": false } -``` - -Setting it `true` passes `--critical-update-version ""` to `generate_appcast`, which writes -`` with no version attribute, making the item critical for every host. -Sparkle then bypasses phasing, hides Skip and Remind Me Later, retitles the alert, and shows the -update even under automatic downloads. +## Staged rollout and critical updates -The bar is data loss, an exposed security issue, a crash or hang on a common path, or a regression -with no workaround. Set it in the release commit and set it back in the next one. The release job -fails when the flag is `true` and the file has not changed since the previous tag, which is what a -leftover flag looks like. +- An ordinary release carries + `21600`. Sparkle splits installs + into seven cohorts by a per-install random id, so the last cohort is offered the build 36 hours + after the first. **Check for Updates…** is never phased. +- `"criticalUpdate": true` in `.github/release-flags.json` passes `--critical-update-version ""` to + `generate_appcast` instead. That writes `` with no version, critical for + every host: Sparkle skips phasing, hides Skip and Remind Me Later, retitles the alert, and shows + it even under automatic downloads. +- The release job fails when the flag is `true` and the file has not changed since the previous + tag, which is what a leftover flag looks like. +- When to set it: `.claude/skills/release/SKILL.md`, Critical update flag. ## Withdrawing a release -Removing the items from `appcast.xml` is the only way to un-ship a Sparkle release. +Removing the version's items from `appcast.xml` is the only way to un-ship a Sparkle release. ```bash -python3 scripts/ci/pull-release.py 0.75.0 --dry-run # reports what it would remove +python3 scripts/ci/pull-release.py --dry-run # reports what it would remove GIT_SSH_COMMAND='ssh -i ~/.ssh/tablepro_release -o IdentitiesOnly=yes' \ - python3 scripts/ci/pull-release.py 0.75.0 # edits, commits and pushes with the release key + python3 scripts/ci/pull-release.py # edits, commits and pushes to main ``` -The GitHub release stays up, so a direct download link still resolves. Anyone already on the bad -build is reached by shipping the fix, not by the withdrawal. +- The second command pushes with the release key, so the maintainer runs it (Keys and rulesets). +- It refuses to push from any branch but `main`, or when local `main` is not `origin/main`, since + the key would carry any other commit past every check. It also refuses to empty the feed. +- The GitHub release stays up, so a direct download link still resolves. Users already on the bad + build are reached by shipping the fix. ## Plugins -Driver plugins release on their own `plugin-*-v*` tags through a separate workflow, and the registry -decides which binary a user gets. [Plugin Registry](https://docs.tablepro.app/development/plugin-registry) covers the -manifest, the tag names, and the bulk re-release after a PluginKit ABI change. +- `.github/workflows/build-plugin.yml` runs on a `plugin-*-v*` tag, or on a dispatch with `tags` + (`tag[:pluginKitVersion],...`) and an optional `baseRef`. Runs are grouped per ref and per matrix + tag and never cancelled. +- `Resolve Plugin Tags` (Ubuntu) picks the runner: `macos-26` when the ref being built pins Xcode 26 + in its own `.github/actions/setup-xcode`, else `xcode-27`. +- It runs `.github/scripts/test_update_registry.py` and `scripts/ci/check-plugin-manifest.py`, and + validates each tag against `^plugin-[a-z0-9-]+-v[0-9]+\.[0-9]+\.[0-9]+$` and each kit version as + an integer. A tag given without a kit version takes `currentPluginKitVersion`. +- The build job checks out `baseRef` when one is given. It builds the Dameng bridge for + `plugin-dameng-*`, a universal HANA helper plus `govulncheck` for `plugin-hana-*`, then runs + `scripts/build-plugin.sh "$TARGET" both "$VERSION"` with notarization. +- Before publishing: `scripts/ci/verify-plugin-loads.sh` dlopens the arm64 bundle beside its + PluginKit framework; both zips must be stapled and accepted by Gatekeeper; the built + `TableProPluginKitVersion` must equal the release label. +- It publishes the release from a draft, then records the binary in the TableProApp/plugins registry + with `.github/scripts/update-registry.py`. The registry decides which binary a user gets: + [Plugin Registry](https://docs.tablepro.app/development/plugin-registry) covers the manifest, the + tag names and the bulk re-release after a PluginKit ABI change. diff --git a/TablePro/Core/MCP/Lifecycle/MCPServerComposition.swift b/TablePro/Core/MCP/Lifecycle/MCPServerComposition.swift index 7a72fa4d2d..5e8a37057a 100644 --- a/TablePro/Core/MCP/Lifecycle/MCPServerComposition.swift +++ b/TablePro/Core/MCP/Lifecycle/MCPServerComposition.swift @@ -23,7 +23,8 @@ internal struct MCPServerComposition { let authenticator = MCPBearerTokenAuthenticator(tokenStore: tokenStore, rateLimiter: rateLimiter) let transport = MCPHttpServerTransport( configuration: .loopback(port: port), - authenticator: authenticator + authenticator: authenticator, + pairingService: .shared ) let subscriptions = MCPSubscriptionRegistry() let services = MCPToolServices( diff --git a/TablePro/Core/MCP/Transport/Extensions/MCPHttpRequestRouter+Pairing.swift b/TablePro/Core/MCP/Transport/Extensions/MCPHttpRequestRouter+Pairing.swift index 4bc61d113b..e4076ecea3 100644 --- a/TablePro/Core/MCP/Transport/Extensions/MCPHttpRequestRouter+Pairing.swift +++ b/TablePro/Core/MCP/Transport/Extensions/MCPHttpRequestRouter+Pairing.swift @@ -50,7 +50,7 @@ internal extension MCPHttpRequestRouter { } do { - let record = try await MCPPairingService.shared.exchange( + let record = try await pairingService.exchange( PairingExchange(code: parsed.code, verifier: parsed.codeVerifier), clientAddress: clientAddress ) diff --git a/TablePro/Core/MCP/Transport/MCPHttpRequestRouter.swift b/TablePro/Core/MCP/Transport/MCPHttpRequestRouter.swift index 4ab5222847..b1a2cfea11 100644 --- a/TablePro/Core/MCP/Transport/MCPHttpRequestRouter.swift +++ b/TablePro/Core/MCP/Transport/MCPHttpRequestRouter.swift @@ -11,6 +11,7 @@ struct MCPHttpRequestRouter: Sendable { static let pairingPath = "/v1/integrations/exchange" let authenticator: any MCPAuthenticator + let pairingService: MCPPairingService let clock: any MCPClock let boundPort: UInt16 let emitInbound: InboundEmitter diff --git a/TablePro/Core/MCP/Transport/MCPHttpServerTransport.swift b/TablePro/Core/MCP/Transport/MCPHttpServerTransport.swift index 2e46adfecf..3501cb2ab1 100644 --- a/TablePro/Core/MCP/Transport/MCPHttpServerTransport.swift +++ b/TablePro/Core/MCP/Transport/MCPHttpServerTransport.swift @@ -18,6 +18,7 @@ public actor MCPHttpServerTransport { private let configuration: MCPHttpServerConfiguration private let authenticator: any MCPAuthenticator + private let pairingService: MCPPairingService private let clock: any MCPClock private var listener: NWListener? @@ -33,13 +34,15 @@ public actor MCPHttpServerTransport { nonisolated public let listenerState: AsyncStream nonisolated private let stateContinuation: AsyncStream.Continuation - public init( + init( configuration: MCPHttpServerConfiguration, authenticator: any MCPAuthenticator, + pairingService: MCPPairingService, clock: any MCPClock = MCPSystemClock() ) { self.configuration = configuration self.authenticator = authenticator + self.pairingService = pairingService self.clock = clock let (exchanges, exchangesContinuation) = AsyncStream.makeStream( @@ -232,6 +235,7 @@ public actor MCPHttpServerTransport { let continuation = exchangesContinuation return MCPHttpRequestRouter( authenticator: authenticator, + pairingService: pairingService, clock: clock, boundPort: boundPort == 0 ? configuration.port : boundPort, emitInbound: { exchange in diff --git a/TablePro/Core/Services/Infrastructure/TabWindowController.swift b/TablePro/Core/Services/Infrastructure/TabWindowController.swift index 42fe631b26..8dd3a14d7f 100644 --- a/TablePro/Core/Services/Infrastructure/TabWindowController.swift +++ b/TablePro/Core/Services/Infrastructure/TabWindowController.swift @@ -79,9 +79,6 @@ extension EditorWindow: CloseCommandNaming { internal final class TabWindowController: NSWindowController, NSWindowDelegate { nonisolated private static let lifecycleLogger = Logger(subsystem: "com.TablePro", category: "NativeTabLifecycle") - /// Namespaced per sandbox under UI test. This one is the main window's own size and position, - /// so a case that resized the window used to hand that size to every case after it in its - /// shard. See `SplitViewAutosaveName`. @MainActor internal static var frameAutosaveName: NSWindow.FrameAutosaveName? { SplitViewAutosaveName.current("MainEditorWindow") diff --git a/TablePro/Extensions/NSWindow+FrameAutosave.swift b/TablePro/Extensions/NSWindow+FrameAutosave.swift index 857a8d6c6c..84d012bf5a 100644 --- a/TablePro/Extensions/NSWindow+FrameAutosave.swift +++ b/TablePro/Extensions/NSWindow+FrameAutosave.swift @@ -6,16 +6,8 @@ import AppKit extension NSWindow { - /// Do not call on a window owned by an `NSWindowController` whose - /// `contentViewController` is an `NSSplitViewController`. The contentVC's - /// intrinsic-size resize during init fires the implicit auto-save observer - /// installed by `setFrameAutosaveName`, overwriting the persisted frame - /// with the small intrinsic size. Use `setFrameUsingName` plus explicit - /// `saveFrame(usingName:)` calls in `NSWindowDelegate` methods instead. - /// See `TabWindowController` for that pattern. - /// Namespaced per sandbox under UI test: a saved frame lands in the standard defaults domain, - /// which the sandbox does not redirect, so without this one case inherits another's window - /// position and size. See `SplitViewAutosaveName`. + /// Not for a window whose content is an `NSSplitViewController`: its resize during init fires the + /// autosave and overwrites the saved frame. `TabWindowController` restores and saves by hand instead. @MainActor func applyAutosaveName(_ name: NSWindow.FrameAutosaveName) { guard let scoped = SplitViewAutosaveName.current(name) else { diff --git a/TableProMobile/TableProMobileTests/README.md b/TableProMobile/TableProMobileTests/README.md deleted file mode 100644 index a0fae318a3..0000000000 --- a/TableProMobile/TableProMobileTests/README.md +++ /dev/null @@ -1,26 +0,0 @@ -# TableProMobileTests - -Swift Testing tests for the iOS view models extracted in P1 #5 (PRs #1164, #1165, #1166). - -## One-time Xcode setup - -The test target is not in the Xcode project yet. To enable these tests: - -1. Open `TableProMobile.xcodeproj` in Xcode -2. File → New → Target -3. iOS → Unit Testing Bundle -4. Product Name: `TableProMobileTests` -5. Target to Test: `TableProMobile` -6. Testing System: **Swift Testing** -7. Finish - -Xcode will create a stub `TableProMobileTests.swift` that you can delete. Because the project uses synchronized file groups (Xcode 16+), the existing files in this folder will be picked up automatically once the target points at this directory. - -If Xcode chose a different folder name when creating the target, drag-and-drop these files into the target in the navigator, or rename the test root group to match. - -## Layout - -- `Mocks/MockDatabaseDriver.swift` - in-memory `DatabaseDriver` and `SecureStore` stubs with scriptable results -- `DataBrowserViewModelTests.swift` - load lifecycle, pagination, sort/filter/search, delete, primary key extraction -- `ConnectionFormViewModelTests.swift` - hydration from existing connection, default port on type change, validation, credential hydration, file picker helpers -- `RowDetailViewModelTests.swift` - edit lifecycle, save with/without changes, primary key requirement, lazy cell load diff --git a/TableProTests/Core/MCP/Helpers/MCPTransportTestStubs.swift b/TableProTests/Core/MCP/Helpers/MCPTransportTestStubs.swift index 173eb1b3c0..fba777f59b 100644 --- a/TableProTests/Core/MCP/Helpers/MCPTransportTestStubs.swift +++ b/TableProTests/Core/MCP/Helpers/MCPTransportTestStubs.swift @@ -418,13 +418,22 @@ enum MCPTransportTestHarness { static func start( port requestedPort: UInt16 = 0, authenticator: any MCPAuthenticator = StubAlwaysAllowAuthenticator(), + pairingService: MCPPairingService? = nil, limits: MCPHttpServerLimits = .standard, clock: any MCPClock = MCPSystemClock(), timeout: Duration = .seconds(5) ) async throws -> (MCPHttpServerTransport, UInt16) { + // Never .shared: its limiter locks 127.0.0.1 out after 5 failed exchanges, across cases. + let pairing: MCPPairingService + if let pairingService { + pairing = pairingService + } else { + pairing = await MCPPairingService() + } let transport = MCPHttpServerTransport( configuration: MCPHttpServerConfiguration.loopback(port: requestedPort, limits: limits), authenticator: authenticator, + pairingService: pairing, clock: clock ) try await transport.start() @@ -457,11 +466,17 @@ enum MCPTransportTestHarness { static func withServer( port requestedPort: UInt16 = 0, authenticator: any MCPAuthenticator = StubAlwaysAllowAuthenticator(), + pairingService: MCPPairingService? = nil, limits: MCPHttpServerLimits = .standard, handler: @escaping @Sendable (MCPInboundExchange) async -> Void = MCPTransportTestHandlers.echo, body: (UInt16) async throws -> Void ) async throws { - let (transport, port) = try await start(port: requestedPort, authenticator: authenticator, limits: limits) + let (transport, port) = try await start( + port: requestedPort, + authenticator: authenticator, + pairingService: pairingService, + limits: limits + ) let consumer = StubExchangeConsumer() await consumer.start(transport: transport, responder: handler) do { diff --git a/TableProTests/Core/MCP/Transport/MCPHttpServerTransportPairingTests.swift b/TableProTests/Core/MCP/Transport/MCPHttpServerTransportPairingTests.swift index aa2af1feb0..5f0b627402 100644 --- a/TableProTests/Core/MCP/Transport/MCPHttpServerTransportPairingTests.swift +++ b/TableProTests/Core/MCP/Transport/MCPHttpServerTransportPairingTests.swift @@ -4,7 +4,6 @@ import TableProPluginKit @testable import TablePro import Testing -@Suite("MCP HTTP Server Transport Pairing", .serialized) struct MCPHttpServerTransportPairingTests { private static let exchangePath = "/v1/integrations/exchange" @@ -17,18 +16,23 @@ struct MCPHttpServerTransportPairingTests { "test-code-\(UUID().uuidString)" } - private func store() async -> PairingExchangeStore { - await MainActor.run { MCPPairingService.shared.store } + private func withServer( + store: PairingExchangeStore, + body: (UInt16) async throws -> Void + ) async throws { + let service = await MCPPairingService(store: store) + try await MCPTransportTestHarness.withServer(pairingService: service, body: body) } private func insertPairingCode( + into store: PairingExchangeStore, code: String, plaintextToken: String, verifier: String, expiresIn: TimeInterval, grantedScopes: Set = MCPScope.readOnlySet ) async throws { - try await store().insert( + try await store.insert( code: code, record: PairingExchangeRecord( plaintextToken: plaintextToken, @@ -40,10 +44,6 @@ struct MCPHttpServerTransportPairingTests { ) } - private func discard(code: String) async { - await store().discard(code: code) - } - private func exchangeRequest(port: UInt16, body: Data?) -> Data { MCPTransportTestRequests.raw( method: "POST", @@ -111,11 +111,13 @@ struct MCPHttpServerTransportPairingTests { @Test("A valid code and verifier exchange for the paired token") func successfulExchangeReturnsTheToken() async throws { - try await MCPTransportTestHarness.withServer { port in + let store = PairingExchangeStore() + try await withServer(store: store) { port in let code = uniqueCode() let verifier = uniqueVerifier() let plaintext = "tp_test-token-\(UUID().uuidString)" try await insertPairingCode( + into: store, code: code, plaintextToken: plaintext, verifier: verifier, @@ -130,17 +132,19 @@ struct MCPHttpServerTransportPairingTests { #expect(try response.plainJsonField("token") == plaintext) #expect(try response.plainJsonField("scope") == "resources:read tools:read") - let stillPending = await store().contains(code: code) + let stillPending = await store.contains(code: code) #expect(!stillPending, "a pairing code is single-use") } } @Test("The exchange names every granted scope, the display scope included") func exchangeReportsGrantedScopes() async throws { - try await MCPTransportTestHarness.withServer { port in + let store = PairingExchangeStore() + try await withServer(store: store) { port in let code = uniqueCode() let verifier = uniqueVerifier() try await insertPairingCode( + into: store, code: code, plaintextToken: "tp_test-token-\(UUID().uuidString)", verifier: verifier, @@ -171,15 +175,16 @@ struct MCPHttpServerTransportPairingTests { @Test("A verifier that does not match the challenge answers 403 and burns the code") func mismatchedVerifierIsForbidden() async throws { - try await MCPTransportTestHarness.withServer { port in + let store = PairingExchangeStore() + try await withServer(store: store) { port in let code = uniqueCode() try await insertPairingCode( + into: store, code: code, plaintextToken: "tp_test", verifier: uniqueVerifier(), expiresIn: 60 ) - defer { Task { await discard(code: code) } } let payload = ["code": code, "code_verifier": uniqueVerifier()] let body = try JSONSerialization.data(withJSONObject: payload, options: [.sortedKeys]) @@ -188,23 +193,24 @@ struct MCPHttpServerTransportPairingTests { #expect(response.statusCode == 403) #expect(try response.plainJsonField("error") == "Challenge mismatch") - let stillPending = await store().contains(code: code) + let stillPending = await store.contains(code: code) #expect(!stillPending, "a failed verification burns the code") } } @Test("An expired code is unredeemable") func expiredCodeIsUnredeemable() async throws { - try await MCPTransportTestHarness.withServer { port in + let store = PairingExchangeStore() + try await withServer(store: store) { port in let code = uniqueCode() let verifier = uniqueVerifier() try await insertPairingCode( + into: store, code: code, plaintextToken: "tp_test", verifier: verifier, expiresIn: -60 ) - defer { Task { await discard(code: code) } } let payload = ["code": code, "code_verifier": verifier] let body = try JSONSerialization.data(withJSONObject: payload, options: [.sortedKeys]) @@ -216,6 +222,25 @@ struct MCPHttpServerTransportPairingTests { } } + @Test("Failed exchanges past the limit lock the address out with 429 and Retry-After") + func repeatedFailuresAreRateLimited() async throws { + let allowedFailures = await MCPPairingService.exchangeRateLimitPolicy.maxFailedAttempts + try await MCPTransportTestHarness.withServer { port in + for _ in 0.. PluginCellValue { /// The point `hexIsDrawn` reads as text, here as the bytes a driver hands over. private let ewkbPoint = "0101000020E610000050FC1873D79A5EC0D0D556EC2FE34240" -/// Names the fixture's column directly rather than going through `SpatialColumn.columns(in:)`. -/// -/// The projector's job is to project whatever column it is handed, and which columns are offered is -/// a separate decision with its own tests above. Those two have to stay separate here, because the -/// gate refuses a column no reader can read and that is exactly the fixture several of these need. +/// Skips `SpatialColumn.columns(in:)`, whose gate refuses a column no reader can read, the fixture several cases need. private func onlyColumn(_ tableRows: TableRows) -> SpatialColumn { SpatialColumn( id: SpatialColumnID(name: tableRows.columns[0], occurrence: 1), @@ -221,14 +217,14 @@ struct SpatialColumnTests { /// label distinguishes them. Naming rather than indexing is what lets a choice survive a /// re-execution that reorders the SELECT list. @Test("Duplicate column names stay distinguishable") - func duplicateNames() { + func duplicateNames() throws { let duplicated = TableRows( rows: [], columns: ["geom", "geom"], columnTypes: [.spatial(rawType: "geometry"), .spatial(rawType: "geometry")] ) let columns = SpatialColumn.columns(in: duplicated) - #expect(columns.count == 2) + try #require(columns.count == 2) #expect(columns[0].id != columns[1].id) #expect(columns[0].displayName == "geom (1)") #expect(columns[1].displayName == "geom (2)") @@ -237,7 +233,7 @@ struct SpatialColumnTests { struct SpatialResultProjectorTests { @Test("EWKT points project to shapes tagged with their row") - func projectsPoints() async { + func projectsPoints() async throws { let table = rows([ "SRID=4326;POINT(-122.4194 37.7749)", "SRID=4326;POINT(-0.1276 51.5072)", @@ -247,16 +243,16 @@ struct SpatialResultProjectorTests { displayIDs: nil, column: onlyColumn(table) ) - #expect(projection.shapes.count == 2) + try #require(projection.shapes.count == 2) #expect(projection.diagnostics.drawnRows == 2) #expect(projection.diagnostics.drawnSRID == wgs84) #expect(projection.shapes[0].rowID == .existing(0)) #expect(projection.shapes[0].kind == .point) - #expect(projection.shapes[0].rings[0][0].longitude == -122.4194) + #expect(projection.shapes[0].rings.first?.first?.longitude == -122.4194) } /// A map cannot show two coordinate systems at once, so the majority is drawn and the rest are - /// counted. Drawing fewer shapes with no explanation is the failure every competitor ships. + /// counted. @Test("A minority SRID is reported rather than silently dropped") func minoritySRIDIsCounted() async { let table = rows([ @@ -276,7 +272,7 @@ struct SpatialResultProjectorTests { } @Test("Web Mercator is inverted rather than refused") - func webMercatorIsDrawn() async { + func webMercatorIsDrawn() async throws { let table = rows(["SRID=3857;POINT(-13627665.271218073 4547675.354340557)"]) let projection = await SpatialResultProjector.shared.project( tableRows: table, @@ -284,7 +280,7 @@ struct SpatialResultProjectorTests { column: onlyColumn(table) ) #expect(projection.diagnostics.projectability == .webMercator) - let coordinate = projection.shapes[0].rings[0][0] + let coordinate = try #require(projection.shapes.first?.rings.first?.first) #expect(abs(coordinate.longitude - -122.4194) < 1e-9) #expect(abs(coordinate.latitude - 37.7749) < 1e-9) } @@ -341,14 +337,14 @@ struct SpatialResultProjectorTests { } @Test("A polygon keeps its holes") - func polygonKeepsHoles() async { + func polygonKeepsHoles() async throws { let table = rows(["POLYGON((0 0,4 0,4 4,0 4,0 0),(1 1,2 1,2 2,1 2,1 1))"]) let projection = await SpatialResultProjector.shared.project( tableRows: table, displayIDs: nil, column: onlyColumn(table) ) - #expect(projection.shapes.count == 1) + try #require(projection.shapes.count == 1) #expect(projection.shapes[0].rings.count == 2) } @@ -393,14 +389,14 @@ struct SpatialResultProjectorTests { /// The map draws the display order, so a value filter narrows it without any extra wiring, and /// the shapes come back in the order the grid is showing. @Test("Only the display order is drawn") - func honoursDisplayOrder() async { + func honoursDisplayOrder() async throws { let table = rows(["POINT(1 1)", "POINT(2 2)", "POINT(3 3)"]) let projection = await SpatialResultProjector.shared.project( tableRows: table, displayIDs: [.existing(2), .existing(0)], column: onlyColumn(table) ) - #expect(projection.shapes.count == 2) + try #require(projection.shapes.count == 2) #expect(projection.shapes[0].rowID == .existing(2)) #expect(projection.shapes[1].rowID == .existing(0)) } diff --git a/TableProTests/Core/Sync/SyncLocalOnlyDependentsTests.swift b/TableProTests/Core/Sync/SyncLocalOnlyDependentsTests.swift index 96b90ecd0b..26daf2b624 100644 --- a/TableProTests/Core/Sync/SyncLocalOnlyDependentsTests.swift +++ b/TableProTests/Core/Sync/SyncLocalOnlyDependentsTests.swift @@ -2,6 +2,7 @@ import CloudKit import Foundation @testable import TablePro import TableProSyncTransport +import TableProTestSupport import Testing @MainActor @@ -99,6 +100,12 @@ struct SyncLocalOnlyDependentsTests { await environment.makeCoordinator(transport: transport).runSyncCycle() } + /// `ConnectionLocalState.purge` releases a Local only owner from a Task it does not return. + /// Waiting for the release keeps that defaults write inside the case, before the suite is removed. + private func waitForKeptOffOwnersReleased() async throws { + try await waitUntil { tracker.ownersKeptOffSync.isEmpty } + } + private static func isolatedQueryHistory() -> QueryHistoryManager { QueryHistoryManager( storage: QueryHistoryStorage( @@ -359,10 +366,7 @@ struct SyncLocalOnlyDependentsTests { queryHistory: history, syncTracker: tracker ) - await ConnectionLocalState.purgeAsyncStores( - [connection.id], origin: .localOnly, sqlFavorites: environment.favorites, queryHistory: history, - syncTracker: tracker - ) + try await waitForKeptOffOwnersReleased() let transport = ScriptedSyncTransport(zoneID: Self.zoneID) let failure = await runCycle(transport) @@ -379,7 +383,7 @@ struct SyncLocalOnlyDependentsTests { } @Test("A deleted Local only connection stays out of iCloud until its saved queries are gone") - func deletedLocalOnlyOwnerStaysExcludedUntilItsSavedQueriesAreGone() async { + func deletedLocalOnlyOwnerStaysExcludedUntilItsSavedQueriesAreGone() async throws { let connection = addConnection(.localOnly) let savedQuery = SQLFavorite(name: "Revenue", query: "SELECT 1", connectionId: connection.id) #expect(await environment.favorites.addFavorite(savedQuery)) @@ -401,17 +405,13 @@ struct SyncLocalOnlyDependentsTests { #expect(tracker.ownersKeptOffSync == [connection.id]) #expect(!coordinator.syncBoundary(settings: .default).includes(.favorite, owner: connection.id)) - await ConnectionLocalState.purgeAsyncStores( - [connection.id], origin: .localOnly, sqlFavorites: environment.favorites, queryHistory: history, - syncTracker: tracker - ) + try await waitForKeptOffOwnersReleased() #expect(await environment.favorites.fetchFavorite(id: savedQuery.id) == nil) - #expect(tracker.ownersKeptOffSync.isEmpty) } @Test("Deleting a Local only connection forgets the deletions held for it, and a synced one keeps them") - func connectionDeleteDiscardsOnlyALocalOnlyOwnersTombstones() { + func connectionDeleteDiscardsOnlyALocalOnlyOwnersTombstones() async throws { let localOnly = addConnection(.localOnly) let synced = addConnection() metadata.addTombstones(["held"], type: .tableFavorite, owner: localOnly.id) @@ -429,6 +429,7 @@ struct SyncLocalOnlyDependentsTests { #expect(environment.connections.deleteConnections([synced])) #expect(tracker.tombstonedIds(for: .tableFavorite) == ["pending"]) #expect(tracker.tombstonedIds(for: .connection) == [synced.id.uuidString]) + try await waitForKeptOffOwnersReleased() } // MARK: - Pruning diff --git a/TableProTests/Helpers/OfflineRegistry.swift b/TableProTests/Helpers/OfflineRegistry.swift index 0b2c6a564e..2807d8fbce 100644 --- a/TableProTests/Helpers/OfflineRegistry.swift +++ b/TableProTests/Helpers/OfflineRegistry.swift @@ -61,6 +61,12 @@ final class OfflineRegistry { OfflineRegistryProtocol.requestCount(for: manifestURL) } + /// A Plugins pane's fetch writes defaults and the cache file when it lands, and a write after + /// deinit recreates both directories. A fresh manifest also makes a fetch still queued a no-op. + func settle() async { + await client.ensureManifest(.ifStale) + } + deinit { try? FileManager.default.removeItem(at: directory) } diff --git a/TableProTests/ViewModels/WelcomeViewModelTests.swift b/TableProTests/ViewModels/WelcomeViewModelTests.swift index 92e06adc31..369a4f1470 100644 --- a/TableProTests/ViewModels/WelcomeViewModelTests.swift +++ b/TableProTests/ViewModels/WelcomeViewModelTests.swift @@ -36,15 +36,17 @@ final class WelcomeViewModelTests { at: connectionFileURL.deletingLastPathComponent(), withIntermediateDirectories: true ) - connectionStorage = ConnectionStorage( + let connectionStorage = ConnectionStorage( fileURL: connectionFileURL, userDefaults: defaults, syncTracker: tracker ) + self.connectionStorage = connectionStorage + // The parameter is an @escaping @autoclosure: `self.connectionStorage` would capture self. groupStorage = GroupStorage( userDefaults: defaults, syncTracker: tracker, - connectionStorage: self.connectionStorage + connectionStorage: connectionStorage ) welcomeRouter = WelcomeRouter() recents = RecentConnectionsStore(defaults: defaults, appEvents: AppEvents()) diff --git a/TableProTests/Views/Settings/SettingsWindowTitleTests.swift b/TableProTests/Views/Settings/SettingsWindowTitleTests.swift index ef9bd473ff..bbeacea0e4 100644 --- a/TableProTests/Views/Settings/SettingsWindowTitleTests.swift +++ b/TableProTests/Views/Settings/SettingsWindowTitleTests.swift @@ -23,19 +23,20 @@ final class SettingsWindowTitleTests: XCTestCase { storedPane = AppStorageEnvironment.shared.defaults.string(forKey: PreferenceKeys.selectedSettingsPane.name) } - override func tearDown() { + override func tearDown() async throws { for window in windows { window.contentViewController = nil window.close() } windows = [] + await registry?.settle() registry = nil if let storedPane { AppStorageEnvironment.shared.defaults.set(storedPane, forKey: PreferenceKeys.selectedSettingsPane.name) } else { AppStorageEnvironment.shared.defaults.removeObject(forKey: PreferenceKeys.selectedSettingsPane.name) } - super.tearDown() + try await super.tearDown() } // The Plugins pane fetches the registry manifest when it appears. diff --git a/scripts/check-doc-symbols.sh b/scripts/check-doc-symbols.sh index 7144c0ef44..137e8bcd7b 100755 --- a/scripts/check-doc-symbols.sh +++ b/scripts/check-doc-symbols.sh @@ -5,7 +5,8 @@ # Prose does not fail a build when the code moves underneath it, so a renamed type or a deleted # script stays named in the docs until something asks. This asks. # -# Scope: CLAUDE.md, AGENTS.md, and every .md under .claude/rules and .claude/skills. +# Scope: CLAUDE.md, AGENTS.md, CONTRIBUTING.md, RELEASING.md, .github/pull_request_template.md, +# and every .md under .claude/rules and .claude/skills. # # What is checked. Only mechanical claims, because those are the ones that rot silently: # paths a backticked repo-relative path must exist @@ -14,6 +15,9 @@ # scripts every .sh named must exist, and be executable unless it is a sourced lib/ file # skills every Skill(name) and hyphenated $name reference must resolve # counts a stated plugin-bundle count must match the tree +# rows each AGENTS.md Area rules row must list exactly its rule's paths: globs, a trailing +# /**/* or /** dropped, because Codex finds a rule only through that row +# globs each paths: glob must match a file, or the rule never loads # # Fenced code blocks are stripped first: a claim in prose is a claim, a name inside an example is # an example. A placeholder (`SomeType`, `TypeName`) and an absolute or home path are not claims @@ -28,12 +32,13 @@ set -uo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" cd "$REPO_ROOT" || exit 3 +REPO_REAL="$(pwd -P)" LIST=0 [ "${1:-}" = "--list" ] && LIST=1 DOCS=() -for top in CLAUDE.md AGENTS.md; do +for top in CLAUDE.md AGENTS.md CONTRIBUTING.md RELEASING.md .github/pull_request_template.md; do [ -f "$top" ] && DOCS+=("$top") done while IFS= read -r f; do DOCS+=("$f"); done < <( @@ -118,7 +123,7 @@ build_symbol_index() { find TablePro Plugins Packages TableProTests TableProUITests TableProMobile \ -name '*.swift' -type f 2> /dev/null | uncommented_identifiers grep -rhoE '\b[A-Za-z][A-Za-z0-9_]{3,}\b' --include='*.h' Plugins 2> /dev/null - grep -hoE '^ [A-Za-z][A-Za-z0-9_+-]*:' project.yml 2> /dev/null | tr -d ' :' + grep -hoE '^ [A-Za-z][A-Za-z0-9_+-]*:' project.yml TableProMobile/project.yml 2> /dev/null | tr -d ' :' printf '%s\n' $HARNESS_TOOLS $ENV_NAMES if command -v xcrun > /dev/null 2>&1; then xcrun swift-frontend -print-supported-features 2> /dev/null | @@ -136,6 +141,9 @@ build_symbol_index() { # point only when the path ends in a slash. find "$frameworks_dir/$fw.framework/Headers/" -name '*.h' -type f 2> /dev/null >> "$WORK/sdk-headers" done + # XCTest ships in the platform, not the SDK. + find "${frameworks_dir%/SDKs/*}/Library/Frameworks/XCTest.framework/Headers/" \ + -name '*.h' -type f 2> /dev/null >> "$WORK/sdk-headers" uncommented_identifiers < "$WORK/sdk-headers" >> "$WORK/symbols.raw" fi LC_ALL=C sort -u "$WORK/symbols.raw" > "$WORK/symbols" @@ -203,7 +211,7 @@ check_paths() { while IFS= read -r hit; do line="${hit%%:*}"; token="${hit#*:}" case "$token" in - http* | /* | *' '* | *'*'* | *'<'* | *'$'* | *'|'* | *'{'*) continue ;; + http* | /* | '~'* | *' '* | *'*'* | *'<'* | *'$'* | *'|'* | *'{'*) continue ;; esac case "$token" in "$DOCS_REPO" | "$DOCS_REPO"/*) @@ -216,8 +224,16 @@ check_paths() { fi continue ;; - # A first segment carrying a dot is a hostname, not a path in this tree. - *.*/*) [ "${token%%/*}" != "${token%%.*}" ] && continue ;; + # Another repository cloned beside this one, present on some machines only. + .. | ../*) + skip "$token (sibling checkout)" "$doc:$line" + continue + ;; + esac + # A dotted first segment is a hostname (github.com/...), unless the dot leads (.claude/...). + case "${token%%/*}" in + .*) ;; + *.*) continue ;; esac # Prose reads like a path when it is a pair of lowercase words: if/else, and/or, read/write. case "$token" in @@ -256,6 +272,9 @@ check_symbols() { esac if grep -qxF "$token" "$WORK/resolved"; then pass "$token" "$doc:$line" + elif [ -e "$token" ]; then + # A top-level folder, as an Area rules row writes `Libs` for Libs/**/*. + pass "$token (top-level path)" "$doc:$line" elif printf '%s\n' "$token" | grep -qE "$PLACEHOLDER_PATTERN"; then skip "$token (placeholder)" "$doc:$line" elif is_site_repo_doc "$doc"; then @@ -324,6 +343,113 @@ check_counts() { done } +# "" for each entry of a rule's paths: list, quotes stripped. +rule_globs() { + awk 'NR == 1 { if ($0 !~ /^---[[:space:]]*$/) exit; next } + /^---[[:space:]]*$/ { exit } + /^paths:/ { inside = 1; next } + /^[^[:space:]-]/ { inside = 0 } + inside && /^[[:space:]]*-[[:space:]]/ { + glob = $0 + sub(/^[[:space:]]*-[[:space:]]*/, "", glob) + sub(/[[:space:]]+$/, "", glob) + gsub(/^["\047]|["\047]$/, "", glob) + print NR "\t" glob + }' "$1" +} + +# "" for every backticked token in the other cells of each row +# of AGENTS.md's Area rules table, plus one "" line per row. +agents_rule_rows() { + awk '/^## / { inside = ($0 ~ /^## Area rules/); next } + inside && /^\|/ { + body = substr($0, 2) + cut = index(body, "|") + if (!cut) next + first = substr(body, 1, cut - 1) + rest = substr(body, cut + 1) + name = ""; names = 0 + while (match(first, /`[^`]+`/)) { + name = substr(first, RSTART + 1, RLENGTH - 2); names++ + first = substr(first, RSTART + RLENGTH) + } + if (!names) next + if (names > 1) name = "*" names + print NR "\t" name "\t" + while (match(rest, /`[^`]+`/)) { + print NR "\t" name "\t" substr(rest, RSTART + 1, RLENGTH - 2) + rest = substr(rest, RSTART + RLENGTH) + } + }' AGENTS.md +} + +check_rule_rows() { + local rule name rows row_line extra missing + [ -f AGENTS.md ] || return 0 + agents_rule_rows > "$WORK/rows" + while IFS="$TAB" read -r row_line rows; do + report "AGENTS.md:$row_line" "an Area rules row names $rows rules; give each rule its own row" + done < <(awk -F'\t' '$3 == "" && $2 ~ /^\*/ { print $1 "\t" substr($2, 2) }' "$WORK/rows") + for rule in .claude/rules/*.md; do + [ -f "$rule" ] || continue + name="${rule##*/}" + rows="$(awk -F'\t' -v r="$name" '$2 == r && $3 == "" { print $1 }' "$WORK/rows")" + if [ -z "$rows" ]; then + report "AGENTS.md" "no Area rules row for $rule, so Codex never reads it" + continue + fi + row_line="$(printf '%s\n' "$rows" | head -1)" + if [ "$rows" != "$row_line" ]; then + report "AGENTS.md:$row_line" "more than one Area rules row for $rule" + continue + fi + rule_globs "$rule" | cut -f2 | sed -E 's#/\*\*(/\*)?$##' | LC_ALL=C sort -u > "$WORK/want" + awk -F'\t' -v r="$name" '$2 == r && $3 != "" { print $3 }' "$WORK/rows" | LC_ALL=C sort -u > "$WORK/have" + missing="$(LC_ALL=C comm -23 "$WORK/want" "$WORK/have" | tr '\n' ' ')" + extra="$(LC_ALL=C comm -13 "$WORK/want" "$WORK/have" | tr '\n' ' ')" + if [ -z "$missing$extra" ]; then + pass "row $name equals its paths:" "AGENTS.md:$row_line" + continue + fi + [ -n "$missing" ] && report "AGENTS.md:$row_line" "the $name row lacks paths: globs ${missing% }" + [ -n "$extra" ] && report "AGENTS.md:$row_line" "the $name row lists what its paths: does not: ${extra% }" + done + while IFS="$TAB" read -r row_line name; do + [ -f ".claude/rules/$name" ] && continue + report "AGENTS.md:$row_line" "an Area rules row names $name, which is not a file in .claude/rules" + done < <(awk -F'\t' '$3 == "" && $2 !~ /^\*/ { print $1 "\t" $2 }' "$WORK/rows") +} + +# An untracked file counts, so a rule can name a file the same uncommitted change adds. +glob_matches_a_file() { + local glob="$1" dir target + [ -n "$(git ls-files -- ":(glob)$glob" 2> /dev/null | head -1)" ] && return 0 + [ -n "$(git ls-files --others --exclude-standard -- ":(glob)$glob" 2> /dev/null | head -1)" ] && return 0 + # git lists a symlinked directory as one entry, so match what is under its target instead. + dir="${glob%%[*?[]*}" + dir="${dir%/*}" + [ -L "$dir" ] && [ -d "$dir" ] || return 1 + target="$(cd "$dir" && pwd -P)" || return 1 + case "$target" in + "$REPO_REAL"/*) glob_matches_a_file "${target#"$REPO_REAL"/}${glob#"$dir"}" ;; + *) return 1 ;; + esac +} + +check_rule_globs() { + local rule line glob + for rule in .claude/rules/*.md; do + [ -f "$rule" ] || continue + while IFS="$TAB" read -r line glob; do + if glob_matches_a_file "$glob"; then + pass "$glob (paths: glob)" "$rule:$line" + else + report "$rule:$line" "paths: glob matches no file, so the rule never loads: $glob" + fi + done < <(rule_globs "$rule") + done +} + # ------------------------------------------------------------------ run build_symbol_index @@ -331,6 +457,8 @@ echo "checking ${#DOCS[@]} documents against the tree" [ "$sdk_indexed" -eq 1 ] || echo "no macOS SDK found: SDK names the tree does not use are skipped" for doc in "${DOCS[@]}"; do check_doc "$doc"; done check_counts +check_rule_rows +check_rule_globs echo if [ "$findings" -eq 0 ]; then diff --git a/scripts/clean-test-residue.sh b/scripts/clean-test-residue.sh new file mode 100755 index 0000000000..4df4197eab --- /dev/null +++ b/scripts/clean-test-residue.sh @@ -0,0 +1,302 @@ +#!/usr/bin/env bash +# +# Lists what TablePro test runs left on this Mac. With --apply, backs all of it up, then removes it. +# +# Usage: +# scripts/clean-test-residue.sh dry run: a count and a few names per row +# scripts/clean-test-residue.sh --apply back up to ~/TablePro-test-residue-/, then remove +# +# Only names traced to a test go: defaults domains in ~/Library/Preferences, the autosave records of +# sandboxed UI runs and the PluginSettingsTests keys in com.TablePro, mcp-audit-test_ databases, and +# the EphemeralDefaults, ScratchDefaults and tablepro-tests entries in the user temp directory. +# Per-connection keys in com.TablePro (QuickSwitcher.frecency. and the like) stay: a real +# connection writes the same key, and a linked-folder connection is never saved, so a key whose +# connection is missing from the store is not proof of a test. +# +# --apply refuses, exit 1, while TablePro or a TablePro test run is active. Exits 2 on a usage error. + +set -euo pipefail + +usage() { + sed -n "6,7s/^# //p" "${BASH_SOURCE[0]}" +} + +case "$#:${1:-}" in + 0:) APPLY=0 ;; + 1:--apply) APPLY=1 ;; + 1:-h | 1:--help) usage; exit 0 ;; + *) usage >&2; exit 2 ;; +esac + +[ "$(uname -s)" = Darwin ] || { echo "clean-test-residue.sh: macOS only" >&2; exit 2; } + +# The unit test host is TablePro itself; package tests run in xctest or swiftpm-testing-helper. +active() { + pgrep -x TablePro > /dev/null \ + || pgrep -f TableProUITests-Runner > /dev/null \ + || pgrep -f '(xctest|swiftpm-testing-helper).*TablePro' > /dev/null +} + +refuse_if_active() { + active || return 0 + echo "TablePro or a TablePro test run is active. Quit it, then run --apply again." >&2 + exit 1 +} + +HOME_DIR="$(cd "$HOME" && pwd -P)" +# Resolved, because /var is a symlink and tar refuses to restore through one. +TEMP_DIR="$(cd "$(getconf DARWIN_USER_TEMP_DIR)" && pwd -P)" +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT +SCRIPT="$WORK/residue.js" + +cat > "$SCRIPT" <<'JXA' +ObjC.import('Foundation') +ObjC.bindFunction('CFPreferencesCopyKeyList', ['id', ['id', 'id', 'id']]) +ObjC.bindFunction('CFPreferencesSetMultiple', ['void', ['id', 'id', 'id', 'id', 'id']]) +ObjC.bindFunction('CFPreferencesSynchronize', ['bool', ['id', 'id', 'id']]) + +const USER = $('kCFPreferencesCurrentUser') +const HOST = $('kCFPreferencesAnyHost') +const APP_DOMAIN = 'com.TablePro' +const UUID = '[0-9A-F]{8}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{12}' +const UUID_NAMED = new RegExp(UUID, 'i') +const quote = s => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + +// Each stem is traced to the test file, or the commit, that named a suite . +const SUITE_STEMS = [ + 'cl-sync-meta-', 'ClosedTabDraftStorageTests.', 'codablelist-', 'ColumnVisibilityPersistenceTests-', + 'colvis-', 'DatabaseTreeFavoriteRefreshTests.databases.', 'DatabaseTreeFavoriteRefreshTests.favorites.', + 'DatabaseTreeFavoriteRefreshTests.sync.', 'DatabaseTreeFilterStorageTests.', 'DatabaseTreeFolderNodeTests.', + 'ExternalConnectionGateTests.', 'ExternalConnectionTrustStoreTests.', 'FavoriteDatabasesStorageTests.favorites.', + 'FavoriteDatabasesStorageTests.sync.', 'FavoritesExpansionStateTests.', 'FavoriteTablesStorageTests.favorites.', + 'FavoriteTablesStorageTests.sync.', 'FilterSettingsStorageTests-', 'ForeignKeyLabelColumnTests.', + 'ImportColumnMappingStoreTests.', 'legacy-theme-fonts-', 'linkedfolder-', 'linkedsqlfolder-', + 'LoadableExtensionApprovalStoreTests-', 'LoadableExtensionGateTests-', 'MCPRemoteToolApprovalTests.', + 'MCPRemoteToolPolicyTests.', 'MCPRemoteToolScopeTests-', 'MCPServerStoreTests-', 'notifications-', + 'QuickSwitcherFrecencyTests.', 'QuickSwitcherRecentIdentityTests.', 'QuickSwitcherTests.', 'recent-', + 'RecentTablesTests.', 'registry-freshness-tests-', 'RightPanelStateTests.', 'RowImportMappingTests.', 'sc-', + 'settings-decode-', 'settings-reset-', 'sidebar-tree-', 'slashcmd-', 'slashcmd-meta-', + 'SQLTableEditAdoptionTests.', 'StartupMigrationTests-', 'syncmetadata-', 'syncscope-', + 'TableFolderStorageTests.folders.', 'TableFolderStorageTests.sync.', 'tablepro-favorite-pull-', + 'tablepro-favorite-remote-', 'tablepro-favorite-sync-', 'tablepro-purge-', 'tablepro-purge-pane-', + 'TrailingPaneRevealTests.', 'TrailingPaneStateTests.', 'tree-group-collapse-', 'tree-group-database-', + 'tree-group-selection-', 'tree-group-writes-', 'tree-groups-', 'vdf-' +] +const DOMAIN_RULES = [ + { label: 'com.TablePro.tests.* domains in ~/Library/Preferences', test: /^com\.TablePro\.tests\..+$/ }, + { label: 'com.TablePro.uitest suites in ~/Library/Preferences', test: /^com\.TablePro\.uitest(\.[0-9a-f]{16})?$/ }, + { + label: ' suites of a traced test in ~/Library/Preferences', + test: new RegExp(`^(?:(?:${SUITE_STEMS.map(quote).join('|')})${UUID}(?:\\.sync)?` + + `|test\\.MacAnalyticsProviderTests\\.[A-Za-z]+\\(\\)\\.[0-9]+\\.${UUID})$`, 'i') + } +] + +// EphemeralDefaults and ScratchDefaults name each suite by a path in the temp directory. +const TEMP_RULES = [ + { label: 'EphemeralDefaults- directories in the temp directory', test: new RegExp(`^EphemeralDefaults-${UUID}$`, 'i') }, + { label: 'com.TablePro.tests..plist suites in the temp directory', test: new RegExp(`^com\\.TablePro\\.tests\\.${UUID}\\.plist$`, 'i') } +] +const TESTS_TEMP_DIR = 'tablepro-tests' +const AUDIT_PREFIX = 'mcp-audit-test_' +const FLUSH_WAIT_SECONDS = 30 + +// Production writes these autosave names bare. Older builds appended "." +// under a UI test sandbox; SplitViewAutosaveName.current now returns nil there, and +// SplitViewAutosaveSweep leaves suffixed names alone. +const RECORD = '(?:NSSplitView Subview Frames |NSWindow Frame |NSTableView Columns v3 |NSTableView Sort Ordering v2 ' + + '|NSTableView Supports v2 |NSTableView Hidden Columns |NSOutlineView Items )' +const BARE_NAMES = [ + 'MainEditorWindow', 'welcome', 'connection-form', 'settings', 'integrations-activity', 'acknowledgements', + 'support', 'compare-sync', 'com.TablePro.DataFileWindow', 'com.TablePro.DataFileSplit', + 'com.TablePro.queryHistory.listDetail', 'com.TablePro.versionHistory.listDetail', + 'com.TablePro.queryPlanTreeSplit', 'com.TablePro.queryPlanOutline.v2', 'com.TablePro.usersRoles.mainSplit', + 'com.TablePro.usersRoles.privilegeSplit', 'com.TablePro.usersRoles.scopeOutline', 'com.TablePro.triggerSplit', + 'com.TablePro.CompareSync.main', 'com.TablePro.CompareSync.script', 'ServerDashboardSplit', + 'CellImageViewerWindow', 'GeometryMapViewerWindow', 'JSONViewerWindow', 'PhpViewerWindow', 'TextViewerWindow' +] +const SANDBOXED_RECORD = new RegExp(`^${RECORD}(?:${BARE_NAMES.map(quote).join('|')}` + + `|HistoryDrawer-${UUID}|QuerySplit-${UUID}(?:-${UUID})?)\\.(.+)$`, 'i') +// From June to August 2026 production named this split per connection, so a suffix counts only +// when the same sandbox name shows up on a record above. +const MAIN_SPLIT = /^NSSplitView Subview Frames com\.TablePro\.mainSplit\.(.+)$/ +const PLUGIN_SETTINGS_TEST = new RegExp(`^com\\.TablePro\\.plugin\\.test\\.settings\\.(?:other\\.)?${UUID}\\.[A-Za-z]+$`, 'i') + +const fm = $.NSFileManager.defaultManager +const list = dir => (fm.contentsOfDirectoryAtPathError(dir, null).js || []).map(n => n.js) +const readLines = path => { + const text = $.NSString.stringWithContentsOfFileEncodingError(path, $.NSUTF8StringEncoding, null) + return text.isNil() ? [] : text.js.split('\n').filter(line => line.length > 0) +} +const writeLines = (path, lines) => { + $(lines.map(line => `${line}\n`).join('')).writeToFileAtomicallyEncodingError(path, true, $.NSUTF8StringEncoding, null) +} +const isEmptyPlist = path => { + const dict = $.NSDictionary.dictionaryWithContentsOfFile(path) + return !dict.isNil() && Number(dict.count) === 0 +} +const dirs = (home, temp) => ({ + prefs: `${home}/Library/Preferences`, + audit: `${home}/Library/Application Support/TablePro`, + temp, + testsTemp: `${temp}/${TESTS_TEMP_DIR}` +}) + +function scan(home, temp, work, mode) { + const d = dirs(home, temp) + const out = [] + const row = (count, label, samples) => { + out.push(`${String(count).padStart(7)} ${label}`) + samples.slice(0, 3).forEach(s => out.push(` ${s}`)) + } + + const domainGroups = DOMAIN_RULES.map(rule => ({ ...rule, names: [] })) + const unproven = [] + for (const name of list(d.prefs).filter(n => n.endsWith('.plist')).map(n => n.slice(0, -6))) { + const group = domainGroups.find(g => g.test.test(name)) + if (group) group.names.push(name) + else if (UUID_NAMED.test(name)) unproven.push(name) + } + const domains = domainGroups.flatMap(g => g.names) + + const keys = ($.CFPreferencesCopyKeyList($(APP_DOMAIN), USER, HOST).js || []).map(k => k.js) + const sandboxNames = new Set() + const sandboxed = keys.filter(k => { + const m = SANDBOXED_RECORD.exec(k) + if (m) sandboxNames.add(m[1].toUpperCase()) + return m !== null + }) + const mainSplit = keys.filter(k => { + const m = MAIN_SPLIT.exec(k) + return m !== null && sandboxNames.has(m[1].toUpperCase()) + }) + const pluginTest = keys.filter(k => PLUGIN_SETTINGS_TEST.test(k)) + const deadKeys = [...sandboxed, ...mainSplit, ...pluginTest] + const dead = new Set(deadKeys) + const leftUUIDKeys = keys.filter(k => UUID_NAMED.test(k) && !dead.has(k)) + + const tempNames = list(d.temp) + const tempGroups = TEMP_RULES.map(rule => ({ + ...rule, + paths: tempNames.filter(n => rule.test.test(n)).map(n => `${d.temp}/${n}`) + })) + const testsTemp = list(d.testsTemp).map(n => `${d.testsTemp}/${n}`) + const audit = list(d.audit).filter(n => n.startsWith(AUDIT_PREFIX)).map(n => `${d.audit}/${n}`) + const files = [...tempGroups.flatMap(g => g.paths), ...testsTemp, ...audit] + + out.push(mode === 'apply' ? 'Removing:' : 'Would remove (dry run):') + domainGroups.forEach(g => row(g.names.length, g.label, g.names)) + row(sandboxed.length + mainSplit.length, 'com.TablePro autosave records of sandboxed UI runs', [...sandboxed, ...mainSplit]) + row(pluginTest.length, 'com.TablePro keys of the PluginSettingsTests plugin', pluginTest) + row(audit.length, `${AUDIT_PREFIX} files in ~/Library/Application Support/TablePro`, audit) + tempGroups.forEach(g => row(g.paths.length, g.label, g.paths)) + row(testsTemp.length, `entries in ${TESTS_TEMP_DIR} in the temp directory`, testsTemp) + out.push('Left alone, not traced to a test:') + row(unproven.length, 'UUID-named domains in ~/Library/Preferences', unproven) + row(leftUUIDKeys.length, 'com.TablePro keys that carry a UUID', leftUUIDKeys) + + writeLines(`${work}/domains`, domains) + writeLines(`${work}/keys`, deadKeys) + writeLines(`${work}/files`, files) + // Relative to /, for tar -C /. + const backup = [...domains.map(n => `${d.prefs}/${n}.plist`), ...files].map(p => p.replace(/^\/+/, '')) + writeLines(`${work}/backup`, backup) + return out.join('\n') +} + +function apply(home, temp, work) { + const d = dirs(home, temp) + const provenKey = k => SANDBOXED_RECORD.test(k) || MAIN_SPLIT.test(k) || PLUGIN_SETTINGS_TEST.test(k) + const provenDomain = n => DOMAIN_RULES.some(rule => rule.test.test(n)) + const provenFile = p => !p.includes('/../') + && [d.temp, d.audit].some(dir => p.startsWith(`${dir}/`) && p.length > dir.length + 1) + + const keys = readLines(`${work}/keys`) + const domains = readLines(`${work}/domains`) + const files = readLines(`${work}/files`) + const strays = [...keys.filter(k => !provenKey(k)), ...domains.filter(n => !provenDomain(n)), ...files.filter(p => !provenFile(p))] + if (strays.length > 0) throw new Error(`refusing, untraced entries in the work lists: ${strays.slice(0, 3).join(', ')}`) + + if (keys.length > 0) { + $.CFPreferencesSetMultiple($(), $(keys), $(APP_DOMAIN), USER, HOST) + $.CFPreferencesSynchronize($(APP_DOMAIN), USER, HOST) + } + + for (const name of domains) { + const domain = $(name) + const domainKeys = $.CFPreferencesCopyKeyList(domain, USER, HOST) + if (!domainKeys.isNil() && Number(domainKeys.count) > 0) $.CFPreferencesSetMultiple($(), domainKeys, domain, USER, HOST) + $.CFPreferencesSynchronize(domain, USER, HOST) + } + // cfprefsd writes an emptied domain out as a 42-byte plist about 5 s later, and writes it back + // if the file is already gone, so a file goes only once it reads back empty. + let pending = domains.map(name => `${d.prefs}/${name}.plist`) + for (let waited = 0; ; waited++) { + pending = pending.filter(path => { + if (!fm.fileExistsAtPath(path)) return false + if (!isEmptyPlist(path)) return true + fm.removeItemAtPathError(path, null) + return false + }) + if (pending.length === 0 || waited >= FLUSH_WAIT_SECONDS) break + $.NSThread.sleepForTimeInterval(1) + } + + const failed = files.filter(p => fm.fileExistsAtPath(p) && !fm.removeItemAtPathError(p, null)) + + const out = [`Removed ${keys.length} com.TablePro keys, ${domains.length - pending.length} domains, ${files.length - failed.length} files and directories.`] + if (pending.length > 0) out.push(`${pending.length} domain files did not read back empty within ${FLUSH_WAIT_SECONDS} s and were kept. Run --apply again.`) + if (failed.length > 0) out.push(`${failed.length} could not be removed, such as ${failed[0]}.`) + return out.join('\n') +} + +function run(argv) { + const [mode, home, temp, work] = argv + if (mode === 'apply') return apply(home, temp, work) + return scan(home, temp, work, argv[4]) +} +JXA + +jxa() { + osascript -l JavaScript "$SCRIPT" "$@" +} + +if [ "$APPLY" -eq 0 ]; then + if active; then + echo "Note: TablePro or a TablePro test run is active, so these counts can still change." + fi + jxa scan "$HOME_DIR" "$TEMP_DIR" "$WORK" dry + echo "--apply backs all of it up to ~/TablePro-test-residue-/ first." + exit 0 +fi + +refuse_if_active +jxa scan "$HOME_DIR" "$TEMP_DIR" "$WORK" apply +if [ ! -s "$WORK/keys" ] && [ ! -s "$WORK/backup" ]; then + echo "Nothing to remove." + exit 0 +fi + +BACKUP="$HOME_DIR/TablePro-test-residue-$(date +%Y%m%d-%H%M%S)" +mkdir "$BACKUP" +if [ -s "$WORK/keys" ]; then + defaults export com.TablePro "$BACKUP/com.TablePro.plist" + cp "$WORK/keys" "$BACKUP/removed-keys.txt" +fi +if [ -s "$WORK/backup" ]; then + tar -czf "$BACKUP/files.tar.gz" --no-mac-metadata -C / -T "$WORK/backup" + cp "$WORK/backup" "$BACKUP/removed-paths.txt" +fi +echo "Backup: $BACKUP" + +refuse_if_active +DOMAINS="$(wc -l < "$WORK/domains" | tr -d ' ')" +echo "Resetting $DOMAINS domains, about $((DOMAINS * 35 / 10000 + 10)) s." +jxa apply "$HOME_DIR" "$TEMP_DIR" "$WORK" +if [ -f "$BACKUP/com.TablePro.plist" ]; then + echo "Restore com.TablePro: defaults import com.TablePro \"$BACKUP/com.TablePro.plist\"" +fi +if [ -f "$BACKUP/files.tar.gz" ]; then + echo "Restore the files: tar -xzf \"$BACKUP/files.tar.gz\" -C /" +fi diff --git a/scripts/sftp-test-server.sh b/scripts/sftp-test-server.sh index 4c8e6a47fc..2f21825562 100755 --- a/scripts/sftp-test-server.sh +++ b/scripts/sftp-test-server.sh @@ -1,21 +1,20 @@ #!/usr/bin/env bash # -# Runs a real OpenSSH server for the SFTP integration tests. +# Runs a real OpenSSH server for scripts/probes/check-sftp-contract.sh. # # The SFTP transport's two most dangerous behaviours cannot be observed against a fake: # libssh2_sftp_write returns short constantly and silently truncates a file if the caller advances # by the requested length rather than the returned one, and libssh2_sftp_rename_ex fails onto an # existing file because its OVERWRITE flag is SFTP v5 and OpenSSH speaks v3. Both were measured -# against this container, and LibSSH2SFTPSessionIntegrationTests keeps them measured. +# against this container, and the probe keeps them measured. # # Usage: # sftp-test-server.sh up # start it, wait until it answers # sftp-test-server.sh down # remove it # sftp-test-server.sh status # report whether it is listening # -# The tests find it by connecting, not by reading an environment variable: xcodebuild does not -# pass the invoking shell's environment to the test host, so an exported variable arrives as -# nothing and the suite silently reports zero cases. +# The probe connects to 127.0.0.1:22022 and reads no environment variable, so it needs the +# default TABLEPRO_SFTP_TEST_PORT. set -euo pipefail