diff --git a/.github/actions/determine-release-modes/README.md b/.github/actions/determine-release-modes/README.md index 9085c0c3..c0d8cbe2 100644 --- a/.github/actions/determine-release-modes/README.md +++ b/.github/actions/determine-release-modes/README.md @@ -10,16 +10,16 @@ type and optional input overrides. | Name | Description | Required | Default | | ---- | ----------- | -------- | ------- | -| `dry-run` | Override dry-run mode (auto-detected from event when empty) | no | `""` | -| `publish` | Override publish flag (auto-detected from event when empty) | no | `""` | +| `dry-run` | Override dry-run (auto-detected) | no | `""` | +| `publish` | Override publish flag (auto-detected) | no | `""` | ## Outputs | Name | Description | | ---- | ----------- | -| `dry-run` | `"true"` or `"false"` indicating dry-run mode | -| `should-publish` | `"true"` or `"false"` indicating whether to publish to a release | -| `should-upload-workflow-artifacts` | `"true"` or `"false"` indicating whether to upload workflow artefacts | +| `dry-run` | Dry-run (`"true"` or `"false"`) | +| `should-publish` | Publish (`"true"` or `"false"`) | +| `should-upload-workflow-artifacts` | Upload (`"true"` or `"false"`) | ## Usage @@ -56,7 +56,7 @@ type and optional input overrides. The action derives modes based on the GitHub event type: | Event | Default dry-run | Default publish | Artefacts | -|-------|-----------------|-----------------|-----------| +| --- | --- | --- | --- | | `push` (tag) | `false` | `true` | Uploaded | | `workflow_call` | From inputs | From inputs | If not dry-run | | `pull_request` | `true` | `false` | None | diff --git a/.github/actions/ensure-cargo-version/README.md b/.github/actions/ensure-cargo-version/README.md index 824f907d..88c7fbb2 100644 --- a/.github/actions/ensure-cargo-version/README.md +++ b/.github/actions/ensure-cargo-version/README.md @@ -1,22 +1,23 @@ # ensure-cargo-version -Validate that the Git tag triggering a release workflow matches the version in one or more Cargo manifests. +Validate that the Git tag triggering a release workflow matches the version in +one or more Cargo manifests. ## Inputs | Name | Required | Default | Description | | ---- | -------- | ------- | ----------- | -| `manifests` | No | `Cargo.toml` | Newline or whitespace separated list of Cargo manifest paths to check. Paths are resolved relative to the GitHub workspace. | -| `tag-prefix` | No | `v` | Prefix stripped from the Git reference name before comparing against manifest versions. Use an empty string to disable prefix removal. | -| `check-tag` | No | `true` | Disable tag comparison by supplying a falsey value (case-insensitive `false`, `0`, `no`, `off`, or an empty string). Truthy values (`true`, `1`, `yes`, `on`) enable comparison while still attempting to read the tag for output purposes. | +| `manifests` | No | `Cargo.toml` | Manifest paths (newline separated) | +| `tag-prefix` | No | `v` | Git tag prefix to strip | +| `check-tag` | No | `true` | Enable tag comparison | ## Outputs | Name | Description | | ---- | ----------- | -| `version` | Version extracted from the tag reference after removing the configured prefix. | -| `crate-version` | Version read from the first manifest path provided (after resolution) after resolving workspace inheritance. | -| `crate-name` | Package name read from the first manifest path provided (after resolution). | +| `version` | Version from tag (prefix removed) | +| `crate-version` | Version from first manifest | +| `crate-name` | Package name from first manifest path | ## Usage diff --git a/.github/actions/export-cargo-metadata/README.md b/.github/actions/export-cargo-metadata/README.md index 5aa3ee46..05f7fccb 100644 --- a/.github/actions/export-cargo-metadata/README.md +++ b/.github/actions/export-cargo-metadata/README.md @@ -15,10 +15,10 @@ manifest and export as GitHub Actions outputs and environment variables. | Name | Description | | ---- | ----------- | -| `name` | Package name from `[package].name` | -| `version` | Package version from `[package].version` (resolves workspace inheritance) | -| `bin-name` | Binary name from first `[[bin]].name` or `[package].name` | -| `description` | Package description from `[package].description` | +| `name` | Package name | +| `version` | Package version | +| `bin-name` | Binary name | +| `description` | Package description | ## Usage diff --git a/.github/actions/linux-packages/README.md b/.github/actions/linux-packages/README.md index e2419355..8f72c656 100644 --- a/.github/actions/linux-packages/README.md +++ b/.github/actions/linux-packages/README.md @@ -11,27 +11,27 @@ nFPM. | Name | Type | Default | Description | Required | | ---- | ---- | ------- | ----------- | -------- | -| project-dir | string | `.` | Directory containing the compiled binary, man pages and optional license file. | no | -| package-name | string | _empty_ | Package identifier written to the nFPM manifest. Defaults to `bin-name` when omitted. | no | -| bin-name | string | — | Name of the release binary to package. | yes | -| target | string | `x86_64-unknown-linux-gnu` | Rust target triple used for the build. | no | -| version | string | — | Version number recorded in the package metadata (for example `1.2.3`). | yes | -| formats | string | `deb` | Comma-, space-, or newline-separated list of package formats (for example `deb,rpm` or a multi-line value). | no | -| release | string | _empty_ | Package release or revision override. Uses the packaging helper default when omitted. | no | -| arch | string | _empty_ | Override the nFPM/GOARCH architecture. Auto-detected from `target` when not set. | no | -| maintainer | string | _empty_ | Maintainer entry for the generated package metadata. | no | -| homepage | string | _empty_ | Homepage URL recorded in package metadata. | no | -| license | string | _empty_ | Software license declared in the package metadata. | no | -| section | string | _empty_ | Package section/category used by Debian-based distributions. | no | -| description | string | _empty_ | Long description stored in the package metadata. | no | -| man-paths | string | _empty_ | Comma-, space-, or newline-separated list of man page paths relative to `project-dir`. | no | -| man-section | string | _empty_ | Default man section applied when a path lacks a suffix (for example `1`). | no | -| man-stage | string | _empty_ | Directory used to stage gzipped man pages before invoking nFPM. | no | -| binary-dir | string | _empty_ | Cargo `target` directory containing build artefacts. | no | -| outdir | string | _empty_ | Directory where packages will be written. | no | -| config-path | string | _empty_ | Location to write the generated `nfpm.yaml` configuration. | no | -| deb-depends | string | _empty_ | Comma-, space-, or newline-separated Debian runtime dependencies (each entry becomes a separate dependency in the generated manifest). | no | -| rpm-depends | string | _empty_ | Comma-, space-, or newline-separated RPM runtime dependencies. Falls back to Debian deps when omitted. | no | +| project-dir | string | `.` | Binary, man, license dir | no | +| package-name | string | _empty_ | Package ID (defaults to bin) | no | +| bin-name | string | — | Binary name | yes | +| target | string | `x86_64-unknown-linux-gnu` | Rust target triple | no | +| version | string | — | Package version | yes | +| formats | string | `deb` | Formats (e.g. `deb,rpm`) | no | +| release | string | _empty_ | Release/revision override | no | +| arch | string | _empty_ | Architecture (auto-detected) | no | +| maintainer | string | _empty_ | Package maintainer | no | +| homepage | string | _empty_ | Homepage URL | no | +| license | string | _empty_ | Software license | no | +| section | string | _empty_ | Package section | no | +| description | string | _empty_ | Package description | no | +| man-paths | string | _empty_ | Man page paths | no | +| man-section | string | _empty_ | Default man section | no | +| man-stage | string | _empty_ | Man staging directory | no | +| binary-dir | string | _empty_ | Cargo target directory | no | +| outdir | string | _empty_ | Output directory | no | +| config-path | string | _empty_ | nfpm config path | no | +| deb-depends | string | _empty_ | Debian dependencies | no | +| rpm-depends | string | _empty_ | RPM dependencies | no | Before invoking sibling actions the composite mirrors the repository snapshot that GitHub already downloaded for the action into a local `_self/` directory. diff --git a/.github/actions/macos-package/README.md b/.github/actions/macos-package/README.md index 62b26e5e..14a6ccfd 100644 --- a/.github/actions/macos-package/README.md +++ b/.github/actions/macos-package/README.md @@ -12,24 +12,24 @@ executing any packaging commands. | Name | Description | Required | Default | | ---- | ----------- | -------- | ------- | -| `name` | Display name of the packaged CLI (also used for output filenames). | yes | – | -| `identifier` | Reverse-DNS package identifier (for example `com.example.tool`). | yes | – | -| `install-prefix` | Installation prefix within the target filesystem. | no | `/usr/local` | -| `binary` | Path to the compiled binary to install. | yes | – | -| `manpage` | Optional path to a man page (`.1`, `.1.gz`, etc.) to embed. | no | empty | -| `license-file` | Path to the license text copied into the package and optional UI. | no | `LICENSE` | -| `include-license-panel` | Show the license text inside the installer UI (requires `license-file`). | no | `false` | -| `version` | Override the package version. Defaults to the Git tag (`v*`) or falls back to the commit SHA. | no | derived | -| `developer-id-installer` | Developer ID Installer identity used with `productsign` for notarized packages. | no | empty | +| `name` | Display name | yes | – | +| `identifier` | Reverse-DNS identifier | yes | – | +| `install-prefix` | Install prefix | no | `/usr/local` | +| `binary` | Binary path | yes | – | +| `manpage` | Man page path | no | empty | +| `license-file` | License file path | no | `LICENSE` | +| `include-license-panel` | Show license in UI | no | `false` | +| `version` | Version override | no | derived | +| `developer-id-installer` | Developer ID identity | no | empty | ## Outputs | Name | Description | | ---- | ----------- | -| `version` | Resolved version used for the package metadata. | -| `version-build-metadata` | Short commit SHA recorded when falling back to a default version. | -| `pkg-path` | Path to the generated installer archive (`dist/-.pkg`). | -| `signed-pkg-path` | Path to the signed installer archive when signing succeeds; empty otherwise. | +| `version` | Resolved package version | +| `version-build-metadata` | Commit SHA (fallback) | +| `pkg-path` | Installer package path | +| `signed-pkg-path` | Signed package path | ## Usage diff --git a/.github/actions/ratchet-coverage/README.md b/.github/actions/ratchet-coverage/README.md index 1f574c4f..2aecb626 100644 --- a/.github/actions/ratchet-coverage/README.md +++ b/.github/actions/ratchet-coverage/README.md @@ -5,10 +5,10 @@ coverage percentage falls below a stored baseline. ## Inputs -| Name | Description | Required | Default | -| ------------- | ------------------------------------------------------------------ | -------- | -------------------- | -| baseline-file | File used to persist the baseline coverage percentage between runs | no | `.coverage-baseline` | -| args | Additional arguments passed to `cargo llvm-cov` | no | `''` | +| Name | Description | Required | Default | +| --- | --- | --- | --- | +| baseline-file | Baseline coverage file | no | `.coverage-baseline` | +| args | Additional cargo llvm-cov args | no | `''` | ## Outputs diff --git a/.github/actions/release-to-pypi-uv/README.md b/.github/actions/release-to-pypi-uv/README.md index 15bf5f6a..1ff186e0 100644 --- a/.github/actions/release-to-pypi-uv/README.md +++ b/.github/actions/release-to-pypi-uv/README.md @@ -7,16 +7,16 @@ Build and publish Python distributions via | Name | Description | Required | Default | | --- | --- | --- | --- | -| tag | Tag to release (e.g. `v1.2.3`). Required when the workflow is not running on a tag ref. | no | _(empty)_ | -| require-confirmation | Require a manual confirmation string before publishing. | no | `false` | -| confirm | Confirmation string. Must equal `release ` when `require-confirmation` is true. | no | _(empty)_ | -| environment-name | GitHub environment to reference in the release summary. | no | `pypi` | -| uv-index | Optional uv index name to publish to (e.g. `testpypi`). Must exist in `tool.uv.index`. | no | _(empty)_ | -| toml-glob | Glob used to discover `pyproject.toml` files for version validation. | no | `**/pyproject.toml` | -| skip-directories | Comma- or newline-separated directory names to skip during discovery. | no | _(empty)_ | -| fail-on-dynamic-version | Fail when a project declares a dynamic PEP 621 version instead of a literal string. | no | `false` | -| fail-on-empty | Fail when no `pyproject.toml` files match the discovery glob. | no | `false` | -| python-version | Python version to install and use for all uv commands. | no | `3.13` | +| tag | Tag to release (e.g. `v1.2.3`) | no | _(empty)_ | +| require-confirmation | Require manual confirmation | no | `false` | +| confirm | Confirmation string | no | _(empty)_ | +| environment-name | GitHub environment name | no | `pypi` | +| uv-index | uv index name (e.g. `testpypi`) | no | _(empty)_ | +| toml-glob | Glob for `pyproject.toml` discovery | no | `**/pyproject.toml` | +| skip-directories | Directories to skip | no | _(empty)_ | +| fail-on-dynamic-version | Fail on dynamic PEP 621 version | no | `false` | +| fail-on-empty | Fail on empty discovery | no | `false` | +| python-version | Python version | no | `3.13` | The composite action installs the interpreter requested through `python-version` before invoking any uv commands, ensuring builds run against the expected @@ -36,8 +36,11 @@ uses additional transient paths that should be excluded. | tag | Resolved release tag. | | version | Resolved release version (tag without the leading `v`). | -> **Required permissions**: set the job to `permissions: contents: read` and `permissions: id-token: write` so uv Trusted Publishing can exchange an OpenID Connect (OIDC) token with PyPI. -> The composite action forwards the workflow's `GITHUB_TOKEN` to its scripts as `GH_TOKEN`, so you do not need to add an extra `env` block. +> **Required permissions**: set the job to `permissions: contents: read` and +> `permissions: id-token: write` so uv Trusted Publishing can exchange an +> OpenID Connect (OIDC) token with PyPI. +> The composite action forwards the workflow's `GITHUB_TOKEN` to its scripts +> as `GH_TOKEN`, so you do not need to add an extra `env` block. ## Usage diff --git a/.github/actions/rust-build-release/CHANGELOG.md b/.github/actions/rust-build-release/CHANGELOG.md index c8f26447..37b7b707 100644 --- a/.github/actions/rust-build-release/CHANGELOG.md +++ b/.github/actions/rust-build-release/CHANGELOG.md @@ -11,17 +11,24 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Added - Cross-compile and stage `x86_64-unknown-illumos` artefacts from Linux runners. -- Provide shared packaging fixtures and helpers that build the sample project once and produce `.deb` and `.rpm` artefacts for the integration tests. -- Support staging and packaging for `unknown-linux-musl` targets alongside GNU triples for x86_64, aarch64, i686, arm*, and riscv64 builds. -- Require containerized `cross` builds for FreeBSD targets on non-FreeBSD hosts to enable `x86_64-unknown-freebsd` cross-compilation. -- Automatically export `CROSS_CONTAINER_ENGINE` for the detected container runtime when running FreeBSD builds with `cross`. +- Provide shared packaging fixtures and helpers that build the sample project + once and produce `.deb` and `.rpm` artefacts for the integration tests. +- Support staging and packaging for `unknown-linux-musl` targets alongside GNU + triples for x86_64, aarch64, i686, arm*, and riscv64 builds. +- Require containerized `cross` builds for FreeBSD targets on non-FreeBSD hosts + to enable `x86_64-unknown-freebsd` cross-compilation. +- Automatically export `CROSS_CONTAINER_ENGINE` for the detected container + runtime when running FreeBSD builds with `cross`. - Add a `manifest-path` input for selecting an alternate Cargo manifest. - Add a `toolchain` input for explicitly overriding the resolved build toolchain. ### Fixed -- Pin `setup-rust` to the commit behind `setup-rust-v1`, so toolchain inputs and OS guards apply when invoked from external repositories. -- Resolve toolchains from the target repository before falling back to the action's bundled default: explicit input first, then `rust-toolchain.toml` or `rust-toolchain`, then manifest `rust-version`. +- Pin `setup-rust` to the commit behind `setup-rust-v1`, so toolchain inputs + and OS guards apply when invoked from external repositories. +- Resolve toolchains from the target repository before falling back to the + action's bundled default: explicit input first, then `rust-toolchain.toml` + or `rust-toolchain`, then manifest `rust-version`. ## [0.1.0] - 2025-09-10 diff --git a/.github/actions/upload-codescene-coverage/README.md b/.github/actions/upload-codescene-coverage/README.md index ba117241..c313f0c2 100644 --- a/.github/actions/upload-codescene-coverage/README.md +++ b/.github/actions/upload-codescene-coverage/README.md @@ -4,12 +4,12 @@ Upload coverage reports to CodeScene and cache the CLI for faster runs. ## Inputs -| Name | Description | Required | Default | -| ------------------ | ------------------------------------------------------------ | -------- | ----------- | -| path | Coverage file path; blank or `__auto__` infers automatically | no | `__auto__` | -| format | Coverage format (`cobertura` or `lcov`) | no | `cobertura` | -| access-token | CodeScene project access token | yes | | -| installer-checksum | SHA-256 checksum of the installer script | no | | +|Name|Description|Required|Default| +|---|---|---|---| +|path|Coverage file path; blank or `__auto__` infers automatically|no|`__auto__`| +|format|Coverage format (`cobertura` or `lcov`)|no|`cobertura`| +|access-token|CodeScene project access token|yes|| +|installer-checksum|SHA-256 checksum of the installer script|no|| If `path` is empty or `__auto__`, the action looks for `lcov.info` when `format` is `lcov`, or `coverage.xml` when `format` is `cobertura`. The diff --git a/.github/actions/upload-release-assets/README.md b/.github/actions/upload-release-assets/README.md index 1b24289e..135fa151 100644 --- a/.github/actions/upload-release-assets/README.md +++ b/.github/actions/upload-release-assets/README.md @@ -14,7 +14,7 @@ release state. | `release-tag` | Git tag identifying the release to publish to | yes | - | | `bin-name` | Binary name used to derive artefact names | yes | - | | `dist-dir` | Directory containing staged artefacts | no | `dist` | -| `dry-run` | When true, only validate artefacts and print the upload plan | no | `"false"` | +| `dry-run` | Validate artefacts without uploading | no | `"false"` | | `clobber` | Overwrite existing assets with the same name | no | `"true"` | ## Outputs @@ -90,7 +90,8 @@ Files in nested directories are namespaced with their path prefix, replacing 1. **Discovery**: Recursively scan `dist-dir` for matching artefacts 2. **Validation**: Verify files are non-empty and have unique asset names -3. **Upload**: Use `gh release upload` to publish artefacts (or print plan in dry-run mode) +3. **Upload**: Use `gh release upload` to publish artefacts (or print plan in + dry-run mode) ### Error Handling diff --git a/.github/actions/windows-package/README.md b/.github/actions/windows-package/README.md index f6076ad9..420db5f2 100644 --- a/.github/actions/windows-package/README.md +++ b/.github/actions/windows-package/README.md @@ -9,8 +9,9 @@ one composite call. - Installs the WiX CLI and UI extension on the runner. - Resolves the MSI version from an explicit input or a tagged Git reference. -- Builds a single-file MSI (`EmbedCab="yes"`) from supplied WiX authoring or a generated template that - installs the provided application and supporting files. +- Builds a single-file MSI (`EmbedCab="yes"`) from supplied WiX authoring or a + generated template that installs the provided application and supporting + files. - Optionally uploads the generated MSI via `actions/upload-artifact`. > [!IMPORTANT] @@ -32,37 +33,34 @@ inputs): └─ README.pdf # documentation shipped with the installer ``` -The `installer/Package.wxs` authoring is optional—omit it when using the default template and -provide the executable (and optional additional files) via the `application-path` and -`additional-files` inputs. +The `installer/Package.wxs` authoring is optional—omit it when using the +default template and provide the executable (and optional additional files) via +the `application-path` and `additional-files` inputs. ## Inputs | Name | Required | Default | Description | | ---- | -------- | ------- | ----------- | -| `wxs-path` | no | `''` | Path to the WiX authoring file used to build the MSI. When omitted a default template is rendered. | -| `application-path` | no | `''` | Main executable or bundle to install. Append `\|relative\\path` to customise the install location. | -| `additional-files` | no | `''` | Additional `source\|relative\\path` mappings (one per line) to include in the installer. | -| `product-name` | no | `''` | Product name used by the generated template when `wxs-path` is omitted. | -| `manufacturer` | no | `''` | Manufacturer string embedded by the generated template. | -| `install-dir-name` | no | `''` | Optional install directory name (defaults to a sanitised product name). | -| `description` | no | `''` | Installer description stored in MSI summary information when templating. | -| `upgrade-code` | no | `''` | Optional UpgradeCode GUID. A deterministic GUID is derived when omitted. | -| `architecture` | no | `x64` | Architecture supplied to `wix build` (`x86`, `x64`, or `arm64`). | -| `version` | no | _auto_ | Version embedded in the MSI. Defaults to a numeric tag-derived value or `0.0.0`. | -| `dotnet-version` | no | `8.0.x` | .NET SDK version installed before running WiX. | -| `wix-tool-version` | no | _latest_ | Specific version of the `wix` .NET global tool to install. | -| `wix-extension` | no | `WixToolset.UI.wixext` | WiX extension coordinate loaded during the build. | -| `wix-extension-version` | no | `''` | Version suffix appended to the extension coordinate. When omitted, the action auto-matches the installed WiX CLI major version (for example `WixToolset.UI.wixext/7` with WiX v7). | -| `output-basename` | no | `MyApp` | Base name used when creating the MSI file. | -| `output-directory` | no | `out` | Directory where the MSI artefact is created. | -| `license-plaintext-path` | no | `''` | Optional path to a UTF-8 (with or without BOM) plain text license | -| | | | that will be converted to RTF using the default Calibri 11 pt | -| | | | template. | -| `license-rtf-path` | no | `''` | Output path for the generated license RTF when converting from | -| | | | plain text. Defaults to replacing the input suffix with `.rtf`. | -| `upload-artefact` | no | `true` | When `true`, publishes the MSI using `actions/upload-artifact`. | -| `artefact-name` | no | `msi` | Name of the uploaded artefact. | +| `wxs-path` | no | `''` | WiX authoring file path | +| `application-path` | no | `''` | Main executable path | +| `additional-files` | no | `''` | Extra files to include | +| `product-name` | no | `''` | Product name | +| `manufacturer` | no | `''` | Manufacturer name | +| `install-dir-name` | no | `''` | Install directory name | +| `description` | no | `''` | MSI description | +| `upgrade-code` | no | `''` | UpgradeCode GUID | +| `architecture` | no | `x64` | Architecture | +| `version` | no | _auto_ | MSI version | +| `dotnet-version` | no | `8.0.x` | .NET version | +| `wix-tool-version` | no | _latest_ | WiX version | +| `wix-extension` | no | `WixToolset.UI.wixext` | WiX extension | +| `wix-extension-version` | no | `''` | Extension version | +| `output-basename` | no | `MyApp` | Base name for generated MSI file | +| `output-directory` | no | `out` | Directory for MSI output | +| `license-plaintext-path` | no | `''` | UTF-8 license to convert to RTF | +| `license-rtf-path` | no | `''` | Output path for RTF license | +| `upload-artefact` | no | `true` | Upload MSI via `actions/upload-artifact` | +| `artefact-name` | no | `msi` | Name of uploaded artefact | ## Outputs @@ -102,27 +100,29 @@ preprocessor: ``` -The `$(var.Version)` preprocessor expression resolves to the version string the -action passes via `-dVersion=...`, ensuring the MSI `Package` uses the same -value that appears in the generated filename. +The `$(var.Version)` preprocessor expression resolves to the version +string the action passes via `-dVersion=...`, ensuring the MSI `Package` +uses the same value that appears in the generated filename. When `version` is omitted the action inspects `GITHUB_REF_TYPE` and -`GITHUB_REF_NAME`. Only tag refs that resemble `v..` (the -minor and build segments are optional) are used to derive the MSI version. All -other refs—including branches and tags with non-numeric suffixes—fall back to -`0.0.0`. +`GITHUB_REF_NAME`. Only tag refs that resemble `v..` +(the minor and build segments are optional) are used to derive the MSI +version. All other refs—including branches and tags with non-numeric +suffixes—fall back to `0.0.0`. MSI ProductVersion components must be integers where the major and minor -segments are `0–255` and the build segment is `0–65535`. Values outside those -ranges cause the action to fail fast so that WiX receives a valid version. - -To display a license in the installer UI, either provide an RTF file directly -or add a UTF-8 plain text document and set `license-plaintext-path` so the -action converts it to RTF prior to invoking WiX. UTF-8 input with or without a -byte-order mark is accepted—the converter strips any BOM and renders the text -using Calibri 11 pt by default. When no explicit `license-rtf-path` is set the -generated file replaces the source suffix with `.rtf`, making it easy to refer -to a stable path from WiX authoring. For example: +segments are `0–255` and the build segment is `0–65535`. Values outside +those ranges cause the action to fail fast so that WiX receives a valid +version. + +To display a license in the installer UI, either provide an RTF file +directly or add a UTF-8 plain text document and set +`license-plaintext-path` so the action converts it to RTF prior to invoking +WiX. UTF-8 input with or without a byte-order mark is accepted—the +converter strips any BOM and renders the text using Calibri 11 pt by +default. When no explicit `license-rtf-path` is set the generated file +replaces the source suffix with `.rtf`, making it easy to refer to a stable +path from WiX authoring. For example: ```xml diff --git a/.gitignore b/.gitignore index f511a98f..79f830e2 100644 --- a/.gitignore +++ b/.gitignore @@ -166,3 +166,4 @@ target/ # Episodic nested repository (separate git history) /episodic-work/ +docs/architecture/ diff --git a/README.md b/README.md index 6c467491..356cb182 100644 --- a/README.md +++ b/README.md @@ -4,30 +4,30 @@ GitHub Actions ## Available actions -| Name | Path | Latest major | -| ------------------------- | ------------------------------------------- | ------------ | -| Determine release modes | `.github/actions/determine-release-modes` | v1 | -| Ensure Cargo version | `.github/actions/ensure-cargo-version` | v1 | -| Export Cargo metadata | `.github/actions/export-cargo-metadata` | v1 | -| Export Postgres URL | `.github/actions/export-postgres-url` | v1 | -| Generate coverage | `.github/actions/generate-coverage` | v1 | -| Linux packages | `.github/actions/linux-packages` | v1 | -| macOS package | `.github/actions/macos-package` | v1 | -| Ratchet coverage | `.github/actions/ratchet-coverage` | v1 | -| Release to PyPI (uv) | `.github/actions/release-to-pypi-uv` | v1 | -| Rust build release | `.github/actions/rust-build-release` | v1 | -| Setup Rust | `.github/actions/setup-rust` | v1 | -| Stage release artefacts | `.github/actions/stage-release-artefacts` | v1 | -| Upload CodeScene Coverage | `.github/actions/upload-codescene-coverage` | v1 | -| Upload release assets | `.github/actions/upload-release-assets` | v1 | -| Validate Linux packages | `.github/actions/validate-linux-packages` | v1 | -| Windows package | [`./.github/actions/windows-package`](./.github/actions/windows-package/README.md) | v0 | +| Name | Path | Latest major | +| --- | --- | --- | +| Determine release modes | `.github/actions/determine-release-modes` | v1 | +| Ensure Cargo version | `.github/actions/ensure-cargo-version` | v1 | +| Export Cargo metadata | `.github/actions/export-cargo-metadata` | v1 | +| Export Postgres URL | `.github/actions/export-postgres-url` | v1 | +| Generate coverage | `.github/actions/generate-coverage` | v1 | +| Linux packages | `.github/actions/linux-packages` | v1 | +| macOS package | `.github/actions/macos-package` | v1 | +| Ratchet coverage | `.github/actions/ratchet-coverage` | v1 | +| Release to PyPI (uv) | `.github/actions/release-to-pypi-uv` | v1 | +| Rust build release | `.github/actions/rust-build-release` | v1 | +| Setup Rust | `.github/actions/setup-rust` | v1 | +| Stage release artefacts | `.github/actions/stage-release-artefacts` | v1 | +| Upload CodeScene Coverage | `.github/actions/upload-codescene-coverage` | v1 | +| Upload release assets | `.github/actions/upload-release-assets` | v1 | +| Validate Linux packages | `.github/actions/validate-linux-packages` | v1 | +| Windows package | [`./.github/actions/windows-package`](./.github/actions/windows-package/README.md) | v0 | ## Reusable workflows -| Name | Path | -| ------------------------ | ---------------------------------------------- | -| Dependabot auto-merge | `.github/workflows/dependabot-automerge.yml` | +| Name | Path | +| --- | --- | +| Dependabot auto-merge | `.github/workflows/dependabot-automerge.yml` | ## Development @@ -36,4 +36,4 @@ Format, validate and test the repository: ```sh make fmt make test -``` \ No newline at end of file +``` diff --git a/docs/adr/0001-stable-manpage-path.md b/docs/adr/0001-stable-manpage-path.md index f74f445a..35335109 100644 --- a/docs/adr/0001-stable-manpage-path.md +++ b/docs/adr/0001-stable-manpage-path.md @@ -29,13 +29,13 @@ absent, emitting a `::warning::` annotation on fallback. ## Consequences -* **Positive:** Staging is decoupled from Cargo's content-addressed build +- **Positive:** Staging is decoupled from Cargo's content-addressed build directories. The path is predictable in CI logs and scripts. -* **Positive:** `cargo:rerun-if-env-changed=CARGO_TARGET_DIR` ensures the +- **Positive:** `cargo:rerun-if-env-changed=CARGO_TARGET_DIR` ensures the build script reruns when `cross` changes the container-mounted target directory, preventing stale-cache failures. -* **Negative:** Consuming projects must update their `build.rs` to write to the +- **Negative:** Consuming projects must update their `build.rs` to write to the stable location. Build scripts that have not been updated will trigger the fallback warning until they adopt the new convention. -* **Neutral:** The legacy glob fallback is retained indefinitely to avoid +- **Neutral:** The legacy glob fallback is retained indefinitely to avoid breaking existing consumers on day one of adoption. diff --git a/docs/cmd-mox-users-guide.md b/docs/cmd-mox-users-guide.md index b64297cc..8de0d3d3 100644 --- a/docs/cmd-mox-users-guide.md +++ b/docs/cmd-mox-users-guide.md @@ -104,9 +104,9 @@ The design document lists the available comparators: - `Predicate` Each comparator is a callable that returns `True` on match. -`with_matching_args` expects one comparator per argv element (excluding the program name, i.e., `argv[1:]`), -and `with_stdin` accepts either an exact string or a predicate `Callable[[str], bool]` -for flexible input checks. +`with_matching_args` expects one comparator per argv element (excluding the +program name, i.e., `argv[1:]`), and `with_stdin` accepts either an exact +string or a predicate `Callable[[str], bool]` for flexible input checks. ## Running tests diff --git a/docs/composite-actions-vs-full-workflows.md b/docs/composite-actions-vs-full-workflows.md index 53eb4860..0b978715 100644 --- a/docs/composite-actions-vs-full-workflows.md +++ b/docs/composite-actions-vs-full-workflows.md @@ -4,16 +4,16 @@ Composite actions are brilliant for bundling a handful of steps you want to reuse, but they remain a *single* workflow-step in the caller job. That design imposes several hard limits you don’t face when you write a full workflow. -| Area | What a **composite action can’t** do | Why a **full workflow** can | -| ------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | -| **Jobs & runners** | Define its own `jobs`, `runs-on`, `services`, `container`, `strategy.matrix`, `timeout-minutes`, `concurrency`, `needs`, or `permissions`. | A workflow owns the job graph, so it can orchestrate multiple jobs on different runners. | -| **Triggers** | Cannot specify `on:` events, `workflow_dispatch`, `schedule`, etc. | Workflows are entry-points; they respond to events directly. | -| **Top-level keys** | Keys such as `env`, `defaults`, `shell`, `continue-on-error`, `timeout-minutes`, `permissions` are **not permitted** under `runs:`. Only `using`, `steps` (plus `pre`, `post`, `post-if`) are allowed. ([docs.github.com](http://docs.github.com), [docs.github.com](http://docs.github.com)) | Workflows accept the full YAML surface. | -| **Secrets & variables** | Has *no* automatic access to repository/organisation secrets, variables, or `vault`. The caller must pass data explicitly via `inputs` or per-step `env`. | Workflows can read any secret/variable configured for the run. | -| **Logging & visibility** | The runner log collapses everything to one line (`uses: author/repo@ref`). Inner steps appear only if you expand the step, making per-step timing and annotations harder to read. ([docs.github.com](http://docs.github.com)) | Each job and step is logged separately by default. | -| **Conditionals & matrix** | No top-level `if:` or matrix-level filtering. Conditionals have to live on individual steps inside the composite. | Workflows support `if:` at job, step, and reusable-workflow call sites, plus full matrix fan-out. | -| **Outputs** | Can emit outputs, but only simple strings set by `echo "::set-output name=foo::bar"`; cannot expose artefacts or cache scopes of its own. | Workflows can upload/download artefacts, define cache keys, and expose complex outputs between jobs. | -| **Nested depth** | Useful inside a single job, but cannot *itself* call another composite action that lives in the *same* repository path (circular reference guard). | Workflows may call up to 20 distinct reusable workflows (four levels deep). ([docs.github.com](http://docs.github.com)) | +| Area | Composite action | Full workflow | +| --- | --- | --- | +| **Jobs** | No custom jobs, runners, matrix | Full job orchestration | +| **Triggers** | No `on:` events or schedules | Direct event response | +| **Config keys** | Only `using`, `steps`, `pre`, `post` | Full YAML surface | +| **Secrets** | No auto access, must pass via inputs | Direct read from config | +| **Logging** | Single line, steps hidden | Per-job/step logging | +| **Conditionals** | Step-level only | Job/step/workflow level | +| **Outputs** | Strings only, no artifacts | Artifacts, cache, complex | +| **Nesting** | No self-references | 20 workflows, 4 levels | ______________________________________________________________________ diff --git a/docs/execplans/1-1-4-record-the-profile-rule-expression-policy.md b/docs/execplans/1-1-4-record-the-profile-rule-expression-policy.md new file mode 100644 index 00000000..6348a806 --- /dev/null +++ b/docs/execplans/1-1-4-record-the-profile-rule-expression-policy.md @@ -0,0 +1,567 @@ +# Record the Profile Rule-Expression Policy (1.1.4) + +This ExecPlan (execution plan) is a living document. The sections +`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, +`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as +work proceeds. + +Status: DRAFT + +## Purpose / big picture + +Prosidy Darn must decide whether profile configuration files allow arbitrary +custom rule expressions or only named rule weights before Phase 2 implementation +begins. This decision affects the segmenter architecture, test strategy, and +feature scope for v1. Once locked, changing this decision requires breaking +changes to the profile schema. + +A user will be able to configure profile files that weight and parameterize +rules. After this decision is recorded in an ADR, the segmenter component can +be designed without uncertainty about expression capability, and Phase 2 can +proceed with a coherent v1 contract. + +## Constraints + +Hard invariants that must hold throughout implementation. + +- The decision must not assume the existence of an expression language. No new + expression-language design decision may be introduced during this task. +- The profile schema must be documented in the ADR with sufficient specificity + that implementation teams can build adapters without asking clarifying + questions. +- All decision criteria and trade-offs must be visible in the ADR. Hidden + rationale or undocumented constraints will cause rework in Phase 2. +- The decision must be reversible in v1.1 without breaking v1.0 profiles. + Architecture must reserve space for future expressions without breaking + changes. +- No code changes to existing Prosidy Darn codebase are permitted during this + task. This is a decision-documentation task, not an implementation task. +- The ADR must follow the format and style of existing ADRs under `docs/adr/`. + See `docs/adr/0001-stable-manpage-path.md` and + `docs/adr/0002-explicit-ps-module-name.md` for format. + +## Tolerances (exception triggers) + +Thresholds that trigger escalation when breached. + +- **Ambiguity:** If the decision permits multiple interpretations of what + "named rule weights" means or how expressions would be added in v1.1, stop + and escalate with the specific ambiguous cases. +- **Prior art:** If research into open-source tools reveals a pattern materially + different from the recommendation, stop and escalate with the new findings. +- **Scope:** If the ADR grows beyond 15 pages (excluding appendices) or requires + more than 2 hours of writing, stop and escalate. +- **Conflicts:** If the decision conflicts with existing ADRs or documented + constraints elsewhere in the codebase, document the conflict and escalate. + +## Risks + +Known uncertainties that might affect the plan. Identify these upfront and +update as work proceeds. + +- **Risk:** Stakeholders disagree on whether arbitrary expressions or named + weights is right for v1. + Severity: high + Likelihood: medium + Mitigation: The research surveyed 12+ production tools and found zero that + support arbitrary expressions in config files; all use named weights or + presets. Include this finding prominently in the ADR so the decision is + grounded in empirical data, not opinion. + +- **Risk:** The recommended option (named weights with v1.1 expression + architecture designed now) is perceived as "half-baked" or inadequate. + Severity: medium + Likelihood: medium + Mitigation: Structure the ADR to show v1.1 architecture reserved space + clearly, with a concrete example of how v1.0 profiles will remain valid + when expressions are added. Demonstrate that v1.0 is complete and useful on + its own, not a placeholder. + +- **Risk:** Segmenter architecture is already partially specified or in flight, + creating path dependencies that conflict with this decision. + Severity: medium + Likelihood: low + Mitigation: Before finalizing the ADR, confirm no segmenter design work has + started. If it has, review its assumptions and update them if they conflict + with the decision. + +- **Risk:** The profile schema is too permissive or too restrictive, causing + Phase 2 to discover issues that require breaking changes. + Severity: medium + Likelihood: low + Mitigation: The ADR must include a concrete example profile showing how + common rule-weighting scenarios are expressed. Phase 2 planning should + review this example and flag any gaps before implementation starts. + +## Progress + +Use a list with checkboxes to summarise granular steps. Every stopping point +must be documented here, even if it requires splitting a partially completed +task into two. + +- [x] (2026-06-18 02:07Z) Agent team research on open-source rule-expression + policies (RESEARCH AGENT 1). +- [x] (2026-06-18 02:07Z) Agent team designs decision framework with three + options and recommendation (RESEARCH AGENT 2). +- [ ] (2026-06-18 TBD) Draft ADR 0003 based on research findings. +- [ ] (2026-06-18 TBD) Validate ADR format against existing ADRs. +- [ ] (2026-06-18 TBD) Validate profile schema example compiles mentally with + Phase 2 requirements. +- [ ] (2026-06-18 TBD) Team review and approval of ADR. +- [ ] (2026-06-18 TBD) Rename branch to + `1-1-4-record-the-profile-rule-expression-policy`. +- [ ] (2026-06-18 TBD) Create draft PR with execplan and ADR. +- [ ] (2026-06-18 TBD) Post PR and await approval before closing DRAFT status. + +## Surprises & discoveries + +Unexpected findings during planning and research that were not anticipated as +risks. + +- **Discovery:** Zero major open-source linters, formatters, or static analysis + tools support arbitrary custom rule expressions in configuration files. All + examined tools (ESLint, Prettier, Pylint, Rustfmt, Biome, Clippy, + Golangci-lint, Stylelint, Flake8, SonarQube, Checkstyle) explicitly reject + arbitrary expressions. This provides very strong empirical support for + choosing named weights over expressions. + +- **Discovery:** The recommended approach ("named weights + v1.1 architecture + reserved") is already in use by multiple production tools (ESLint, Biome, + Clippy) where v1.0 used named weights and v1.1+ added presets or plugins + without breaking v1.0 configs. This is not a novel approach; it is a proven + pattern. + +## Decision log + +Record every significant decision made while designing the plan. + +- **Decision:** Use Option B-Extended (named rule weights for v1.0, with v1.1 + expression architecture designed during v1.0) as the recommended profile + rule-expression policy. + Rationale: Open-source research shows no major tool supports arbitrary + expressions in config (security, performance, auditability constraints). + Named weights are proven in ESLint, Biome, Clippy. This approach unblocks + Phase 2 immediately while reserving space for expressions in v1.1 without + breaking v1.0 profiles. Lowest risk, highest value for v1.0. + Date/Author: 2026-06-18, Agent Research Team. + +- **Decision:** Research scope includes 12+ open-source tools with mature + config systems, focusing on patterns and rationales, not just feature + coverage. + Rationale: We need to understand *why* tools chose their policies, not just + *what* they chose. The why informs whether a decision is replicable in + Prosidy Darn. + Date/Author: 2026-06-18, Agent Research Team. + +## Outcomes & retrospective + +Summary of outcomes and lessons learned. To be updated after completion. + +(Pending: to be completed after ADR draft and team review.) + +## Context and orientation + +**Project:** Prosidy Darn, a text processing and analysis tool with a +plugin-based rule engine. + +**Phase 1 (Current):** Foundational contracts and build spine—establish v1 +architectural decisions before feature work. + +**This Task (1.1.4):** Record the profile rule-expression policy decision. +Profiles are configuration files that users create to customize which rules +apply and how they are parameterized. The decision locked by this task: do +profiles allow arbitrary expressions (e.g., `min_length > 10 && +severity == "error"`), or only named weights (e.g., `severity: high, +min_length: 10`)? + +**Related work:** + +- 1.0.1: Hexagonal architecture design (prerequisite; likely complete) +- 1.1.2: Package boundary definition (prerequisite; likely complete) +- Blocked by 1.1.4: 1.2.1 and later Phase 2 work (implementation of the + segmenter, rule engine, profile loader) + +**Key stakeholders:** + +- Architecture: needs locked schema to design segmenter +- Phase 2 team: needs locked policy to select dependencies and design rule + representation +- Users: will depend on profile schema stability across v1 patches + +**Key files:** + +- This execplan: + `docs/execplans/1-1-4-record-the-profile-rule-expression-policy.md` +- ADR to be written: `docs/adr/0003-profile-rule-expression-policy.md` (new) + +## Plan of work + +### Stage 1: Validate Research and Synthesize Recommendation (no code changes) + +The agent team has completed research into 12+ open-source tools and designed +a decision framework with three options: + +- **Option A:** Arbitrary expressions in profiles (requires new + expression-language design decision). +- **Option B:** Named rule weights only (no expressions, simple schema). +- **Option B-Extended (recommended):** Named weights for v1.0, with v1.1 + architecture designed in v1.0 to add expressions later without breaking v1.0 + profiles. + +First, validate the research findings against the following criteria: + +1. The pattern analysis is accurate (spot-check 3-4 tools from the research + against their official documentation). +2. The recommendation aligns with Prosidy Darn's constraints (does not assume + an expression language; does not require Phase 2 to make new decisions). +3. The v1.1 architecture sketch is coherent (v1.0 profiles remain valid when + expressions are added). +4. The scope is bounded (no blockers for Phase 2 implementation). + +### Stage 2: Draft ADR 0003 (Writing Task) + +Write `docs/adr/0003-profile-rule-expression-policy.md` using the format +established by ADRs 0001 and 0002. The ADR must contain: + +- **Status:** Proposed (not Accepted until team approves). +- **Decision:** Named rule weights for v1.0. Profiles consist of rule names, + severity levels, and named parameters. Arbitrary expressions are not + supported. v1.1 may add an optional expression-based rule type, designed in + v1.0 to ensure backward compatibility. +- **Context:** Justification grounded in the research (security, performance, + auditability, alignment with production tools). +- **Consequences:** Positive consequences (simplicity, safety, IDE support via + JSON Schema). Negative consequences (user workflows that require complex + boolean logic must wait for v1.1). Neutral consequences (migration path from + v1.0 to v1.1 is clear). +- **v1.1 Architecture Sketch (Appendix A):** Concrete example showing how a + v1.0 profile remains valid when v1.1 adds expressions. +- **Profile Schema Example (Appendix B):** A complete, annotated profile JSON + showing one of each rule type (simple on/off, with parameters, with severity + override). Schema must be compatible with Phase 2 segmenter implementation. +- **Research Summary (Appendix C):** One-paragraph summary of the open-source + research findings and why no major tool supports arbitrary expressions. + +### Stage 3: Validation (Reading and Spot-Check) + +Before finalizing, validate the ADR against: + +1. **Format check:** Paragraph structure, heading hierarchy, and style match + `docs/adr/0001-*.md` and `docs/adr/0002-*.md`. +2. **Clarity check:** A reader unfamiliar with Prosidy Darn can understand the + decision, rationale, and v1.1 path without external context. +3. **Completeness check:** The profile schema example is sufficient for Phase 2 + to begin segmenter design without asking clarifying questions. +4. **Conflict check:** The decision does not contradict any existing ADRs or + documented Phase 1 design constraints. + +### Stage 4: Team Review and Approval + +Present the ADR to stakeholders (architecture, Phase 2 leads, decision makers). +Gather feedback and update the ADR as needed. The approval gate is: "The team +agrees the decision is locked and suitable for Phase 2 implementation." + +Once approved, the status changes from DRAFT to APPROVED, and this execplan +proceeds to the final steps. + +### Stage 5: Finalize and Prepare PR + +Once approved: + +1. Rename the current branch from + `10-2-5-playbook-variant-compiles-under-pedantic-lint-profile` to + `1-1-4-record-the-profile-rule-expression-policy`. +2. Stage the execplan and ADR for commit. +3. Create a draft PR with the following: + - PR title: `(1.1.4) Record the profile rule-expression policy` + - PR summary: mention the execplan document and note that this is a + decision-documentation task (no code changes). + - Body section: "## References" with a link to the lody session. +4. Push to `origin/1-1-4-record-the-profile-rule-expression-policy` and update + the branch tracking. + +## Concrete steps + +The following are the exact steps to execute, with expected transcripts where +applicable. + +### Step 1: Validate Research (Stage 1) + +Read the agent research summaries to spot-check the recommendation. Spot-check +3-4 tools against their official docs. For example: + +- ESLint: confirm it uses "rules" as a map of rule names to severity levels + and config objects (not arbitrary expressions in rule config). +- Biome: confirm it uses severity + presets, not arbitrary expressions. +- Pylint: confirm it uses rule names and parameter values, not arbitrary + expressions in the config file. + +Record findings in the `Surprises & Discoveries` section if any contradictions +emerge. + +### Step 2: Draft ADR 0003 (Stage 2) + +Create `docs/adr/0003-profile-rule-expression-policy.md`. Use the following +outline and structure it to match ADRs 0001 and 0002: + +```markdown +# ADR 0003: Profile Rule-Expression Policy + +**Status:** Proposed +**Date:** 2026-06-18 + +## Context + +[2-3 paragraphs explaining why this decision matters now, what triggered the +need, how it affects Phase 2] + +## Decision + +[1 paragraph: named rule weights for v1.0; no arbitrary expressions; v1.1 may +add expressions without breaking v1.0] + +## Rationale + +[3-4 paragraphs grounded in the research: why arbitrary expressions are +rejected, why named weights align with production tools, why the v1.1 path is +designed now, what constraints guide the choice] + +## Consequences + +### Positive +- Security: no code execution in profiles +- Performance: O(n) parsing, no evaluation overhead +- Auditability: profiles are human-readable and reviewable +- IDE Support: schema in JSON Schema for autocomplete +- Alignment: follows ESLint, Biome, Clippy pattern + +### Negative +- Extensibility: users cannot write custom boolean logic in profiles before + v1.1 +- Learning curve: users must learn named rule parameters instead of writing + expressions + +### Neutral +- Migration: v1.0 profiles will be valid in v1.1 without changes +- Implementation: segmenter design is not complex with named weights + +## v1.1 Architecture Sketch (Appendix A) + +[Example showing how v1.0 profile structure maps to v1.1 with optional +expressions] + +## Profile Schema Example (Appendix B) + +[JSON example with annotations showing rule names, severity, parameters] + +## Research Findings (Appendix C) + +[One paragraph summary of open-source research] +``` + +Expected output: A file at `docs/adr/0003-profile-rule-expression-policy.md` +(estimated 6-8 pages, ~2,000 words). + +### Step 3: Validation (Stage 3) + +After drafting, validate the ADR: + +```bash +# Lint Markdown (if a linter is available) +markdownlint docs/adr/0003-profile-rule-expression-policy.md || true +``` + +Update the ADR if any issues emerge. + +### Step 4: Team Review and Approval (Stage 4) + +Present the ADR to stakeholders and gather approval. This is a manual gate. +Update the ADR's Status field to "Accepted" once approval is given. + +### Step 5: Finalize and Prepare PR (Stage 5) + +Once approved: + +```bash +# Rename the branch +git branch -m 1-1-4-record-the-profile-rule-expression-policy + +# Stage the execplan and ADR +git add docs/execplans/1-1-4-record-the-profile-rule-expression-policy.md +git add docs/adr/0003-profile-rule-expression-policy.md + +# Commit with gated checks +make check-fmt +make lint +make typecheck +git commit -m "Add ADR 0003 and execplan for profile rule-expression policy" + +# Create draft PR +gh pr create --draft \ + --title "(1.1.4) Record the profile rule-expression policy" \ + --body "## Summary +Finalize the v1.0 profile rule-expression policy. + +- Decision: Named rule weights for v1.0; no arbitrary expressions. +- Rationale: Security, performance, alignment with production tools. +- Future path: v1.1 architecture reserved for expressions without breaking. +- Deliverables: ADR 0003 and this execplan. + +## References +- Execplan: docs/execplans/1-1-4-record-the-profile-rule-expression-policy.md +- Session: https://lody.ai/leynos/sessions/\${LODY_SESSION_ID} +" + +# Track the remote branch +git push -u origin 1-1-4-record-the-profile-rule-expression-policy +``` + +Expected output: + +```plaintext +Draft pull request # created: https://github.com///pull/ +Branch 1-1-4-record-the-profile-rule-expression-policy set up to track +origin/1-1-4-record-the-profile-rule-expression-policy. +``` + +## Validation and acceptance + +### Quality criteria + +1. **ADR completeness:** The ADR must include all sections (Context, Decision, + Rationale, Consequences, Appendices). No section may be empty or + placeholder. +2. **Format:** The ADR must follow the format of ADRs 0001 and 0002 (heading + hierarchy, paragraph structure, style). +3. **Clarity:** A reader unfamiliar with Prosidy Darn must be able to + understand the decision, rationale, and v1.1 path without external context. +4. **Profile schema:** The schema example must be complete enough for Phase 2 + to design the segmenter without clarifying questions. +5. **No code changes:** The execplan and ADR are documentation only. No changes + to source code, tests, or build configuration. +6. **Team approval:** Stakeholders must approve the decision before closing + DRAFT status. + +### Quality method + +```bash +# After completing all stages: +make check-fmt +make lint +git log --oneline -5 +gh pr view --json status +``` + +Expected output: + +- All formatting and lint checks pass. +- Commits are clean (no unintended files staged). +- Draft PR exists and is visible. + +### Acceptance narrative + +After approval and the draft PR is created, the task is complete. The decision +is locked in the ADR. Phase 2 can now begin implementation with confidence that +the profile schema is stable. The execplan serves as a record of how the +decision was made and what was considered. + +The successful outcome is: "Phase 2 architecture and segmenter design begins +without needing clarification on whether profiles support arbitrary +expressions." + +## Idempotence and recovery + +All steps are idempotent: + +- Drafting the ADR can be repeated; overwrite the file and re-validate. +- Validation steps use read-only checks; no side effects. +- Team review can be repeated; just update the ADR and re-present. +- Branch rename and PR creation are one-time operations, but if they fail, + delete the branch/PR and retry. + +If a branch rename fails: + +```bash +# Delete locally +git branch -d 1-1-4-record-the-profile-rule-expression-policy +# Delete remote +git push origin --delete 1-1-4-record-the-profile-rule-expression-policy +# Rename again +git branch -m 1-1-4-record-the-profile-rule-expression-policy +``` + +## Artifacts and notes + +Key artifacts produced: + +1. **ADR 0003:** `docs/adr/0003-profile-rule-expression-policy.md` + (~2,000 words, 6-8 pages). +2. **Execplan (this document):** + `docs/execplans/1-1-4-record-the-profile-rule-expression-policy.md` + (living document). +3. **Draft PR:** Created on GitHub with title + "(1.1.4) Record the profile rule-expression policy". + +## Interfaces and dependencies + +### Profile Schema (Target Interface for v1.0) + +The ADR must define a profile schema suitable for the segmenter to consume. +Here is the expected structure (to be detailed in the ADR): + +```json +{ + "version": "1.0", + "rules": { + "identifier-length": { + "enabled": true, + "severity": "error", + "minLength": 3, + "maxLength": 100 + }, + "line-length": { + "enabled": false + }, + "no-trailing-whitespace": { + "enabled": true, + "severity": "warning" + } + } +} +``` + +This schema is the contract between the profile loader and the segmenter. +Changing it after v1.0 release requires a major version bump. + +### v1.1 Extension (Reserved, Not Implemented) + +The ADR must sketch how v1.1 would extend this schema to support expressions +without breaking v1.0: + +```json +{ + "version": "1.0 or 1.1", + "rules": { + "identifier-length": { + "enabled": true, + "severity": "error", + "minLength": 3 + }, + "custom-rule-expr": { + "enabled": true, + "expression": "length(identifier) > ${minLength}", + "severity": "error", + "params": { "minLength": 3 } + } + } +} +``` + +The segmenter in v1.0 ignores unknown rule types (like `custom-rule-expr`) so +v1.1 profiles work in v1.0 (with the expression rule simply not evaluated). +This forward compatibility is the key to the reversible design. + +--- + +**Revision note:** Initial draft created 2026-06-18 based on agent research. +Sections will be updated as drafting and team review proceed. diff --git a/docs/execplans/3-3-3-replace-shared-tui-references-with-host-neutral-references.md b/docs/execplans/3-3-3-replace-shared-tui-references-with-host-neutral-references.md new file mode 100644 index 00000000..594d1cb5 --- /dev/null +++ b/docs/execplans/3-3-3-replace-shared-tui-references-with-host-neutral-references.md @@ -0,0 +1,798 @@ +# 3.3.3 Replace Shared TUI References with Host-Neutral Review View References + +This ExecPlan is a living document. The sections `Constraints`, `Tolerances`, +`Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, and `Outcomes & +Retrospective` must be kept up to date as work proceeds. + +Status: DRAFT + + +## Purpose / big picture + +This milestone introduces a host-neutral abstraction for review view references, +decoupling TUI-specific implementation details from domain models that represent +code review summaries and comments. Currently, review linking is tightly coupled +to the TUI presentation layer, making it impossible for other clients (CLI tools, +libraries, web services) to reference review comments without importing +TUI-specific types and protocols. + +After this change, code review summary data transfer objects (DTOs) will expose +host-neutral `ReviewViewRef` values that reference review comments without +encoding TUI-specific URLs or view implementations. The TUI adapter will render +these references as `frankie://review-comment/?view=detail` links, and CLI +and library tests will prove that serialization does not depend on TUI-only +types, enabling other delivery mechanisms (CLI commands, web APIs, library +consumers) to resolve references independently using host-specific adapters. + +Users will be able to export, serialize, and consume review data in CLI tools +and library code without runtime or static dependencies on the TUI layer. + + +## Glossary + +**Host-neutral reference**: A serializable, domain-owned value object that +identifies a review comment without encoding any host-specific presentation +details (e.g., TUI URLs, handler functions, view technologies). It can be +safely serialized to JSON and deserialized in contexts with no TUI dependencies. + +**Host**: The runtime environment consuming review data. Includes TUI (terminal +user interface), CLI commands, web service APIs, and library consumers. + +**ReviewViewRef**: The domain type (dataclass) that represents a host-neutral +reference to a review comment. Contains only logical identifiers (comment ID, +view type), never host-specific details. + +**Summary DTO**: Data transfer object representing aggregated review comment +data, including the comment text, metadata, and a `ReviewViewRef` for linking. +Does not import TUI-specific modules. + +**View resolver**: An adapter interface that translates a `ReviewViewRef` into +a host-specific URL or reference string (e.g., `frankie://` for TUI, `cli://` +for CLI, raw ID for library). One resolver implementation per host. + +**frankie://**: The TUI-specific URL scheme used by the Terminal User Interface +to render review comment links. Example: `frankie://review-comment/abc123?view=detail`. + + +## Constraints + +Hard invariants that must hold throughout implementation. + +- New public APIs introduced for review data export must remain stable after + release. Internal APIs (non-exported functions) may be refactored if necessary. +- The TUI must continue to work without modification after this change; the + adapter layer must translate host-neutral references to `frankie://` URLs. +- No new external dependencies without explicit approval. Review models and + resolvers must use only Python standard library (dataclasses, abc, typing). +- All code must pass type checking with the project's type checker configuration. +- Tests must achieve ≥85% line coverage for new modules; domain model files + (`models.py`, `resolution.py`) must have 100% coverage to ensure no accidental + TUI imports. +- The domain model for `ReviewViewRef` and view resolution must not import from + TUI-specific modules. This is a hard architectural boundary enforced by static + and runtime checks. +- ReviewViewRef serialization must use JSON format and be JSON-safe + (strings, numbers, booleans, dicts, lists only). Deserialization must handle + schema evolution gracefully (ignore unknown fields, validate required fields). + + +## Tolerances (exception triggers) + +Thresholds that trigger escalation when breached. + +- Scope: if implementation requires changes to more than 8 files or more than 800 net lines of code, stop and escalate. +- Interface changes: if any public function signature in an existing module must change, stop and escalate unless the change is a non-breaking addition. +- Dependencies: if any new external package is required, stop and escalate. +- Test coverage: if any new module falls below 85% coverage, do not commit without escalating. +- Iterations: if a single test fails more than 3 consecutive attempts to fix, stop and escalate. +- Design ambiguity: if a choice between two equally valid approaches materially affects the outcome, stop and present options for approval. + + +## Risks + +Known uncertainties that might affect the plan. + +- Risk: Unclear specification of what "host-neutral" means in the context of review link resolution. + Severity: medium + Likelihood: medium + Mitigation: Define host-neutral as "no imports of TUI-specific modules in domain models; view references must be serializable and discoverable by non-TUI code". Prototype a small resolver to confirm this is achievable. + +- Risk: Existing code in review-related modules may already have tight coupling to TUI types that is difficult to untangle without broader refactoring. + Severity: medium + Likelihood: medium + Mitigation: Start with a codebase exploration to measure coupling; if coupling is severe, propose a two-phase approach: (1) create new domain models and tests, (2) refactor adapters gradually. Escalate if more than 3 files would need changes to untangle existing coupling. + +- Risk: Prerequisite tasks (2.1.3 and 3.3.2) may introduce design conflicts or incomplete contracts that affect this task. + Severity: high + Likelihood: low + Mitigation: This plan assumes no review data structures or contracts exist yet in the expected shape. Phase 1 includes a pre-flight check to confirm prerequisites have not been implemented. If 2.1.3 or 3.3.2 are implemented concurrently, suspend this task and merge work at a common base. + +- Risk: CLI and library tests may reveal that the view reference abstraction is too loose or too strict to be useful for non-TUI consumers. + Severity: low + Likelihood: medium + Mitigation: Design tests that exercise view reference resolution from both TUI and non-TUI contexts (e.g., mock CLI and library contexts). Use test-driven design to refine the abstraction as needed. + +- Risk: Python type checker (type annotation requirements) may flag dataclass usage or missing Protocol definitions. + Severity: low + Likelihood: medium + Mitigation: Use frozen dataclasses with slots for domain models. Test with the project's type checker after Phase 2. If type checking fails, define Protocol interfaces for critical types. + +- Risk: Domain models living in `workflow_scripts/` alongside procedural scripts may cause namespace confusion. + Severity: low + Likelihood: low + Mitigation: Phase 1 evaluation may propose moving domain models to `src/review_views/` or `workflow_scripts/lib/review_views/`. Document the final location choice in Decision Log. + +- Risk: Coverage measurement tool (slipcover vs coverage.py) may produce different results than the specified pytest --cov flag. + Severity: low + Likelihood: low + Mitigation: Use `make test` (the canonical test command) which manages slipcover setup. Validate coverage with the project's standard tooling, not manual pytest --cov. + + +## Progress + +Use a list with checkboxes to summarise granular steps with timestamps. + +- [ ] **Phase 1: Design and Specification** + - [ ] (TBD) Pre-flight check: Confirm prerequisites 2.1.3 and 3.3.2 have not been implemented. If they have, STOP and rebase onto prerequisite work. + - [ ] (TBD) Clarify "host-neutral" definition and inventory existing TUI-specific modules in the codebase. + - [ ] (TBD) Determine where the TUI adapter layer should live (e.g., `src/tui/adapters/`, `.github/actions/*/src/tui/`) and confirm `workflow_scripts/review_views/` is the right home for domain models. + - [ ] (TBD) Create ADR 008: PR Discussion Summary Contract (defines summary DTO schema and ReviewViewRef structure). + - [ ] (TBD) Create ADR 010: Review Adapter Capability Gap (explains view resolver pattern, adapter responsibilities, and TUI/domain boundary). + - [ ] (TBD) Design the view resolver interface, including resolver semantics (what each host returns for a given ReviewViewRef). + - [ ] (TBD) Design reference equality and round-trip serialization strategy. + - [ ] (TBD) Obtain design approval before proceeding to Phase 2. + +- [ ] **Phase 2: Domain Model Implementation (Red-Green-Refactor)** + - [ ] (TBD) Write failing tests for `ReviewViewRef` serialization, deserialization, error handling, and edge cases (missing fields, round-trip, JSON safety, boundary isolation). + - [ ] (TBD) Implement `ReviewViewRef` dataclass in `workflow_scripts/review_views/models.py` (frozen, with slots, JSON-safe fields only). + - [ ] (TBD) Implement view resolver interface in `workflow_scripts/review_views/resolution.py`. + - [ ] (TBD) Add `workflow_scripts/review_views/errors.py` with custom exception types. + - [ ] (TBD) Verify `models.py` and `resolution.py` have 100% coverage; verify no TUI imports appear in domain modules. + - [ ] (TBD) Add static import verification test (enforce domain layer cannot import from TUI-specific modules). + +- [ ] **Phase 3: Summary DTO Refactoring (Red-Green-Refactor)** + - [ ] (TBD) Phase 1 output determines: does a summary DTO exist? If yes, refactor in place; if no, create `workflow_scripts/review_views/summary.py`. + - [ ] (TBD) Write failing tests that verify summary DTOs do not import TUI types and expose `ReviewViewRef` instead of TUI-specific link fields (type hint constraint test, no-tui-import test, serialization test). + - [ ] (TBD) Implement/refactor summary DTO to include `ReviewViewRef` field; remove or deprecate TUI-specific fields (no `frankie_url`, `tui_handler`, etc. in domain layer). + - [ ] (TBD) Enumerate all existing consumers of the old summary DTO format and update them to use `ReviewViewRef`. + - [ ] (TBD) Verify summary DTO module has ≥85% coverage and 100% coverage for no-TUI-import constraints. + +- [ ] **Phase 4: TUI Adapter Layer and Registry** + - [ ] (TBD) Phase 1 output determines: where should the TUI adapter live? (e.g., `src/tui/adapters/review_views.py`, `.github/actions/*/src/adapters/`, etc.). Document location in Decision Log. + - [ ] (TBD) Implement ViewResolver registry in `workflow_scripts/review_views/adapters/__init__.py` to allow stateless, host-agnostic resolver discovery (TUI code registers its resolver at startup). + - [ ] (TBD) Implement TUI adapter that translates `ReviewViewRef` to `frankie://` URLs (may live in TUI-specific module, not in domain layer). + - [ ] (TBD) Write integration tests that verify TUI adapter correctly renders `frankie://` links from summary data. + - [ ] (TBD) Verify no TUI types appear in domain layer (both via runtime sys.modules check and via boundary isolation test). + +- [ ] **Phase 5: CLI and Library Testing (Non-TUI Contexts)** + - [ ] (TBD) Write tests demonstrating that summary DTOs serialize/deserialize to JSON without TUI imports (`test_library_review_context.py`). + - [ ] (TBD) Implement mock CLI resolver and write CLI integration test showing a CLI tool can consume review data and resolve view references without TUI dependency (`test_cli_review_context.py`). + - [ ] (TBD) Write library example code showing how to export review data, serialize it, and consume it in a context with zero TUI imports. + - [ ] (TBD) Verify both CLI and library tests pass with ≥85% coverage. + +- [ ] **Phase 6: Documentation and Quality Gates (Local)** + - [ ] (TBD) Update `docs/developers-guide.md` with a new "View Resolution Pattern" section: explain how domain models expose view references, how adapters translate them per-host, include code example showing the three layers, and add ASCII dependency diagram. + - [ ] (TBD) Update `docs/users-guide.md` with observable behavior (how users will interact with view references in CLI and library contexts). + - [ ] (TBD) Run `make check-fmt`, `make lint`, `make test` and verify all pass with no failures. + - [ ] (TBD) Verify coverage: use project's canonical tooling (likely `make test` with slipcover) and confirm ≥85% for all new modules, 100% for domain models. + +- [ ] **Phase 7: Integration and Edge Cases** + - [ ] (TBD) Write tests for view reference resolution edge cases (missing references, ambiguous IDs, invalid hosts). + - [ ] (TBD) Integration test with real TUI: verify that TUI can still render review comments and links after changes. + - [ ] (TBD) Verify backward compatibility: if existing code exports review data, verify it still works. + +- [ ] **Phase 8: Final Quality Verification (Local)** + - [ ] (TBD) Run full test suite: `make test` and verify all pass. + - [ ] (TBD) Run `make check-fmt` and `make lint` and verify no violations. + - [ ] (TBD) Verify coverage: ≥85% for all new modules, 100% for domain models. + - [ ] (TBD) Confirm all constraints and tolerances have been met. + +- [ ] **Phase 9: Code Review and Completion** + - [ ] (TBD) Rename branch to `3-3-3-replace-shared-tui-references-with-host-neutral-references` and push to remote. + - [ ] (TBD) Create draft PR with execplan summary, ADR 008/010 links, and lody session reference. + - [ ] (TBD) Request CodeRabbit review: `coderabbit review --agent`. + - [ ] (TBD) Resolve all CodeRabbit concerns and mark PR ready for review. + - [ ] (TBD) Mark task 3.3.3 as "done" in roadmap (if roadmap exists). + + +## Surprises & discoveries + +Unexpected findings during implementation that were not anticipated as risks. + +- Observation: (placeholder) + Evidence: (to be filled as work progresses) + Impact: (to be filled as work progresses) + + +## Decision log + +Record every significant decision made while working on the plan. + +- Decision: Use Python dataclasses for `ReviewViewRef` and related DTOs (following project convention). + Rationale: The project already uses dataclasses extensively (e.g., `stage_common/` pattern). This ensures consistency and type safety with mypy. + Date/Author: 2026-06-18 Claude (planning phase) + +- Decision: Place domain models in new `workflow_scripts/review_views/` submodule, following the `stage_common/` pattern. + Rationale: The codebase already separates domain logic into dedicated submodules. This pattern is proven in the monorepo and keeps review-specific code isolated and testable. + Date/Author: 2026-06-18 Claude (planning phase) + +- Decision: Assume prerequisites (2.1.3 and 3.3.2) do not yet exist and plan for greenfield implementation of review data structures. + Rationale: Codebase exploration found no existing review domain models or TUI-specific linking code. If prerequisites are implemented later, this plan can be adapted. + Date/Author: 2026-06-18 Claude (planning phase) + +- Decision: (placeholder—to be filled during implementation) + Rationale: (to be filled) + Date/Author: TBD + + +## Outcomes & retrospective + +Summarize outcomes, gaps, and lessons learned at major milestones or at completion. + +- Placeholder: to be completed at end of work. + + +## Context and orientation + +This project is a GitHub Actions monorepo written in Python, containing 14+ actions and shared utilities. The codebase demonstrates clear separation of concerns: presentation/CLI code at the `src/` level, domain logic in submodules like `stage_common/`, and infrastructure concerns isolated in adapters. + +The project uses: +- **Python 3.13+** with strong type checking (mypy). +- **pytest** for testing with ≥80% branch coverage requirement. +- **dataclasses** for domain modeling. +- **Modular architecture** with explicit submodule boundaries (e.g., `stage_common/config.py`, `stage_common/pipeline.py`). + +The task requires decoupling TUI-specific types from review data models. Currently, no `TuiViewLink`, `ReviewViewRef`, or review linking code exists in the codebase (confirmed via codebase exploration). This is greenfield work to introduce new abstractions that enable CLI and library code to consume review data without TUI-specific imports. + +Key directories: +- `workflow_scripts/`: Where domain logic and data structures live (e.g., `stage-release-artefacts/stage_common/`). +- `workflow_scripts/tests/`: Where unit tests are located. +- `docs/adr/`: Where architecture decision records are stored. +- `docs/developers-guide.md`: Where internal patterns and conventions are documented. +- `docs/users-guide.md`: Where user-visible behavior is documented. + + +## Plan of work + +The implementation proceeds in nine stages, each with clear validation and go/no-go criteria. Stages 1–3 are exploratory and design-focused; stages 4–8 are implementation and testing; stage 9 is completion. + +**Stage 1: Design and Specification** answers the question "What is a host-neutral review view reference and how does it work?" This stage produces ADR 008 (summary contract) and ADR 010 (adapter gap). No code is written yet. Output: two ADRs and a decision on the `ReviewViewRef` abstraction and resolver interface. Validation: design review approval and clarity on the resolver protocol. + +**Stage 2: Domain Model Implementation** introduces the core `ReviewViewRef` dataclass and a view resolver interface. Using Red-Green-Refactor, failing tests are written first to specify serialization and deserialization behavior. The `ReviewViewRef` must be serializable (JSON-safe) and must not include TUI-specific URLs or imports. Output: `workflow_scripts/review_views/models.py` with `ReviewViewRef` dataclass and `workflow_scripts/review_views/resolution.py` with resolver interface. Validation: tests pass, coverage ≥85%, no TUI imports in domain models. + +**Stage 3: Summary DTO Refactoring** updates review summary data structures to use `ReviewViewRef` instead of TUI-specific link types. Using Red-Green-Refactor, failing tests verify that summary DTOs do not import from TUI modules and expose `ReviewViewRef`. Existing consumers are updated to use the new abstraction. Output: updated summary DTO (or new file `workflow_scripts/review_views/summary.py`). Validation: tests pass, coverage ≥85%, no TUI imports visible in the domain model. + +**Stage 4: TUI Adapter Layer** implements a TUI-specific adapter that translates `ReviewViewRef` to `frankie://` URLs. This adapter lives in the TUI layer and can import TUI types (the boundary is one-directional: domain → TUI adapter, never TUI → domain). Output: `workflow_scripts/review_views/adapters/tui_adapter.py` with translation logic. Validation: integration tests verify that TUI can render correct URLs from review data. + +**Stage 5: CLI and Library Testing** writes end-to-end tests demonstrating that summary DTOs can be serialized and used in non-TUI contexts (e.g., a CLI tool or library that has no TUI dependencies). Output: test cases in `workflow_scripts/tests/test_review_serialization.py` and `workflow_scripts/tests/test_cli_review_context.py`. Validation: tests pass, serialization round-trip works without TUI imports. + +**Stage 6: Documentation** updates developer and user guides to explain the view resolver pattern and the new host-neutral reference design. Output: updates to `docs/developers-guide.md` and `docs/users-guide.md`. Validation: quality gates pass (fmt, lint, test). + +**Stage 7: Integration and Edge Cases** covers error handling, missing references, and backward compatibility. Output: comprehensive edge-case tests. Validation: all tests pass, coverage remains ≥85%. + +**Stage 8: Final Quality Verification** ensures the complete implementation meets all acceptance criteria. Output: passing tests, code review approval. Validation: make check-fmt, make lint, make test all pass; CodeRabbit review resolves all concerns. + +**Stage 9: Completion** marks the task as done in the roadmap, renames the branch, and creates a PR. Output: draft PR with execplan summary and lody session link. + + +## Concrete steps + +1. **Phase 1 exploration** (before any code changes): Run the following commands to understand the current state and confirm no review linking code exists: + +```bash +cd /tmp/lody-title-agent +grep -r "TuiViewLink\|ReviewViewRef\|frankie" --include="*.py" . 2>/dev/null | grep -v ".venv" | head -20 +``` + +Expected: No results or only results from venv or unrelated files. + +```bash +find . -path "./.venv" -prune -o -name "*review*.py" -type f -print | grep -v ".venv" +``` + +Expected: List of any existing review-related files (may be empty). + +2. **Phase 2 (Red-Green-Refactor)** starts with failing tests. Create `workflow_scripts/tests/test_review_views.py`: + +```python +import pytest +import json +import sys +from dataclasses import dataclass +from workflow_scripts.review_views.models import ReviewViewRef + +# Red tests: Serialization, deserialization, error handling, edge cases +def test_review_view_ref_serialization(): + """ReviewViewRef must be serializable without TUI imports.""" + ref = ReviewViewRef(comment_id="abc123", view_type="detail") + data = ref.to_dict() + assert "comment_id" in data + assert "view_type" in data + assert "frankie_url" not in data # No TUI-specific fields + +def test_review_view_ref_deserialization(): + """ReviewViewRef must deserialize from dict without TUI imports.""" + data = {"comment_id": "abc123", "view_type": "detail"} + ref = ReviewViewRef.from_dict(data) + assert ref.comment_id == "abc123" + assert ref.view_type == "detail" + +def test_review_view_ref_deserialization_missing_comment_id(): + """from_dict raises KeyError if comment_id is missing.""" + data = {"view_type": "detail"} + with pytest.raises(KeyError): + ReviewViewRef.from_dict(data) + +def test_review_view_ref_serialization_roundtrip(): + """Serialization and deserialization are inverses.""" + original = ReviewViewRef(comment_id="xyz789", view_type="summary") + dict_form = original.to_dict() + restored = ReviewViewRef.from_dict(dict_form) + assert restored == original + +def test_review_view_ref_json_safe(): + """ReviewViewRef must be JSON-serializable.""" + ref = ReviewViewRef(comment_id="abc123", view_type="detail") + json_str = json.dumps(ref.to_dict()) # Must not raise + data = json.loads(json_str) + restored = ReviewViewRef.from_dict(data) + assert restored == ref + +def test_domain_models_do_not_import_tui(): + """Domain models must have zero TUI imports at runtime.""" + # Record modules before import + modules_before = set(sys.modules.keys()) + + from workflow_scripts.review_views.models import ReviewViewRef + + # Check modules loaded during import + modules_after = set(sys.modules.keys()) + new_modules = modules_after - modules_before + tui_modules = [m for m in new_modules if 'tui' in m.lower() or 'frankie' in m.lower()] + assert not tui_modules, f"Domain imported TUI modules: {tui_modules}" +``` + +Run the tests to confirm they fail: + +```bash +cd /tmp/lody-title-agent +python -m pytest workflow_scripts/tests/test_review_views.py::test_review_view_ref_serialization -xvs +``` + +Expected output: + +```plaintext +FAILED workflow_scripts/tests/test_review_views.py::test_review_view_ref_serialization +ModuleNotFoundError: No module named 'workflow_scripts.review_views' +``` + +3. **Phase 2 (Green)** implements the minimal code to make the test pass. Create `workflow_scripts/review_views/__init__.py` (empty) and `workflow_scripts/review_views/models.py`: + +```python +from dataclasses import dataclass +from typing import Dict, Any + +@dataclass +class ReviewViewRef: + """Host-neutral reference to a review view. No TUI-specific types.""" + comment_id: str + view_type: str # e.g., "detail", "summary" + + def to_dict(self) -> Dict[str, Any]: + """Serialize to dict (JSON-safe).""" + return { + "comment_id": self.comment_id, + "view_type": self.view_type, + } + + @classmethod + def from_dict(cls, data: Dict[str, Any]) -> "ReviewViewRef": + """Deserialize from dict.""" + return cls( + comment_id=data["comment_id"], + view_type=data["view_type"], + ) +``` + +Run the test again: + +```bash +cd /tmp/lody-title-agent +python -m pytest workflow_scripts/tests/test_review_views.py::test_review_view_ref_serialization -xvs +``` + +Expected: test passes. + +4. **Phase 3 verification**: After domain model is implemented, verify no TUI imports appear: + +```bash +cd /tmp/lody-title-agent +python -c "import workflow_scripts.review_views.models; print('Domain model loaded successfully without TUI imports')" +``` + +Expected: successful import with no TUI-related errors. + +5. **Phase 4 (TUI Adapter and Registry)**: Create `workflow_scripts/review_views/adapters/__init__.py`: + +```python +from abc import ABC, abstractmethod +from workflow_scripts.review_views.models import ReviewViewRef + +class ViewResolver(ABC): + """Abstract resolver for translating ReviewViewRef to host-specific URLs.""" + @abstractmethod + def resolve(self, ref: ReviewViewRef) -> str: + """Resolve a ReviewViewRef to a host-specific view URL.""" + pass + +class ViewResolverRegistry: + """Central registry for view resolvers across hosts.""" + _resolvers = {} + + @classmethod + def register(cls, host: str, resolver: ViewResolver) -> None: + cls._resolvers[host] = resolver + + @classmethod + def resolve_for_host(cls, host: str) -> ViewResolver: + if host not in cls._resolvers: + raise ValueError(f"No resolver registered for host: {host}") + return cls._resolvers[host] +``` + +Create `workflow_scripts/review_views/adapters/tui_adapter.py` (in TUI-specific module location, NOT in domain): + +```python +from workflow_scripts.review_views.models import ReviewViewRef +from workflow_scripts.review_views.adapters import ViewResolver + +class TUIViewResolver(ViewResolver): + """Resolves ReviewViewRef to frankie:// URLs for TUI rendering.""" + def resolve(self, ref: ReviewViewRef) -> str: + return f"frankie://review-comment/{ref.comment_id}?view={ref.view_type}" +``` + +Write tests in `workflow_scripts/tests/test_review_views.py`: + +```python +def test_tui_adapter_translation(): + """TUI adapter translates ReviewViewRef to frankie:// URL.""" + from workflow_scripts.review_views.adapters.tui_adapter import TUIViewResolver + ref = ReviewViewRef(comment_id="xyz789", view_type="detail") + resolver = TUIViewResolver() + url = resolver.resolve(ref) + assert url == "frankie://review-comment/xyz789?view=detail" + +def test_cli_resolver_without_tui(): + """Mock CLI resolver works without TUI imports.""" + from workflow_scripts.review_views.adapters import ViewResolver + + class CLIViewResolver(ViewResolver): + def resolve(self, ref: ReviewViewRef) -> str: + return f"cli://view/{ref.comment_id}" + + ref = ReviewViewRef(comment_id="abc123", view_type="detail") + resolver = CLIViewResolver() + url = resolver.resolve(ref) + assert url == "cli://view/abc123" + assert "frankie" not in url # No TUI-specific content + +def test_library_serializes_without_tui(): + """Library code can serialize review data for export without TUI imports.""" + import json + ref = ReviewViewRef(comment_id="xyz789", view_type="summary") + json_str = json.dumps(ref.to_dict()) + data = json.loads(json_str) + restored = ReviewViewRef.from_dict(data) + assert restored == ref + assert "frankie_url" not in json_str + assert "tui_" not in json_str +``` + +Run tests: + +```bash +cd /tmp/lody-title-agent +python -m pytest workflow_scripts/tests/test_review_views.py::test_tui_adapter_translation workflow_scripts/tests/test_review_views.py::test_cli_resolver_without_tui workflow_scripts/tests/test_review_views.py::test_library_serializes_without_tui -xvs +``` + +Expected: all tests pass. + +6. **Quality gates after each phase**: + +```bash +cd /tmp/lody-title-agent +python -m pytest workflow_scripts/tests/test_review_views.py -v --cov=workflow_scripts.review_views --cov-fail-under=85 +``` + +Expected: all tests pass, coverage ≥85%. + +```bash +cd /tmp/lody-title-agent +make lint # or equivalent linting command +make check-fmt +``` + +Expected: no lint or format violations. + +7. **Final validation** after all phases: + +```bash +cd /tmp/lody-title-agent +make test +make lint +make check-fmt +``` + +Expected: all pass. + +## Validation and acceptance + +The feature is complete when all of the following acceptance criteria are satisfied: + +**Phase 1 Pre-Flight Check:** +1. Prerequisites 2.1.3 and 3.3.2 do not exist in the codebase (confirmed via grep or git search). + +```bash +cd /tmp/lody-title-agent +grep -r "ReviewCommentSummary\|PullRequestDiscussionSummary" --include="*.py" workflow_scripts/ || echo "OK: No existing summary contracts found" +``` + +Expected: "OK: No existing summary contracts found" or empty results. + +**Phase 2 Validation: Domain Model is Host-Neutral** +2. `ReviewViewRef` and view resolver interface can be imported without TUI-specific modules: + +```bash +python -c " +import sys +from workflow_scripts.review_views.models import ReviewViewRef +from workflow_scripts.review_views.resolution import ViewResolver +tui_modules = [m for m in sys.modules if 'tui' in m.lower() or 'frankie' in m.lower()] +assert not tui_modules, f'Domain imported TUI modules: {tui_modules}' +print('PASS: Domain models imported without TUI dependencies') +" +``` + +Expected: "PASS: Domain models imported without TUI dependencies" + +3. Serialization is JSON-safe and round-trip preserves data: + +```bash +cd /tmp/lody-title-agent +python -m pytest workflow_scripts/tests/test_review_views.py::test_review_view_ref_serialization workflow_scripts/tests/test_review_views.py::test_review_view_ref_deserialization workflow_scripts/tests/test_review_views.py::test_review_view_ref_serialization_roundtrip workflow_scripts/tests/test_review_views.py::test_review_view_ref_json_safe -v +``` + +Expected: all 4 tests pass. + +4. Deserialization error handling is correct: + +```bash +cd /tmp/lody-title-agent +python -m pytest workflow_scripts/tests/test_review_views.py::test_review_view_ref_deserialization_missing_comment_id -v +``` + +Expected: test passes (raises KeyError on missing fields). + +**Phase 4 Validation: Adapter Boundary is Isolated** +5. Domain models do not import from adapter layer: + +```bash +cd /tmp/lody-title-agent +python -m pytest workflow_scripts/tests/test_review_views.py::test_domain_models_do_not_import_tui -v +``` + +Expected: test passes (zero TUI imports in domain). + +6. Multiple adapters can be implemented and used independently: + +```bash +cd /tmp/lody-title-agent +python -m pytest workflow_scripts/tests/test_review_views.py::test_tui_adapter_translation workflow_scripts/tests/test_review_views.py::test_cli_resolver_without_tui -v +``` + +Expected: both tests pass (TUI and CLI adapters work independently). + +**Phase 5 Validation: CLI and Library Contexts Work** +7. Library code can serialize review data without TUI imports: + +```bash +cd /tmp/lody-title-agent +python -m pytest workflow_scripts/tests/test_review_views.py::test_library_serializes_without_tui -v +``` + +Expected: test passes (JSON serialization works, no TUI-specific fields). + +**Phase 6 Validation: Code Quality** +8. All tests pass with adequate coverage: + +```bash +cd /tmp/lody-title-agent +make test +``` + +Expected: all tests pass. Coverage (using project's canonical tooling, typically slipcover): +- Domain model files (`models.py`, `resolution.py`): ≥100% +- Adapter files: ≥85% +- All other files: ≥85% + +9. Code quality gates pass: + +```bash +cd /tmp/lody-title-agent +make check-fmt && make lint +``` + +Expected: all commands exit with status 0, no format or lint violations. + +10. Documentation is updated: + +- `docs/developers-guide.md` includes a "View Resolution Pattern" section with code example and ASCII dependency diagram. +- `docs/users-guide.md` describes how view references work in CLI and library contexts. + +**Phase 9 Validation: Final PR and Review** +11. PR created with draft status and CodeRabbit review requested: + +```bash +cd /tmp/lody-title-agent +# PR URL should contain "draft" label +# PR description includes execplan summary and lody session link +# CodeRabbit review completed: coderabbit review --agent +``` + +Expected: PR is draft, CodeRabbit review has no blocking concerns, all concerns resolved. + + +## Idempotence and recovery + +All steps in this plan are idempotent and can be re-run safely: + +- Running tests multiple times with the same code should produce the same results. +- Creating files is safe if the files don't exist; if they do, the plan specifies what should be in them. +- Deleting files: No destructive operations are planned. If a file needs to be removed, document the reason and decision in the Decision Log before deletion. + +**Rollback**: If the entire implementation needs to be rolled back, delete the `workflow_scripts/review_views/` directory and revert any changes to existing files (tracked by git). No database migrations or configuration changes are planned. + + +## Artifacts and notes + +Key design decisions and expected artifacts: + +1. **ReviewViewRef dataclass** (critical, Phase 2): + +```python +from dataclasses import dataclass + +@dataclass(frozen=True) +class ReviewViewRef: + """Host-neutral reference to a review comment.""" + comment_id: str + view_type: str + + def to_dict(self) -> dict: + """Serialize to JSON-safe dict.""" + return {"comment_id": self.comment_id, "view_type": self.view_type} + + @classmethod + def from_dict(cls, data: dict) -> "ReviewViewRef": + """Deserialize from dict; raises KeyError if required fields missing.""" + return cls(comment_id=data["comment_id"], view_type=data["view_type"]) +``` + +2. **ViewResolver interface and registry** (critical, Phase 4): + +```python +from abc import ABC, abstractmethod +from workflow_scripts.review_views.models import ReviewViewRef + +class ViewResolver(ABC): + """Abstract resolver for translating ReviewViewRef to host-specific URLs.""" + @abstractmethod + def resolve(self, ref: ReviewViewRef) -> str: + """Resolve a ReviewViewRef to a host-specific view URL.""" + pass + +class ViewResolverRegistry: + """Central registry for view resolvers across hosts.""" + _resolvers = {} + + @classmethod + def register(cls, host: str, resolver: ViewResolver) -> None: + cls._resolvers[host] = resolver + + @classmethod + def resolve_for_host(cls, host: str) -> ViewResolver: + if host not in cls._resolvers: + raise ValueError(f"No resolver registered for host: {host}") + return cls._resolvers[host] +``` + +3. **Expected test files** (all should pass by end of Phase 7): + - `workflow_scripts/tests/test_review_views.py` — comprehensive domain model tests (serialization, deserialization, error handling, edge cases, adapter boundary, JSON safety) + - `workflow_scripts/tests/test_cli_review_context.py` — CLI resolver and non-TUI context + - `workflow_scripts/tests/test_library_review_context.py` — library serialization without TUI + +4. **ADRs to be created** (Phase 1 deliverables): + - `docs/adr/0008-pr-discussion-summary-contract.md` — defines ReviewCommentSummary schema, ReviewViewRef structure, serialization format, and backward compatibility strategy + - `docs/adr/0010-close-review-adapter-capability-gap.md` — explains ViewResolver pattern, adapter responsibilities (translation only, no business logic), hexagonal dependency direction, registry pattern, TUI/domain boundary enforcement + +5. **Documentation updates** (Phase 6): + - `docs/developers-guide.md`: New section "View Resolution Pattern" with code example, ASCII dependency diagram, and adapter registration example + - `docs/users-guide.md`: Describe how users/developers interact with ReviewViewRef in CLI and library contexts + + +## Interfaces and dependencies + +**New interfaces to be created**: + +1. `ReviewViewRef` dataclass in `workflow_scripts/review_views/models.py`: + - Fields: `comment_id: str`, `view_type: str` (frozen=True, slots=True if Python supports) + - Methods: `to_dict() -> dict`, `from_dict(data: dict) -> ReviewViewRef` + - Must be serializable to JSON without loss of information + - Must not import TUI-specific modules + - Deserialization must validate required fields and raise KeyError if missing + - Serialization must not include TUI-specific fields or metadata + +2. `ViewResolver` abstract base class in `workflow_scripts/review_views/adapters/__init__.py`: + - Abstract method: `resolve(ref: ReviewViewRef) -> str` + - Concrete implementations (one per host): `TUIViewResolver`, `CLIViewResolver`, `LibraryViewResolver` + - Stateless (no shared mutable state; safe to instantiate once and reuse) + +3. `ViewResolverRegistry` class in `workflow_scripts/review_views/adapters/__init__.py`: + - Class methods: `register(host: str, resolver: ViewResolver)`, `resolve_for_host(host: str) -> ViewResolver` + - Allows hosts to discover and use the correct resolver without importing domain types directly + - TUI code calls `ViewResolverRegistry.register("tui", TUIViewResolver())` at startup + +4. **ReviewCommentSummary DTO** in `workflow_scripts/review_views/summary.py` (or refactored from existing location, Phase 1 determines): + - Fields: `id: str`, `text: str`, `view_ref: ReviewViewRef` + - Methods: `to_dict() -> dict`, `from_dict(data: dict) -> ReviewCommentSummary` + - Must not include TUI-specific link fields (`frankie_url`, `tui_handler`, etc.) + - Must be serializable via to_dict()/from_dict() without TUI imports + +5. Custom exception types in `workflow_scripts/review_views/errors.py`: + - `ReviewViewError` (base exception) + - `ResolutionError` (raised when view reference cannot be resolved) + +**Dependencies** (external packages): + +- **No new external packages required**. The implementation uses only Python standard library: + - `dataclasses` (for dataclass decorator and frozen/slots) + - `abc` (for ABC and abstractmethod) + - `typing` (for type hints) + - `json` (for serialization tests) + +**Existing modules to integrate with** (identified during design phase, Phase 1): + +- Any existing review summary export code in the codebase (location TBD, Phase 1 discovers these) +- TUI-specific code that renders review comments (will be updated to use ViewResolverRegistry) + +--- + +## Revision Note (2026-06-18) + +**What changed:** +- Added formal Glossary section defining key terms ("host-neutral reference", "host", "ReviewViewRef", "view resolver", "frankie://", "summary DTO") +- Enhanced Constraints to specify serialization format (JSON), type checking requirements, and coverage thresholds (100% for domain models, 85% for others) +- Added five new Risks specific to Python ecosystem (type checking, namespace pollution, coverage tooling, test infrastructure) +- Expanded Phase 1 with pre-flight check (verify prerequisites), explicit design artifacts (ADR 008/010), and approval gate +- Clarified Phase 3 decision criteria (create vs. refactor summary DTO) +- Introduced ViewResolverRegistry pattern in Phase 4 to prevent circular dependencies between TUI and domain layers +- Added comprehensive error handling and edge case tests in Phase 2 (missing fields, serialization round-trip, JSON safety, adapter boundary isolation) +- Expanded Phase 5 with mock CLI resolver and library context examples +- Moved CodeRabbit review from Phase 6 to Phase 9 (PR creation) where it belongs +- Enhanced validation and acceptance section with concrete test commands and expected outputs +- Updated Artifacts section with complete code examples (frozen dataclasses with slots, registry pattern, complete DTO) +- Updated Interfaces and dependencies to specify ViewResolverRegistry entry point and custom exception types + +**Why it changed:** +The expert review team identified three classes of gaps: +1. **Foundational concept clarity**: "Host-neutral reference" needed formal definition before Phase 1 work could lock in the abstraction +2. **Architectural enforcement**: Hexagonal boundary violations (TUI types leaking into domain, adapter business logic) needed explicit tests and patterns (registry) to prevent +3. **Test completeness**: Error handling, edge cases, and adapter isolation tests were missing; CLI/library tests were mentioned but not exemplified + +**How it affects remaining work:** +- Phase 1 is now more demanding (pre-flight check, design artifacts, approval gate) but reduces downstream rework +- Phase 2 test examples are now concrete and comprehensive, reducing ambiguity during Red-Green-Refactor +- Phases 4-5 include explicit adapter boundary tests and registry pattern, enforcing the hexagonal architecture +- The validation section now provides exact test commands and expected outputs, making acceptance criteria unambiguous +- Overall risk of architectural drift (TUI types in domain, circular dependencies) is significantly reduced through explicit tests and the registry pattern diff --git a/docs/execplans/4-2-2-safe-host-mounted-workspaces.md b/docs/execplans/4-2-2-safe-host-mounted-workspaces.md new file mode 100644 index 00000000..9c0079b5 --- /dev/null +++ b/docs/execplans/4-2-2-safe-host-mounted-workspaces.md @@ -0,0 +1,1114 @@ +# 4.2.2 Implement Safe Host-Mounted Workspaces + +This ExecPlan is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, +`Decision Log`, and `Outcomes & Retrospective` must be kept up to date +as work proceeds. + +Status: APPROVED + +## Purpose / big picture + +This milestone implements safe host-mounted workspaces—the mechanism by +which operators can mount host directories into container sessions while +maintaining strict boundaries against symlink escapes, unauthorized path +access, and privilege escalation vectors. + +After this change, users will be able to configure host-mounted +workspaces with confidence that all mount paths are canonicalized, +mounts are confined to allowlisted roots, write permissions are +validated, the threat model is documented, and forbidden paths are +rejected even under adversarial conditions. + +Success is observable when: +1. Running `cargo test` in `tests/workspace_mounts_tests.rs` shows ≥40 tests passing with ≥90% coverage +2. BDD scenarios in `tests/bdd/workspace_mounts.feature` all pass +3. Documentation in `docs/users-guide.md` and `docs/developers-guide.md` describes configuration and threat model +4. `make check-fmt`, `make lint`, and `make test` all pass without errors +5. A PR against main is created with all gates passing + +## Constraints + +Hard invariants that must hold throughout implementation. + +- **No public API breaking changes**: Existing container launch signatures must remain stable. New validation functions are internal to workspace_mounts module. +- **No new external crate dependencies**: Use only Rust std::fs, std::path, and existing project dependencies (thiserror for error handling, tracing for logging). +- **Rootless Podman compatibility**: All validation must work correctly in rootless namespaces without requiring uid 0. +- **Backward compatibility**: Existing workspace configurations must continue to work; validation is additive. +- **Zero symlinks in validated paths**: After canonicalization, no symlinks may exist in any component of the validated path. +- **Domain logic has no I/O**: All filesystem operations pass through FilesystemPort trait; domain validator is pure logic, all I/O happens in adapters. +- **Hexagonal architecture maintained**: Dependency graph must point inward; ports defined at domain boundary; adapters implement ports, never call each other. +- **All errors are domain-owned**: WorkspaceMountError enum is exhaustive; infrastructure errors converted to domain errors at adapter boundaries. + +## Tolerances (exception triggers) + +Thresholds that trigger escalation when breached. + +- **Scope**: ≤20 files modified, ≤3000 net lines of code added. If exceeded, stop and present impact analysis. +- **Dependencies**: Any new external crate must be escalated. Standard library only, unless project already uses a crate (e.g., thiserror, tracing). +- **Interface changes**: If any public API signature in existing modules must change, stop and escalate with rationale. +- **Test coverage**: Domain logic must reach ≥90% line coverage and ≥90% branch coverage. If coverage gaps exist after Phase 5, stop and investigate. +- **Build/test failures**: If make test, make lint, or make check-fmt fails after implementation, stop and fix before proceeding. +- **Threat model disagreement**: If during implementation the threat model interpretation differs materially from the documented 7 attack vectors, escalate with evidence. +- **Time**: If any single phase takes >6 hours, document progress and reassess plan feasibility. + +## Risks + +Known uncertainties that might affect the plan. + +- **TOCTOU race: symlink added after validation but before mount** + Severity: High | Likelihood: Medium | Mitigation: Minimize validation window; use `canonicalize()` + re-check symlinks immediately before mount; document assumption that operator controls allowlist roots + +- **Permission validation unreliable in rootless: UID/GID remapping** + Severity: High | Likelihood: High | Mitigation: Add RootlessPermissionAdapter that uses `libc::geteuid()` and `libc::getegid()` to validate namespace context; write integration tests with `podman unshare` + +- **Symlink cycles cause infinite loop during depth traversal** + Severity: Medium | Likelihood: Low | Mitigation: Set hard limit of 40 components per path; detect cycles explicitly in OsFilesystemAdapter.has_symlinks() + +- **Allowlist prefix attacks: /tmp/workspaces_evil matches /tmp/workspaces** + Severity: High | Likelihood: Low | Mitigation: Validate component boundaries, not just string prefix; use `path.starts_with(root) && (path.len() == root.len() || path[root.len()] == b'/')` pattern + +- **Mount flag escapes via remount/shared flags** + Severity: Medium | Likelihood: Low | Mitigation: Future enhancement; document in roadmap; for now assume container runtime enforces safe flags + +- **Test suite insufficient**: Coverage tools may miss branches + Severity: Medium | Likelihood: Medium | Mitigation: Use `cargo tarpaulin --out Html` for detailed branch coverage; manually inspect untested paths; use property tests (proptest) for idempotence guarantees + +- **Integration test environment differences**: Tests pass locally but fail in CI with different rootless setup + Severity: Medium | Likelihood: Medium | Mitigation: Test both as regular user and with `podman unshare`; document platform assumptions; skip tests that require rootless if not available + +## Progress + +Use checkboxes to track granular steps with timestamps. Each stage must complete validation before proceeding. + +- [ ] **Stage 1: Threat model & architecture documentation** (Est. 2-3 hours) + - [ ] (TBD) Create docs/architecture/04-workspace-mounts-threat-model.md + - [ ] (TBD) Create docs/architecture/05-workspace-mounts-hexagonal-design.md + - [ ] (TBD) Create docs/adr/0003-workspace-mount-validation.md + - [ ] (TBD) Validation: Threat model is unambiguous, design approved + +- [ ] **Stage 2: Module structure & port traits** (Est. 1-2 hours) + - [ ] (TBD) Create src/workspace_mounts/ directory structure + - [ ] (TBD) Define all port traits (Filesystem, Permission, Config, ThreatReporter) + - [ ] (TBD) Define value object stubs + - [ ] (TBD) Validation: `cargo check` passes + +- [ ] **Stage 3: Error type & value objects (Red-Green-Refactor)** (Est. 1-2 hours) + - [ ] (TBD) Write Red: Unit tests for all 10 error variants + - [ ] (TBD) Write Green: Implement WorkspaceMountError, ValidatedWorkspacePath, AllowedWorkspaceRoot + - [ ] (TBD) Refactor: Document and improve error messages + - [ ] (TBD) Validation: `cargo test --lib workspace_mounts` all pass + +- [ ] **Stage 4: FilesystemPort & OsFilesystemAdapter (Red-Green-Refactor)** (Est. 2-3 hours) + - [ ] (TBD) Write Red: Unit & integration tests for symlink detection, depth limit, cycles + - [ ] (TBD) Write Green: Implement OsFilesystemAdapter with canonicalize, has_symlinks, metadata checks + - [ ] (TBD) Refactor: Extract helpers, add property tests + - [ ] (TBD) Validation: `cargo test --test integration` passes, ≥90% coverage + +- [ ] **Stage 5: PermissionPort & RootlessPermissionAdapter (Red-Green-Refactor)** (Est. 2-3 hours) + - [ ] (TBD) Write Red: Unit & integration tests for mode bits, UID/GID checks + - [ ] (TBD) Write Green: Implement RootlessPermissionAdapter using libc + - [ ] (TBD) Add rootless integration tests + - [ ] (TBD) Validation: Works in both regular and rootless environments + +- [ ] **Stage 6: Domain Validator (Pure logic, Red-Green-Refactor)** (Est. 3-4 hours) + - [ ] (TBD) Write Red: Unit tests for all attack vectors, property tests for idempotence + - [ ] (TBD) Write Green: Implement WorkspacePathValidator::validate() + - [ ] (TBD) Refactor: Extract pure helpers, document assumptions + - [ ] (TBD) Validation: ≥90% branch coverage, property tests pass + +- [ ] **Stage 7: Integration, BDD scenarios, & application service (Red-Green-Refactor)** (Est. 3-4 hours) + - [ ] (TBD) Write Red: BDD scenarios in tests/bdd/workspace_mounts.feature + - [ ] (TBD) Write Green: Implement WorkspaceMountServiceImpl, BDD steps + - [ ] (TBD) Add snapshot tests, stress tests + - [ ] (TBD) Validation: All BDD scenarios pass, integration tests pass + +- [ ] **Stage 8: Documentation & deployment guide** (Est. 2-3 hours) + - [ ] (TBD) Update docs/users-guide.md with configuration and examples + - [ ] (TBD) Update docs/developers-guide.md with architecture and testing + - [ ] (TBD) Create docs/adr/0004-workspace-mount-validation-rationale.md + - [ ] (TBD) Validation: Documentation builds, examples are correct + +- [ ] **Stage 9: Quality gates & final validation** (Est. 2-3 hours) + - [ ] (TBD) Run make check-fmt, make lint, cargo test + - [ ] (TBD) Run cargo tarpaulin --out Html and verify ≥90% coverage + - [ ] (TBD) Create PR and request CodeRabbit review + - [ ] (TBD) Address review comments + - [ ] (TBD) Update roadmap marking 4.2.2 as COMPLETE + - [ ] (TBD) Validation: All gates pass, PR approved + +## Surprises & discoveries + +Unexpected findings during implementation. This section is updated as +work proceeds. + +(None yet; will be populated during implementation.) + +## Decision log + +Record every significant decision made while working on the plan. + +- Decision: Use Rust `std::fs::canonicalize()` as baseline + Rationale: Stable, audited. Avoids external crates unless stdlib + proves insufficient + Date: 2026-06-18 (planning phase) + +- Decision: Allowlist roots configured in YAML at startup + Rationale: Enforces operator control. Errors detected at startup, not + mount time + Date: 2026-06-18 (planning phase) + +- Decision: Permission validation uses `podman unshare` in rootless + Rationale: UID/GID remapping makes naive checks unreliable. Testing + in actual namespace is only reliable method + Date: 2026-06-18 (planning phase) + +(Additional decisions recorded during implementation.) + +## Outcomes & retrospective + +This section is populated at major milestones and at completion. + +(To be updated as work proceeds.) + +## Context and orientation + +### Current state + +Podbot is a container orchestration tool organized around composing container launches from reusable request/plan primitives. The codebase currently lacks specialized workspace mount validation, which is a gap for safe host-mounted workspace support. + +**Related existing infrastructure**: +- `src/config.rs` — Configuration loading and validation +- `src/container.rs` — Container launch and execution +- `Cargo.toml` — Dependencies (thiserror for errors, tracing for logging likely available) +- Tests framework: cargo test (unit and integration) + +**Current limitations**: +- No canonicalization of mount paths +- No allowlist enforcement +- No validation of mount permissions +- No threat model documentation +- Risk: Operators could accidentally mount dangerous paths (symlinks, parent dirs, read-only dirs) + +### Project structure + +The Podbot codebase uses a modular approach with clear separation between config, domain logic, and container execution. + +### Implementation approach: Hexagonal architecture + +This task uses **hexagonal architecture** (ports & adapters) to maintain clean separation: +- **Domain** (`src/workspace_mounts/domain/`) — Pure business logic, no I/O +- **Ports** (`src/workspace_mounts/domain/ports/`) — Abstract interfaces (what domain requires from infrastructure) +- **Adapters** (`src/workspace_mounts/adapters/`) — Concrete implementations (filesystem I/O, permission checks, config loading) +- **Application Service** (`src/workspace_mounts/lib.rs`) — Orchestrates domain + adapters for callers + +This design ensures: +- ✓ Domain logic is testable without I/O (use mocks) +- ✓ Adapters are easy to replace or extend (e.g., add SELinux checks later) +- ✓ All dependencies point inward (domain ← adapters) +- ✓ Errors are domain-owned (not infrastructure-specific) + +### Key files to create + +**Domain logic** (no I/O): +- `src/workspace_mounts/domain/errors.rs` — WorkspaceMountError enum (10 variants) +- `src/workspace_mounts/domain/model.rs` — Value objects: ValidatedWorkspacePath, AllowedWorkspaceRoot, WorkspaceConfig +- `src/workspace_mounts/domain/validator.rs` — WorkspacePathValidator (pure logic) +- `src/workspace_mounts/domain/ports/filesystem.rs` — FilesystemPort trait +- `src/workspace_mounts/domain/ports/permission.rs` — PermissionPort trait +- `src/workspace_mounts/domain/ports/config.rs` — WorkspaceConfigPort trait +- `src/workspace_mounts/domain/ports/threat_reporter.rs` — ThreatReportPort trait + +**Adapters** (with I/O): +- `src/workspace_mounts/adapters/os_filesystem.rs` — OsFilesystemAdapter (std::fs wrapper) +- `src/workspace_mounts/adapters/permission.rs` — RootlessPermissionAdapter (libc wrapper) +- `src/workspace_mounts/adapters/yaml_config.rs` — YamlConfigAdapter (YAML config loader) +- `src/workspace_mounts/adapters/logging.rs` — LoggingThreatReporter (tracing integration) + +**Application service** (boundary): +- `src/workspace_mounts/lib.rs` — WorkspaceMountServiceImpl (orchestrates domain + adapters) + +**Tests**: +- `tests/workspace_mounts_tests.rs` — Unit tests with mocks +- `tests/workspace_mounts_integration_tests.rs` — Integration tests with real filesystem +- `tests/bdd/workspace_mounts.feature` — Behavior-driven scenarios +- `tests/bdd/workspace_mounts_steps.rs` — BDD step implementations + +**Documentation**: +- `docs/architecture/04-workspace-mounts-threat-model.md` — Security threat model +- `docs/architecture/05-workspace-mounts-hexagonal-design.md` — Architecture diagram & design +- `docs/adr/0003-workspace-mount-validation.md` — Architecture decision record +- `docs/users-guide.md` — Updated with configuration section +- `docs/developers-guide.md` — Updated with testing & architecture guidance + +## Plan of work + +Implementation proceeds through seven stages with explicit Red-Green-Refactor validation at each domain logic milestone. All code changes are small, testable, and committed after each stage passes quality gates. + +### Stage 1: Threat model & architecture documentation + +**Objective**: Design the domain model, ports, and adapters before writing code. + +**Work**: +1. Create `docs/architecture/04-workspace-mounts-threat-model.md` documenting: + - All 7 attack vectors (symlink escape, TOCTOU, path traversal, allowlist prefix bypass, permission escalation, rootless UID/GID mismatch, symlink cycles) + - Prevention mechanisms for each + - Explicit threat boundaries (what we control vs. what we delegate) + - Assumptions (operator correctness, POSIX filesystem, atomic mounts) +2. Create `docs/architecture/05-workspace-mounts-hexagonal-design.md` with: + - Domain model (WorkspaceMountError, ValidatedWorkspacePath, AllowedWorkspaceRoot, WorkspaceConfig) + - Ports (FilesystemPort, PermissionPort, WorkspaceConfigPort, ThreatReportPort) + - Adapters (OsFilesystemAdapter, RootlessPermissionAdapter, YamlConfigAdapter, LoggingThreatReporter) + - Hexagonal diagram +3. Create `docs/adr/0003-workspace-mount-validation.md` recording architecture decisions + +**Validation**: Threat model is unambiguous; hexagonal diagram is reviewed; no implementation has begun. + +**Estimated effort**: 2-3 hours + +### Stage 2: Module structure & port traits + +**Objective**: Create the module skeleton and port trait definitions. + +**Work**: +1. Create new module `src/workspace_mounts/` directory with: + - `mod.rs` — exports public API + - `domain/errors.rs` — WorkspaceMountError enum (10 variants) + - `domain/model.rs` — Value objects (ValidatedWorkspacePath, AllowedWorkspaceRoot, WorkspaceConfig) + - `domain/validator.rs` — Pure-logic domain service (WorkspacePathValidator) + - `domain/ports/filesystem.rs` — FilesystemPort trait + - `domain/ports/permission.rs` — PermissionPort trait + - `domain/ports/config.rs` — WorkspaceConfigPort trait + - `domain/ports/threat_reporter.rs` — ThreatReportPort trait + - `adapters/os_filesystem.rs` — OsFilesystemAdapter + - `adapters/permission.rs` — RootlessPermissionAdapter + - `adapters/yaml_config.rs` — YamlConfigAdapter + - `adapters/logging.rs` — LoggingThreatReporter + - `lib.rs` or public types in `mod.rs` — WorkspaceMountServiceImpl (application service) + +2. Write all trait signatures and struct definitions (no implementation bodies yet) + +**Validation**: `cargo check` passes; no missing trait methods; module structure matches hexagonal design. + +**Estimated effort**: 1-2 hours + +### Stage 3: Error type & value objects (Red-Green-Refactor) + +**Objective**: Define exhaustive error type and invariant-enforcing value objects. + +**Red**: Write unit tests in `tests/workspace_mounts_tests.rs` that: +- Construct each of the 10 WorkspaceMountError variants +- Verify ValidatedWorkspacePath can only be created by validator +- Verify AllowedWorkspaceRoot rejects invalid paths at construction + +**Green**: Implement: +- WorkspaceMountError with `#[derive(Debug, Clone, PartialEq)]` and thiserror::Error +- ValidatedWorkspacePath as newtype with private constructor; `pub fn new_unchecked()` for validator only +- AllowedWorkspaceRoot with validation at construction (path exists, is directory, is absolute) +- WorkspaceConfig as aggregate root (holds list of AllowedWorkspaceRoot) + +**Refactor**: Ensure error Display messages are actionable; document each variant with example. + +**Validation**: `cargo test --lib workspace_mounts` passes; 100% error type coverage; all error messages are tested. + +**Estimated effort**: 1-2 hours + +### Stage 4: FilesystemPort & OsFilesystemAdapter (Red-Green-Refactor) + +**Objective**: Implement abstracted filesystem operations. + +**Red**: Write unit tests with mocks: +- `test_canonicalize_rejects_symlinks_in_path` +- `test_canonicalize_resolves_dotdot` +- `test_has_symlinks_detects_all_components` +- `test_has_symlinks_respects_depth_limit` +- `test_has_symlinks_detects_cycles` + +Write integration tests with real filesystem (using tempfile): +- Create actual symlink, verify detection +- Create symlink cycle, verify depth limit prevents DoS +- Create deeply nested symlinks, verify limit respected + +**Green**: Implement: +- FilesystemPort trait with methods: `canonicalize()`, `has_symlinks()`, `read_metadata_nofollow()`, `exists()` +- OsFilesystemAdapter using std::fs, std::path, fs::read_link() +- Depth limit constant (40 components) +- Cycle detection via visited set + +**Refactor**: Extract helper functions; document assumptions; add property tests for idempotence. + +**Validation**: `cargo test --test integration filesystem` passes; `cargo tarpaulin` shows ≥90% branch coverage for adapter. + +**Estimated effort**: 2-3 hours + +### Stage 5: PermissionPort & RootlessPermissionAdapter (Red-Green-Refactor) + +**Objective**: Implement permission validation compatible with rootless containers. + +**Red**: Write unit tests with mocks: +- `test_is_writable_checks_mode_bits` +- `test_is_writable_rejects_read_only` +- `test_container_ids_returns_euid_egid` +- `test_rootless_context_validation_succeeds_in_namespace` + +Write integration tests: +- Set path mode to 0o444, verify writable check rejects +- Run in `podman unshare` namespace, verify UID/GID checks work + +**Green**: Implement: +- PermissionPort trait with methods: `is_writable()`, `container_ids()`, `check_security_context()` +- RootlessPermissionAdapter using libc::geteuid(), libc::getegid() +- Mode bit checking: `(stat.st_mode & 0o200) != 0` + +**Refactor**: Document namespace assumptions; add error context showing actual vs. expected UID. + +**Validation**: `cargo test --test integration permission` passes; works in both regular and rootless environments. + +**Estimated effort**: 2-3 hours + +### Stage 6: Domain Validator (Pure logic, Red-Green-Refactor) + +**Objective**: Implement pure-logic domain validator without I/O. + +**Red**: Write comprehensive unit tests with mocks: +- Three-stage validation passes for valid paths +- Symlink escape attempt rejected at canonicalize step +- Path traversal (/../..) rejected at canonicalize step +- Allowlist prefix attack (/tmp/workspaces_evil vs /tmp/workspaces) rejected +- Permission check catches read-only directories +- TOCTOU windows are minimized (re-check after external operations) + +Property tests: +- `validate(validate(path))` returns same result (idempotence) +- Allowlist member check is consistent (same path always passes/fails) + +**Green**: Implement WorkspacePathValidator::validate(): +```rust +pub fn validate(&self, path: &Path) -> Result { + // Step 1: Canonicalize (rejects symlinks) + let canonical = self.filesystem.canonicalize(path)?; + + // Step 2: Enforce allowlist (checks component boundaries) + self.check_in_allowlist(&canonical)?; + + // Step 3: Validate permissions (checks writable) + self.validate_permissions(&canonical)?; + + // Step 4: Report success + self.threat_reporter.report_mount_approved(&canonical); + Ok(ValidatedWorkspacePath::new_unchecked(canonical)) +} +``` + +**Refactor**: Extract pure helper methods; document assumptions; add error context. + +**Validation**: `cargo test --lib validator` passes; ≥90% branch coverage; property tests pass. + +**Estimated effort**: 3-4 hours + +### Stage 7: Integration, BDD scenarios, & application service (Red-Green-Refactor) + +**Objective**: Integrate all components; write E2E tests; expose public API. + +**Red**: Write BDD scenarios in `tests/bdd/workspace_mounts.feature`: +```gherkin +Feature: Safe host-mounted workspaces + Scenario: Valid workspace path is validated successfully + Given an allowed root "/tmp/workspaces" + And a valid workspace path "/tmp/workspaces/job-123" + When I validate the workspace mount + Then the mount should be approved + + Scenario: Symlink escape attempt is rejected + Given an allowed root "/tmp/workspaces" + And a symlink "/tmp/workspaces/link" pointing to "/etc" + When I validate the workspace mount for "/tmp/workspaces/link" + Then the mount should be rejected with reason "symlink detected" + + Scenario: Path outside allowlist is rejected + Given allowed roots ["/tmp/workspaces"] + And a valid path "/home/attacker/workspace" + When I validate the workspace mount + Then the mount should be rejected with reason "not in allowlist" +``` + +Run `cargo test --test bdd` and watch all scenarios fail. + +**Green**: Implement WorkspaceMountServiceImpl: +```rust +pub struct WorkspaceMountServiceImpl { + validator: Arc, +} + +impl WorkspaceMountServiceImpl { + pub fn new(config, filesystem, permission, threat_reporter) -> Self { ... } + pub fn validate_mount(&self, path: &Path) -> Result { ... } + pub fn validate_mounts(&self, paths: &[PathBuf]) + -> Result, Vec<(PathBuf, WorkspaceMountError)>> { ... } +} +``` + +Write BDD step implementations (using cucumber or similar). + +Add snapshot tests showing error message formatting for each error variant. + +Add stress tests: validate 1000 paths with various attacks. + +**Refactor**: Ensure error messages are consistent; add structured logging via ThreatReportPort; document composition pattern. + +**Validation**: `cargo test --test bdd` all scenarios pass; `cargo test --test integration` all integration tests pass; `cargo test --lib` all unit tests pass. + +**Estimated effort**: 3-4 hours + +### Stage 8: Documentation & deployment guide + +**Objective**: Document configuration, threat model, and deployment steps. + +**Work**: +1. Update `docs/users-guide.md` with: + - Configuration schema (YAML example) + - Allowed roots setup and validation + - Error messages and troubleshooting + - Examples of valid and invalid paths + +2. Update `docs/developers-guide.md` with: + - Module architecture (domain, adapters) + - How to add new adapters (e.g., SELinux context port) + - Testing guidelines (unit, integration, BDD) + - Security review checklist + +3. Create `docs/adr/0004-workspace-mount-validation-rationale.md` with decisions and trade-offs + +**Validation**: Documentation builds without errors; examples are tested (copy-paste works). + +**Estimated effort**: 2-3 hours + +### Stage 9: Quality gates & final validation + +**Objective**: Run all checks and verify production readiness. + +**Work**: +1. Run `make check-fmt` — all workspace_mounts code is formatted +2. Run `make lint` — no clippy warnings +3. Run `cargo test` — all tests pass +4. Run `cargo tarpaulin --out Html --timeout 300` — verify ≥90% coverage +5. Run `make check-docs` if available — documentation builds +6. Create draft PR and request CodeRabbit review +7. Address any review comments +8. Mark task as COMPLETE in roadmap + +**Validation**: All gates pass; PR is ready for merge. + +**Estimated effort**: 2-3 hours (including review turnaround) + +## Validation and acceptance + +### Acceptance criteria (all must be satisfied) + +1. **Test coverage**: `cargo tarpaulin --out Html` shows ≥90% line coverage and ≥90% branch coverage for all workspace_mounts code +2. **Unit tests pass**: `cargo test --lib workspace_mounts` runs with no failures; tests exercise all 10 error variants +3. **Integration tests pass**: `cargo test --test integration workspace_mounts` passes; tests use real filesystem and tempfile +4. **BDD scenarios pass**: `cargo test --test bdd workspace_mounts` passes; all Gherkin scenarios in `tests/bdd/workspace_mounts.feature` pass +5. **Property tests pass**: Idempotence properties validated; `validate(validate(path))` always returns same result +6. **Quality gates**: + - `make check-fmt` passes (all workspace_mounts code is formatted) + - `make lint` passes (no clippy warnings) + - `cargo test` (full suite) passes with no failures +7. **Documentation complete**: + - `docs/users-guide.md` includes configuration section with examples + - `docs/developers-guide.md` describes module architecture and testing strategy + - `docs/architecture/04-workspace-mounts-threat-model.md` documents all threat vectors + - `docs/adr/0003-workspace-mount-validation.md` records design decisions +8. **Configuration integration**: Podbot config loading accepts workspace_mounts configuration and validates at startup +9. **Rootless compatibility**: Tests pass in both regular and rootless (via `podman unshare`) environments +10. **PR created**: Branch pushed; PR created against main with all checks passing; CodeRabbit review completed + +### Quality method: Red-Green-Refactor + +For each domain logic component (error type, validator, adapters), follow this discipline: + +**Red phase**: +```bash +# Write failing test(s) that specify desired behaviour +# For unit tests, use mocks; for integration, use real filesystem +cargo test --lib workspace_mounts -- --nocapture +# Expected: test fails with "assertion failed" or "not implemented" +``` + +**Green phase**: +```bash +# Implement minimal code to make test pass +# Prioritize correctness over optimization +cargo test --lib workspace_mounts -- --nocapture +# Expected: test passes +``` + +**Refactor phase**: +```bash +# Clean up implementation without changing behaviour +# Extract pure helpers, improve error messages, add documentation +cargo test --lib workspace_mounts -- --nocapture +# Expected: test still passes; code is cleaner +``` + +### Quality method: Coverage verification + +After implementation of each stage, measure coverage: + +```bash +# Install tarpaulin if not present +cargo install cargo-tarpaulin + +# Run coverage on workspace_mounts module +cargo tarpaulin --out Html --timeout 300 --exclude-files 'tests/*' +# Expected: ≥90% line coverage, ≥90% branch coverage for workspace_mounts/ + +# Review HTML report to identify untested paths +open tarpaulin-report.html # or your browser +``` + +If coverage is <90%, investigate untested branches and add tests before proceeding. + +### Quality method: Integration testing with real filesystem + +For filesystem and permission adapters, test with real filesystem: + +```bash +# Create temp directory for tests +TEMP_DIR=$(mktemp -d) +cd $TEMP_DIR + +# Create test fixtures +mkdir -p workspaces/valid +ln -s /etc symlink_to_etc +touch readonly.txt && chmod 444 readonly.txt + +# Run integration tests +cd /path/to/podbot +cargo test --test integration workspace_mounts -- --nocapture + +# Cleanup +rm -rf $TEMP_DIR +``` + +### Quality method: Rootless testing + +For permission validation, test in rootless namespace: + +```bash +# Verify podman is available +podman --version + +# Run tests in rootless namespace (if available) +podman unshare cargo test --test integration permission -- --nocapture + +# Expected: Permission checks succeed in namespace context +# Expected: UID/GID in namespace context match expected values +``` + +### Quality method: BDD scenario validation + +```bash +# Run BDD scenarios +cargo test --test bdd workspace_mounts -- --nocapture + +# Expected output includes: +# Scenario: Valid workspace path is validated successfully ... PASSED +# Scenario: Symlink escape attempt is rejected ... PASSED +# Scenario: Path outside allowlist is rejected ... PASSED +# All scenarios pass with correct rejection reasons +``` + +### Expected outputs + +**After Stage 3 (error type)**: +``` +running 10 tests +test domain::errors::tests::error_variant_symlink_detected ... ok +test domain::errors::tests::error_variant_not_in_allowlist ... ok +... (8 more variants) +test result: ok. 10 passed; 0 failed +``` + +**After Stage 4 (filesystem adapter)**: +``` +running 25 tests (15 unit + 10 integration) +... filesystem canonicalization tests +... symlink detection tests +... depth limit tests +test result: ok. 25 passed; 0 failed +coverage: 92% (24/26 branches) +``` + +**After Stage 6 (domain validator)**: +``` +running 30 tests (20 unit + 10 property) +test domain::validator::tests::validate_accepts_allowlisted_path ... ok +test domain::validator::tests::validate_rejects_symlink_escape ... ok +test domain::validator::properties::prop_idempotent ... ok +... (27 more) +test result: ok. 30 passed; 0 failed +``` + +**After Stage 7 (full integration)**: +``` +running 50 tests (20 unit + 15 integration + 15 BDD) +test tests::workspace_mounts_tests::* ... ok +test tests::workspace_mounts_integration_tests::* ... ok +running feature: workspace_mounts +Scenario: Valid workspace path is validated successfully ... PASSED +Scenario: Symlink escape attempt is rejected ... PASSED +... (13 more scenarios) +test result: ok. 50 passed; 0 failed +coverage: 92% (lines), 91% (branches) +``` + +**Final quality gates**: +```bash +$ make check-fmt +All files formatted correctly ✓ + +$ make lint +No clippy warnings ✓ + +$ cargo test +... 150+ tests pass (unit + integration + BDD + all other tests) ✓ + +$ cargo tarpaulin +workspace_mounts: 92% coverage ✓ + +$ cargo build --release +Build succeeded ✓ +``` + +## Threat model summary + +The threat model identifies and mitigates seven major attack vectors: + +| # | Attack Vector | Prevention | Test | +|---|---|---|---| +| 1 | **Symlink Escape**: Mount `/tmp/ws/link→/etc` as `/workspace` | Canonicalize; reject symlinks; re-check immediately before mount | `test_symlink_escape_prevented` | +| 2 | **TOCTOU Race**: Symlink added after validation but before mount | Minimize validation window; canonicalize + re-check in atomic operation | `prop_validation_repeatable` | +| 3 | **Symlink Cycle DoS**: `/tmp/ws/a→b/b→a` causes infinite loop during traversal | Limit depth to 40 components; detect visited nodes | `test_symlink_cycle_detected` | +| 4 | **Path Traversal**: `/tmp/ws/../../../../etc/passwd` escapes root | Canonicalization resolves `..` and `.` | `test_path_traversal_prevented` | +| 5 | **Allowlist Prefix Bypass**: `/tmp/workspaces_evil` matches `/tmp/workspaces` | Validate component boundaries: `path.starts_with(root) && (len_match \|\| next_char == '/')` | `test_allowlist_prefix_attack_prevented` | +| 6 | **Permission Escalation**: Mount read-only directory (0o444) as writable | Pre-mount permission check: `(mode & 0o200) != 0` | `test_permission_check_read_only_rejected` | +| 7 | **Rootless UID/GID Mismatch**: Namespace remapping enables escape | Validate UID/GID in actual container namespace using `libc::geteuid()` and `libc::getegid()` | `test_rootless_permission_validation` | + +**Out of scope** (delegated to container runtime): +- Kernel bugs (runc/crun) — Mitigation: keep patched +- Mount flag attacks (remount, shared) — Future enhancement +- Capability escalation — Delegated to container runtime LSM/AppArmor + +**Assumptions**: +1. Operator configures allowlist roots correctly (no symlinks in config itself) +2. Filesystem is standard POSIX (ext4, tmpfs, overlayfs) +3. Podman/container runtime enforces safe mount flags +4. UID/GID mapping in rootless mode is correct + +## Test strategy + +### Test pyramid (40+ tests total) + +**Unit Tests (25+)** — Domain logic with mocks, no I/O +- Error enum: 10 tests (one per variant) +- Value objects: 5 tests (construction, invariants) +- Canonicalization logic: 5 tests (symlinks, traversal, depth) +- Allowlist: 3 tests (membership, prefix bypass, component boundaries) +- Permission: 2 tests (mode bits, idempotence) +- Domain validator: 5 tests (integration of three checks) + +**Property Tests (6+)** — Formal verification of invariants +- Idempotence: `validate(validate(x)) == validate(x)` +- Allowlist consistency: `is_member(x, roots)` always same result +- Canonicalization symmetry: `canonicalize(canonicalize(x)) == canonicalize(x)` +- Error stability: Same invalid path always produces same error +- Depth limit: Paths >40 components always rejected +- Symlink coverage: All symlink scenarios detected + +**Integration Tests (10+)** — Real filesystem, no mocks +- Symlink creation and detection +- Symlink cycle creation and handling +- Depth limit enforcement with nested symlinks +- Permission bits validation (read-only files) +- Real config file loading +- Temporary directory cleanup + +**BDD Scenarios (8+)** — End-to-end behavior specification +- Valid workspace path accepted +- Symlink escape attempt rejected +- Path outside allowlist rejected +- Read-only directory rejected +- Configuration loading and validation +- Error messages are actionable + +**Stress Tests (1+)** +- Validate 1000 paths with various attacks +- Measure canonicalization performance + +### Tools & commands + +```bash +# Run all tests +cargo test + +# Unit tests only (fast, no I/O) +cargo test --lib workspace_mounts + +# Integration tests (slower, real filesystem) +cargo test --test integration workspace_mounts + +# BDD scenarios +cargo test --test bdd workspace_mounts + +# Property tests (proptest crate) +cargo test --lib workspace_mounts -- --test-threads=1 proptest + +# Coverage measurement +cargo tarpaulin --out Html --timeout 300 + +# Coverage targeting specific module +cargo tarpaulin --packages podbot --lib workspace_mounts --out Html +``` + +### Coverage targets + +- **Line coverage**: ≥90% for workspace_mounts module +- **Branch coverage**: ≥90% for workspace_mounts module +- **Error path coverage**: All 10 error variants tested +- **Untested paths**: None (coverage gaps require investigation and new tests) + +## Interfaces and dependencies + +### New module: `src/workspace_mounts/` + +Public API exported from `src/workspace_mounts/lib.rs`: + +```rust +// Error type (10 variants, all recoverable) +pub enum WorkspaceMountError { ... } + +// Value objects (invariant-enforced) +pub struct ValidatedWorkspacePath { ... } +pub struct AllowedWorkspaceRoot { ... } +pub struct WorkspaceConfig { ... } + +// Application service (what consumers use) +pub struct WorkspaceMountServiceImpl { ... } + +impl WorkspaceMountServiceImpl { + pub fn new(config, filesystem, permission, threat_reporter) -> Self { ... } + pub fn validate_mount(&self, path: &Path) -> Result { ... } + pub fn validate_mounts(&self, paths: &[PathBuf]) + -> Result, Vec<(PathBuf, WorkspaceMountError)>> { ... } +} +``` + +### Internal module structure + +``` +src/workspace_mounts/ +├── mod.rs # Public exports +├── lib.rs # Application service +├── domain/ +│ ├── errors.rs # WorkspaceMountError (10 variants) +│ ├── model.rs # Value objects +│ ├── validator.rs # WorkspacePathValidator (pure logic) +│ ├── ports/ +│ │ ├── filesystem.rs # FilesystemPort trait +│ │ ├── permission.rs # PermissionPort trait +│ │ ├── config.rs # WorkspaceConfigPort trait +│ │ └── threat_reporter.rs # ThreatReportPort trait +│ └── tests/ +│ ├── errors_tests.rs +│ ├── model_tests.rs +│ ├── validator_tests.rs +│ └── properties_tests.rs +│ +└── adapters/ + ├── os_filesystem.rs # OsFilesystemAdapter (std::fs) + ├── permission.rs # RootlessPermissionAdapter (libc) + ├── yaml_config.rs # YamlConfigAdapter (config loading) + └── logging.rs # LoggingThreatReporter (tracing) +``` + +### Extension to existing modules + +**`src/config.rs`** — Add configuration support: +```rust +pub struct PodobotConfig { + // ... existing fields + pub workspace_mounts: Option, +} + +pub struct WorkspaceMountConfig { + pub allowed_roots: Vec, + // future: mount_flags, validation_mode, etc. +} +``` + +### Dependencies + +**Use from Rust standard library**: +- `std::fs` — Filesystem operations +- `std::path` — Path handling +- `std::fs::read_link()` — Symlink detection +- `libc` — POSIX UID/GID checks (likely already dependency) + +**Use from existing Podbot dependencies**: +- `thiserror` — Error enum derivation +- `tracing` — Structured logging + +**No new external crates** without escalation. + +**Testing-only crates** (already available): +- `proptest` — Property testing +- `tempfile` — Temporary test directories +- `mockall` — Mock trait implementations (or `double` if available) +- `insta` — Snapshot testing +- Cucumber/BDD framework if available, else write custom BDD runner + +### Type signatures (reference for implementation) + +```rust +// Domain errors +#[derive(Debug, Clone, Error)] +pub enum WorkspaceMountError { + #[error("symlink detected at {path}: {component:?}")] + SymlinkDetected { path: PathBuf, component: Option }, + + #[error("path not in allowlist: {path}\nallowed: {allowed_roots:?}")] + NotInAllowlist { path: PathBuf, allowed_roots: Vec }, + + // ... 8 more variants +} + +// Ports (abstractions) +pub trait FilesystemPort { + fn canonicalize(&self, path: &Path) -> Result; + fn has_symlinks(&self, path: &Path) -> Result; + fn read_metadata_nofollow(&self, path: &Path) -> Result; + fn exists(&self, path: &Path) -> Result; +} + +pub trait PermissionPort { + fn is_writable(&self, path: &Path) -> Result<()>; + fn container_ids(&self) -> Result<(u32, u32)>; + fn check_security_context(&self, path: &Path) -> Result<()>; +} + +// Domain service (pure logic) +pub struct WorkspacePathValidator { + config: WorkspaceConfig, + filesystem: Arc, + permission: Arc, + threat_reporter: Arc, +} + +impl WorkspacePathValidator { + pub fn validate(&self, path: &Path) -> Result; +} + +// Application service (what callers use) +pub struct WorkspaceMountServiceImpl { + validator: Arc, +} + +impl WorkspaceMountServiceImpl { + pub fn validate_mount(&self, path: &Path) -> Result; + pub fn validate_mounts(&self, paths: &[PathBuf]) + -> Result, Vec<(PathBuf, WorkspaceMountError)>>; +} +``` + +## Integration with other tasks + +This task depends on and integrates with: + +- **Task 1.4.1 (Container Launch)**: The WorkspaceMountServiceImpl validates all workspace paths before container command construction. Canonical paths are passed to bind_mount(). + +- **Task 2.2.5 (MCP Integration)**: The same service is used by MCP workspace mount protocol handler. Ensures consistent validation across all launch entry points. + +**Data flow**: +``` +HTTP/CLI/MCP request + ↓ +WorkspaceMountServiceImpl::validate_mount() + ↓ +WorkspacePathValidator (pure domain logic) + ├─→ Canonicalize (FilesystemPort) + ├─→ Check allowlist (pure logic) + └─→ Validate permissions (PermissionPort) + ↓ +ValidatedWorkspacePath (newtype, invariant-enforced) + ↓ +Container launch (Task 1.4.1) or MCP response (Task 2.2.5) +``` + +Both tasks use **identical validation**, preventing validation bypass via different launch paths. + +## Concrete steps for implementation + +### Setup and environment + +```bash +# Ensure you're on the correct branch +git branch --show-current +# Expected: 4-2-2-safe-host-mounted-workspaces + +# Verify Rust toolchain +rustc --version +cargo --version +# Expected: stable or recent nightly + +# Install tarpaulin for coverage (if not present) +cargo install cargo-tarpaulin + +# Create temp directory for test fixtures +export TEST_FIXTURES_DIR=$(mktemp -d) +echo "Test fixtures will be in: $TEST_FIXTURES_DIR" +``` + +### Stage-by-stage implementation + +Each stage proceeds through Red-Green-Refactor, followed by validation: + +```bash +# After completing each stage, run: +cargo check # Compilation check +cargo test --lib # Unit tests +cargo test --test integration # Integration tests +make check-fmt # Format check +make lint # Lint check + +# If all pass, proceed to next stage. +# If any fails, stop and fix before proceeding. +``` + +### Coverage measurement at each stage + +```bash +# After Stage 4 (filesystem adapter) and beyond: +cargo tarpaulin --out Html --timeout 300 --exclude-files 'tests/*' +# Expected: ≥85% coverage (will increase as more stages complete) + +# Final coverage check (Stage 9): +cargo tarpaulin --out Html --timeout 300 --exclude-files 'tests/*' +# Expected: ≥90% line coverage, ≥90% branch coverage +``` + +### PR creation (final step) + +```bash +# After all stages complete and quality gates pass: +git log --oneline -10 +# Verify commits are descriptive (one per stage) + +git push -u origin 4-2-2-safe-host-mounted-workspaces + +# Create PR (use gh or GitHub web interface) +gh pr create --title "Implement safe host-mounted workspaces (task 4.2.2)" \ + --body "Closes # + +## Summary +- Implement WorkspaceMountService with hexagonal architecture +- Add validation for canonicalization, allowlist, permissions +- Add 40+ tests with ≥90% coverage +- Document threat model and configuration + +## Validation +- All quality gates pass (fmt, lint, test) +- Coverage ≥90% line and branch +- BDD scenarios pass +- Documentation updated" + +# Expected: PR created and CI checks run +``` + +## Idempotence and recovery + +All stages are idempotent: + +- **Tests are idempotent**: Run `cargo test` multiple times, expect same results +- **Code changes are additive**: Each stage adds new modules or functions; no destructive refactoring of existing code +- **Configuration is immutable at runtime**: Once loaded, config is never modified +- **Test fixtures are cleaned up**: Use `tempfile::TempDir` to auto-cleanup + +**If a stage fails partway through**: + +1. Identify which step failed (read error message) +2. Fix the issue (e.g., add missing import, fix logic) +3. Rerun the stage's validation from the beginning +4. Do not skip steps or skip commits + +**If a test fails repeatedly** (3+ attempts): + +1. Stop and escalate (per Tolerances) +2. Document the issue in `Decision Log` +3. Propose a path forward with trade-offs + +## Artifacts and notes + +### Key design decisions + +1. **Hexagonal architecture** — Isolates domain logic from I/O; enables testing without infrastructure; simplifies future enhancements (e.g., SELinux checks) + +2. **Canonicalization always rejects symlinks** — Stricter security posture; no symlinks in validated paths ever + +3. **Allowlist is configuration-based** — Operators control explicitly via YAML; validated at startup (fail-fast) + +4. **Permission validation is strict** — Read-only directories are rejected; prevents mount-time failures + +5. **Error type is exhaustive** — 10 variants, each with actionable context; all errors are domain-owned + +6. **Three-stage validation** — Canonicalize → Allowlist → Permissions; each stage is independent and testable + +7. **Shared service across launch paths** — Same WorkspaceMountServiceImpl used by HTTP, CLI, MCP; prevents validation bypass + +### Example configuration + +```yaml +podbot: + workspace_mounts: + allowed_roots: + - /tmp/workspaces + - /home/ci/work + - /var/lib/podbot/volumes +``` + +### Example error flow + +**Invalid path: symlink escape** +```rust +// Input: /tmp/workspaces/link → /etc +validate_mount("/tmp/workspaces/link") + ↓ +canonicalize() → detected symlink at "link" + ↓ +Error::SymlinkDetected { path, component: Some("link") } + ↓ +ThreatReporter::report_mount_rejection() + ↓ +Response to operator: "Mount rejected: symlink detected at /tmp/workspaces/link (component: link)" +``` + +**Invalid path: outside allowlist** +```rust +// Input: /home/attacker/workspace +// Config: allowed_roots: [/tmp/workspaces] +validate_mount("/home/attacker/workspace") + ↓ +canonicalize() → Ok(/home/attacker/workspace) + ↓ +check_in_allowlist() → not a member + ↓ +Error::NotInAllowlist { path, allowed_roots: [/tmp/workspaces] } + ↓ +Response: "Mount rejected: /home/attacker/workspace not in allowlist [/tmp/workspaces]" +``` + +--- + +## Revision note + +(None yet; to be updated if the plan is revised during implementation.) diff --git a/docs/execplans/6-2-1-write-quickstart-guide.md b/docs/execplans/6-2-1-write-quickstart-guide.md new file mode 100644 index 00000000..4eaf40cf --- /dev/null +++ b/docs/execplans/6-2-1-write-quickstart-guide.md @@ -0,0 +1,990 @@ +# Write Quickstart Guide for shared-actions (6.2.1) + +This ExecPlan (execution plan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, +and `Outcomes & Retrospective` must be kept up to date as work proceeds. + +## Status + +DRAFT + +## Purpose / big picture + +The shared-actions repository contains 17 reusable GitHub Actions focused on +Rust and Python build, test, package, and release automation. Today, new users +face a steep discovery curve: the README provides only a bare table listing +actions with no guidance on which ones to use, how they fit together, or how to +compose them into working workflows. Individual action READMEs exist but are +isolated reference docs showing single-action usage patterns, not realistic +end-to-end pipelines. + +This ExecPlan will deliver `docs/quickstart.md`, a scenario-driven onboarding +guide that bridges the gap between the action table and deep reference +documentation. After following this guide, a user new to shared-actions will be +able to: + +1. Understand what these actions do and which ones solve their problem + (within 5 minutes of reading) +2. See a complete, runnable workflow example for their use case (Rust + build+release, Python PyPI publish, coverage measurement, or Dependabot + auto-merge) +3. Know how to customize the example for their project and where to find + deeper docs +4. Understand common pitfalls and security considerations when using GitHub + Actions + +The guide will be validated by running all provided YAML examples through the +project's existing action-validator tool and composability smoke tests to +ensure they produce expected outputs and work end-to-end. + +## Constraints + +Hard invariants that must hold throughout implementation. + +- **Scope:** The guide must remain a quickstart (max 1,200 lines including + YAML examples). Complex topics (Cargo configuration, nFPM templating, + platform-specific cross-compilation) link to existing deep-dive + documentation rather than being explained in full. + +- **Content source:** Every action mentioned in the guide already exists in + this repository. No new actions are created as part of this task. All + examples derive from or are validated against existing action tests and + documented workflows (`.github/workflows/ci.yml`, test fixtures). + +- **Linking over duplication:** When explaining concepts (composite actions, + reusable workflows, caching, GitHub token permissions), link to existing + reference docs (AGENTS.md, developers-guide.md, individual action READMEs) + rather than duplicating explanation. + +- **Single source of truth:** The master table of actions remains in the root + README.md. The quickstart guide adds narrative and examples but does not + replace that table. + +- **Action versioning:** Examples use published major-version tags for remote + usage (e.g., `owner/shared-actions/.github/actions/setup-rust@v1`) and + local-path syntax for development (`./.github/actions/setup-rust`). + Examples must match the current semantic versioning strategy documented in + AGENTS.md. + +- **Platform coverage:** Examples must be valid YAML and runnable on at least + ubuntu-latest. Platform-specific gotchas (Windows GUI tools, macOS + cross-compilation, Linux systemd services) must be explicitly noted or + omitted. + +- **No breaking changes:** Implementing this guide must not require changes to + existing actions, workflows, or tooling. If such changes are discovered to + be necessary, stop and escalate. + +## Tolerances (exception triggers) + +Thresholds that trigger escalation when breached. + +- **Scope creep:** If the guide grows beyond 1,200 lines (excluding code + examples) or requires adding more than 4 complete YAML scenario examples, + stop and escalate. + +- **Example validation failure:** If any provided YAML example fails + `action-validator` or would fail on a representative CI runner + (ubuntu-latest), stop with error logs and escalate. + +- **Design inconsistencies:** If writing examples reveals that following the + guide's recommended action sequence would fail at runtime on a specific OS + or with specific inputs (even if no documentation contradiction exists), + file an issue with a minimal reproduction and escalate. Do not work around + the inconsistency with undocumented workarounds. + +- **Documentation conflicts:** If two action READMEs contradict each other + (e.g., conflicting input defaults, incompatible output formats), file a + GitHub issue and escalate rather than proceeding. + +- **Ambiguous target audience:** If it becomes unclear whether the guide + should prioritize Rust users, Python users, or equal coverage, escalate. + The current decision is: Rust-primary, with Python as a secondary example, + since shared-actions is primarily a Rust tooling library. + +- **Missing testing infrastructure:** If the guide requires inventing new test + fixtures, CI workflows, or smoke-test infrastructure not already present in + the repository, escalate. + +- **Scope definition:** The quickstart will include full, runnable YAML + examples for exactly these actions: (1) `setup-rust`, (2) + `rust-build-release`, (3) one platform packager (`linux-packages` + preferred), (4) `release-to-pypi-uv`. All other actions receive 1-line + descriptions + links to individual READMEs. Deviations from this list + trigger scope-creep escalation. + +## Risks + +Known uncertainties that might affect the plan. + +- **Risk:** Examples become stale as actions evolve. + Severity: high + Likelihood: high + Mitigation: Cross-reference each example against corresponding `action.yml` + files. Add comments pinpointing exact input versions. Include a validation + date in the guide (e.g., "validated against shared-actions v1.2.3, + 2026-06-18"). Establish a quarterly review cadence to refresh examples + against the latest action defaults. + +- **Risk:** Action interdependencies are more complex than documented. + Severity: high + Likelihood: high + Mitigation: Before writing examples, create a dependency matrix + distinguishing: (a) always-required (e.g., `setup-rust` before compilation), + (b) conditionally-required (e.g., `linux-packages` only for release builds), + (c) optional (e.g., coverage generation). Document this matrix upfront in + the guide to set correct expectations. + +- **Risk:** Examples work on ubuntu-latest but fail on macOS or Windows + runners. + Severity: medium + Likelihood: medium + Mitigation: Validate Rust examples on ubuntu-latest and macos-latest if + feasible. For platform-specific actions (e.g., `windows-package`), test + locally or note the limitation. Clearly annotate platform-specific sections + in the guide. + +- **Risk:** First-time users are confused by the difference between actions + (reusable steps) and reusable workflows (full workflow files). + Severity: medium + Likelihood: medium + Mitigation: Include a brief comparison section early in the guide. Link to + existing doc: docs/composite-actions-vs-full-workflows.md. + +- **Risk:** Users skip the quickstart and go directly to individual action + READMEs, missing the narrative context. + Severity: low + Likelihood: medium + Mitigation: Add a prominent "Start here" link in the root README.md pointing + to the new quickstart. + +- **Risk:** Glossary terms are unfamiliar to GitHub Actions newcomers. + Severity: low + Likelihood: medium + Mitigation: Include a "Glossary" section at the end of the guide. Define: + composite action, reusable workflow, runner, sccache, nextest, nFPM, + artefacts, staging. + +## Progress + +Use a list with checkboxes to summarise granular steps. Every stopping point +must be documented here. + +- [ ] (TBD) Phase 1: Research & scope clarification + - [ ] Confirm Rust-primary vs equal coverage decision + - [ ] Create dependency matrix for actions + - [ ] Finalize file path and location (docs/quickstart.md vs + docs/getting-started.md) + - [ ] Define maintenance owner and update process + +- [ ] (TBD) Phase 2: Outline & structure + - [ ] Draft the guide outline section by section + - [ ] Select 3–4 representative scenarios (Rust build, Python release, + coverage, auto-merge) + - [ ] Create a spreadsheet of action categories and dependencies + - [ ] Identify which existing docs to link vs which content to inline + +- [ ] (TBD) Phase 3: Content creation + - [ ] Write hero section and prerequisites + - [ ] Write "What's Inside" section with category overview + - [ ] Write Scenario A: Rust binary build + release (with full YAML) + - [ ] Write Scenario B: Python package to PyPI (with full YAML) + - [ ] Write Scenario C: Coverage measurement (with example) + - [ ] Write Scenario D: Dependabot auto-merge (with link to workflow) + - [ ] Write "Understanding Action Dependencies" section + - [ ] Write "Common Patterns" section + - [ ] Write "Troubleshooting" section + - [ ] Write "Next Steps" section + - [ ] Add "Glossary" section + +- [ ] (TBD) Phase 4: Validation & testing + - [ ] Run all YAML examples through `make lint` (action-validator) + - [ ] Verify all internal doc links (to AGENTS.md, developers-guide.md, etc.) + - [ ] Create minimal smoke-test workflows for each scenario + - [ ] Validate on ubuntu-latest runner + - [ ] Peer review for clarity and accuracy + - [ ] Ensure consistent tone and voice + +- [ ] (TBD) Phase 5: Integration & merge + - [ ] Update root README.md to link to quickstart + - [ ] Add quickstart link to AGENTS.md "Next Steps" + - [ ] Add maintenance notes to execplan Outcomes section + - [ ] Create draft PR with comment linking to this execplan + - [ ] Incorporate review feedback + - [ ] Merge with all CI gates passing + +## Surprises & discoveries + +Unexpected findings during implementation that were not anticipated as risks. To +be updated as work proceeds. + +(No surprises yet; implementation has not begun.) + +## Decision log + +Record every significant decision made while working on the plan. + +- **Decision:** Prioritize Rust-primary content with Python as secondary +example. + Rationale: The action catalog contains 16 Rust-specific actions and only 1 + Python-specific action (release-to-pypi-uv). A guide treating both equally + would create asymmetric expectations. Python users can reference external PyPI + docs; Rust users have minimal alternatives within shared-actions. + Date/Author: 2026-06-18 / execplan research phase. + +- **Decision:** Limit examples to 4 actions (setup-rust, rust-build-release, +linux-packages, release-to-pypi-uv). + Rationale: Prevents scope creep. All other actions receive mention + link. A + user seeking deeper guidance on a specific action reads that action's + individual README. + Date/Author: 2026-06-18 / expert review gap #3. + +- **Decision:** Create dependency matrix before writing examples. + Rationale: Addresses expert review gap #1. Actions have conditional + dependencies (setup-rust is optional if user already has Rust; linux-packages + is only needed for release builds). A matrix clarifies when each action is + required vs optional. + Date/Author: 2026-06-18 / expert review gap #1. + +- **Decision:** Assign explicit maintenance owner. + Rationale: Addresses expert review gap #6. Guide must be kept current as + actions evolve. Owner is responsible for quarterly review and updating + examples when action defaults change materially. + Date/Author: 2026-06-18 / expert review gap #6. + +- **Decision:** Use `docs/quickstart.md` as the filename. + Rationale: Consistent with GitHub's documentation convention. Discoverable + from docs/ directory and linkable from README.md. + Date/Author: 2026-06-18 / expert review gap #6. + +(Additional decisions to be recorded as implementation proceeds.) + +## Outcomes & retrospective + +To be updated at major milestones or completion. Compare result against purpose +and note lessons learned. + +(Outcomes to be recorded upon completion.) + +## Context and orientation + +The shared-actions repository is a GitHub-hosted collection of 17 reusable +GitHub Actions and 1 reusable workflow, focused on automating build, test, +package, and release workflows for Rust and Python projects. + +### Key files + +- **README.md** (root): Master table of 17 actions with major version and + path. This is the entry point users see. +- **docs/developers-guide.md**: Internal architecture for contributors. Covers + concurrency assumptions, venv management, caching strategies. +- **docs/composite-actions-vs-full-workflows.md**: Explains when to use actions + vs workflows. +- **docs/generate-coverage-design.md**: Deep dive into coverage tooling + (slipcover, pytest-xdist). +- **docs/rust-build-release-pipeline.md**: Detailed guide to Rust + build/package/release sequencing. +- **AGENTS.md**: Foundational constraints, tool resolution, CI/CD strategies. +- **.github/actions/**: Directory containing 17 action subdirectories, each + with action.yml and README.md. +- **.github/workflows/**: CI workflows (ci.yml, dependabot-automerge.yml) that + exemplify action usage. +- **Makefile**: Contains targets like `fmt`, `lint`, `test`. The `lint` target + runs action-validator. + +### Key actions for this guide + +1. **setup-rust**: Installs Rust toolchain, cargo-binstall, optional DB dev + libraries, cross-compilers. +2. **rust-build-release**: Builds Rust binaries with configurable features and + targets. Outputs staging paths for packagers. +3. **linux-packages**: Creates .deb and .rpm packages using nFPM from staged + binaries. +4. **release-to-pypi-uv**: Publishes Python packages to PyPI using uv. +5. **generate-coverage**: Measures code coverage with slipcover (Python) or + cargo-tarpaulin (Rust). +6. **dependabot-automerge**: Reusable workflow for automatically merging + Dependabot PRs. + +### Current documentation gaps + +- No narrative guide showing how to compose actions into a realistic + end-to-end workflow. +- No clear decision tree for users ("Am I building a Rust binary? A Python + package? Both?"). +- Examples exist in ci.yml but are not surfaced or explained to new users. +- Individual action READMEs are reference docs, not tutorials. + +### Target audience + +- First-time users of GitHub Actions or shared-actions specifically (estimated + background: basic GitHub familiarity). +- Maintainers of mono-repositories needing templated CI/CD. +- DevOps engineers building composite workflows. +- Project owners evaluating whether shared-actions fits their pipeline. + +### Success criteria (observable) + +1. A new user can read the quickstart in ≤10 minutes and understand: what these +actions do, which ones solve their problem, how to use a chosen action in their +workflow. +2. At least one complete Rust example (setup-rust → rust-build-release → +linux-packages → release asset upload) is provided and validated. +3. At least one Python example (release-to-pypi-uv) is provided and validated. +4. All YAML examples pass `make lint` (action-validator). +5. All internal doc links are correct. +6. A peer reviewer unfamiliar with shared-actions can follow the guide without +external context. + +## Plan of work + +The work consists of five sequential phases. Each phase has defined go/no-go +validation before proceeding to the next. + +### Phase 1: Research & Scope Clarification (2 hours) + +**Goal:** Resolve the 6 gaps identified in expert review before writing begins. + +### Phase 1 Steps + +1. Create a dependency matrix distinguishing: + - Always-required actions (e.g., setup-rust before rust-build-release) + - Conditionally-required actions (e.g., linux-packages only for release + builds with packaging) + - Optional actions (e.g., generate-coverage can run independently) + + Document this matrix in a plaintext file (e.g., + `docs/execplans/6-2-1-dependency-matrix.txt`) so it's visible during + implementation. + +2. Finalize the 4-action scope: setup-rust, rust-build-release, linux-packages, +release-to-pypi-uv. Confirm this list aligns with the author's intent. If not, +escalate. + +3. Decide on file location and hierarchy. File will be `docs/quickstart.md`. +Confirm this path and update this execplan if different. + +4. Identify maintenance owner (a person or team responsible for quarterly review +and updates). Document in execplan Outcomes section once identified. + +5. Review the four scenarios and their representative use cases: + - Scenario A (Rust build + release): for maintainers releasing Rust binaries + - Scenario B (Python PyPI): for maintainers publishing Python packages + - Scenario C (Coverage): for any project measuring code coverage + - Scenario D (Dependabot auto-merge): for any project using Dependabot + +### Phase 1 Go/no-go gate + +- All 5 items above are completed and documented. +- Author confirms the scope and 4-action list. +- If any item is ambiguous, escalate for clarification before proceeding to +Phase 2. + +**Artifacts:** dependency-matrix.txt, updated execplan Decision Log. + +--- + +### Phase 2: Outline & Structure (2 hours) + +**Goal:** Draft the guide structure without writing full prose, ensuring +coverage and flow. + +### Phase 2 Steps + +1. Create a detailed outline of all sections: + - Hero section (1-liner, audience, key features) + - Prerequisites (GitHub Actions knowledge, repo setup) + - What's Inside (category overview, action selector) + - Scenario A outline (Rust build + release) + - Scenario B outline (Python PyPI) + - Scenario C outline (Coverage) + - Scenario D outline (Dependabot auto-merge) + - Understanding Action Dependencies (matrix summary) + - Common Patterns (matrix builds, conditional steps, caching) + - Troubleshooting (3–5 common issues) + - Next Steps (links to deeper docs) + - Glossary (5–8 key terms) + +2. For each scenario, identify: + - The exact actions to show (which of the 4 core actions apply) + - The YAML file path and snippet to provide + - Links to existing docs that provide deeper explanation + - Expected output or success criteria + +3. Create a spreadsheet mapping: + - Use case → Actions involved → Individual README links → Where in guide + +4. Identify all internal doc links (AGENTS.md, developers-guide.md, individual +action READMEs, etc.) and verify they exist. + +5. Draft section headers and sub-headers in a skeleton .md file. + +### Phase 2 Go/no-go gate + +- Outline is complete with all sections clearly named. +- All internal doc links are verified to exist. +- The 4-action scope is reflected consistently in the outline. +- Author reviews outline and confirms structure. + +**Artifacts:** Skeleton `docs/quickstart.md` with headers only; spreadsheet of +action mappings. + +--- + +### Phase 3: Content Creation (4–5 hours) + +**Goal:** Write all narrative and code sections, validating YAML syntax as you +go. + +### Phase 3 Steps + +1. **Hero & Prerequisites section** (30 min): + - Write 1-line description: "Reusable GitHub Actions for Rust and Python + projects." + - Explain who this guide is for. + - List 3–4 key features (build, test, package, release). + - Explain prerequisites (GitHub Actions familiarity, .github/workflows/ + directory structure). + +2. **What's Inside section** (30 min): + - Provide a category-based overview (Build, Test, Package, Release, + Utilities). + - Create a small table mapping categories to representative actions. + - Add a decision tree: "Are you building Rust, Python, or both? → Go to + Scenario A/B/C/D." + +3. **Scenario A: Rust Binary Build + Release** (60 min): + - Write prose explaining the use case. + - Provide a minimal workflow.yml YAML snippet (checkout → setup-rust → + rust-build-release → linux-packages → upload-release-assets). + - Annotate the YAML with comments explaining each step. + - Show typical input values and how to customize. + - Explain the output (staging directory, release assets). + - Link to individual action READMEs for deeper customization. + - **Validation**: Run this YAML through `make lint` (action-validator) and + verify it passes. + +4. **Scenario B: Python Package to PyPI** (30 min): + - Write prose explaining the use case. + - Provide a minimal workflow.yml snippet (checkout → release-to-pypi-uv). + - Annotate with comments. + - Explain how to set up PyPI credentials/token. + - Link to action README and external PyPI docs. + - **Validation**: Run YAML through `make lint` and verify it passes. + +5. **Scenario C: Coverage Measurement** (30 min): + - Explain the use case (measure code coverage across Rust and Python). + - Provide a workflow snippet showing generate-coverage with lang=rust and + lang=python in separate jobs. + - Link to docs/generate-coverage-design.md for deep dive. + - **Validation**: Run YAML through `make lint`. + +6. **Scenario D: Dependabot Auto-Merge** (20 min): + - Explain the reusable workflow pattern. + - Show how to reference the workflow in a user's repository. + - Link to docs/composite-actions-vs-full-workflows.md and the + dependabot-automerge.yml workflow file. + - No new YAML needed (workflow already exists in repo). + +7. **Understanding Action Dependencies section** (20 min): + - Present the dependency matrix in prose form (not a table; use bullet + points). + - Example: "setup-rust must run before rust-build-release, but is optional if + your runner already has Rust installed." + - Explain sequencing rules and when actions can run in parallel. + - Reference the matrix created in Phase 1. + +8. **Common Patterns section** (20 min): + - Show: local vs remote usage (`./.github/actions/setup-rust` vs + `owner/shared-actions/.github/actions/setup-rust@v1`). + - Show: matrix builds across platforms. + - Show: conditional step execution. + - Link to developers-guide.md for caching details. + - **Note:** Keep these as brief examples; deeper guidance lives in linked + docs. + +9. **Troubleshooting section** (20 min): + - Provide 3–5 common issues and resolutions: + 1. "My build failed in the action but worked locally" → Explain runner + differences, link to act documentation. + 2. "How do I debug an action step?" → Explain GitHub Actions debug logs and + ACTIONS_STEP_DEBUG. + 3. "Can I use just one of these actions without the others?" → Yes, they're + composable; show cherry-pick example. + 4. "My Dependabot PR didn't auto-merge" → Check permissions, token, branch + protection rules. + 5. "I need to package for Windows, but windows-package is v0" → Explain + versioning strategy, point to action README. + - Link to local-validation-of-github-actions-with-act-and-pytest.md. + +10. **Next Steps section** (15 min): + - Link to full AGENTS.md for comprehensive action list. + - Link to developers-guide.md for architecture deep-dives. + - Link to individual action READMEs. + - Link to GitHub Actions official documentation. + - Contributing guidelines. + +11. **Glossary section** (15 min): + - Define 5–8 terms: + - Composite action + - Reusable workflow + - Runner + - sccache + - nextest + - nFPM + - Artefacts (staging) + - Slipcover + +### Phase 3 Go/no-go gate + +- All prose sections are written. +- All YAML examples pass `make lint` (action-validator). +- All internal doc links are verified. +- No section is missing. + +**Artifacts:** Complete `docs/quickstart.md` file. + +--- + +### Phase 4: Validation & Testing (2–3 hours) + +**Goal:** Ensure all examples are correct, testable, and discoverable. + +### Phase 4 Steps + +1. **YAML Validation** (30 min): + - Run `make lint` from repository root. + - Verify all YAML snippets in the guide pass action-validator. + - Fix any indentation, syntax, or reference errors. + +2. **Link Verification** (30 min): + - Verify all internal links (to AGENTS.md, developers-guide.md, individual + action READMEs, etc.) exist and are correct. + - Use grep or a link checker to catch broken paths. + - Example: `grep -n "AGENTS.md" docs/quickstart.md` should return valid file + references. + +3. **Composability Smoke Tests** (60 min): + - For Scenario A (Rust build + release): Create a minimal test workflow that + runs setup-rust → rust-build-release with a fixture Rust project. + - For Scenario B (Python PyPI): Create a test workflow that runs + release-to-pypi-uv with a mock PyPI endpoint or dry-run flag (if supported). + - For Scenario C (Coverage): Create a test workflow that runs + generate-coverage with both lang=rust and lang=python. + - Run each test workflow locally with `act` or in CI. + - Verify that actions produce expected outputs and no runtime errors occur. + +4. **Platform Coverage** (30 min): + - Validate Rust examples on ubuntu-latest (minimum). + - If feasible, also test on macos-latest. + - Document any platform-specific gotchas in the guide. + +5. **Peer Review** (30 min): + - Have a maintainer unfamiliar with the actions read the guide. + - Gather feedback on clarity, completeness, and tone. + - Flag any sections that are confusing or require external context. + - Incorporate feedback. + +### Phase 4 Go/no-go gate + +- All YAML passes `make lint`. +- All links are correct. +- Smoke tests run without errors on ubuntu-latest. +- Peer review is complete with no major clarity issues. + +**Artifacts:** Validated `docs/quickstart.md`, smoke-test workflow files, peer +review notes. + +--- + +### Phase 5: Integration & Merge (1 hour) + +**Goal:** Finalize the guide, update root README, and merge to main branch. + +### Phase 5 Steps + +1. **Update root README.md** (15 min): + - Add a link in the README.md hero or "Next Steps" section pointing to + docs/quickstart.md. + - Example: "New to shared-actions? Start with the [Quickstart + Guide](docs/quickstart.md)." + +2. **Update AGENTS.md** (5 min): + - Add a link to docs/quickstart.md in the "Getting Started" or + "Documentation" section. + +3. **Document maintenance** (10 min): + - Update this execplan's Outcomes section with: + - Name and GitHub handle of maintenance owner. + - Quarterly review schedule. + - Process for updating examples when action defaults change. + +4. **Create draft PR** (15 min): + - Commit changes (docs/quickstart.md, updated README.md, updated AGENTS.md). + - Push to branch `6-2-1-write-quickstart-guide`. + - Create a draft PR with title: "(6.2.1) Write Quickstart Guide for + shared-actions". + - In PR description, link to this execplan and summarize what was delivered. + - Include a "References" section with the lody session link. + +5. **Merge with gates passing** (15 min): + - Ensure all CI/CD gates pass (lint, typecheck, tests if any). + - Mark PR as ready for review. + - Await approval and merge to main. + +### Phase 5 Go/no-go gate + +- All phases 1–4 are complete and validated. +- PR is open and all CI gates pass. + +**Artifacts:** Merged docs/quickstart.md, updated README.md and AGENTS.md, +merged PR. + +## Concrete steps + +**Working directory:** `/tmp/lody-title-agent` (the shared-actions repository). + +### Phase 1: Research & Scope Clarification + +```bash +# Step 1: Create dependency matrix +cat > docs/execplans/6-2-1-dependency-matrix.txt << 'EOF' +ACTION DEPENDENCY MATRIX + +Always-required (must run before dependent actions): +- setup-rust → rust-build-release (setup-rust installs Rust; + rust-build-release requires it) + +Conditionally-required (needed only for specific workflows): +- rust-build-release → linux-packages (only if you want to create .deb/.rpm packages) +- rust-build-release → macos-package (only if you want to create macOS .pkg) +- rust-build-release → windows-package (only if you want to create Windows .msi/.zip) +- Any build action → generate-coverage (only if measuring coverage) +- generate-coverage → ... (optional; produces artifacts but doesn't + affect other actions) + +Optional (can run independently): +- setup-rust (if runner already has Rust, can skip) +- release-to-pypi-uv (independent Python workflow; does not require any Rust actions) +- dependabot-automerge (separate reusable workflow; does not depend on build actions) + +Platform-specific: +- linux-packages requires ubuntu-latest runner +- macos-package requires macos-latest runner +- windows-package requires windows-latest runner +EOF + +# Verify it was created +ls -l docs/execplans/6-2-1-dependency-matrix.txt +echo "Matrix created." +``` + +Expected output: + +```text +-rw-r--r--. 1 leynos leynos 822 Jun 18 xx:xx docs/execplans/6-2-1-dependency-matrix.txt +Matrix created. +``` + +**Go/no-go:** Proceed to Phase 2 if all 5 items from Phase 1 steps are completed +and documented in this execplan's Decision Log. + +--- + +### Phase 2: Outline & Structure + +```bash +# Create skeleton docs/quickstart.md with headers only +cat > docs/quickstart.md << 'EOF' +# Quickstart Guide + +## Hero Section + +## Prerequisites + +## What's Inside + +### Action Categories + +### How to Choose Your Path + +## Scenarios + +### Scenario A: Build and Release a Rust Binary + +### Scenario B: Publish a Python Package to PyPI + +### Scenario C: Measure Code Coverage + +### Scenario D: Automate Dependabot Merges + +## Understanding Action Dependencies + +## Common Patterns + +## Troubleshooting + +## Next Steps + +## Glossary +EOF + +echo "Skeleton created at docs/quickstart.md" + +# Verify links to existing docs +ls -1 docs/{developers-guide,composite-actions,generate-coverage}*.md | head -5 +``` + +Expected output: + +```text +Skeleton created at docs/quickstart.md +docs/composite-actions-vs-full-workflows.md +docs/developers-guide.md +docs/generate-coverage-design.md +``` + +**Go/no-go:** Skeleton exists with all section headers. Outline is complete. +Author confirms structure before proceeding. + +--- + +### Phase 3: Content Creation + +This phase involves writing prose for each section. Example validation during +Scenario A: + +```bash +# After writing Scenario A YAML, extract and validate it +grep -A 50 "Scenario A:" docs/quickstart.md | grep -A 20 '```yaml' > /tmp/scenario-a.yml + +# Run through action-validator +make lint 2>&1 | tee /tmp/lint-output.txt | head -20 +``` + +Expected output if no errors: + +```text +[OK] All actions validated successfully. +``` + +If errors occur, fix YAML indentation and rerun until passing. + +--- + +### Phase 4: Validation & Testing + +```bash +# Verify all internal links exist +for link in AGENTS.md developers-guide.md \ + composite-actions-vs-full-workflows.md generate-coverage-design.md; do + if grep -q "$link" docs/quickstart.md; then + if [ -f "docs/$link" ] || [ -f "$link" ]; then + echo "✓ $link exists" + else + echo "✗ $link NOT FOUND" + fi + fi +done +``` + +Expected output: + +```text +✓ AGENTS.md exists +✓ developers-guide.md exists +✓ composite-actions-vs-full-workflows.md exists +✓ generate-coverage-design.md exists +``` + +--- + +### Phase 5: Integration & Merge + +```bash +# Add link to root README.md (in "Next Steps" or "Getting Started" section) +# This is a manual edit; update the appropriate section with: +# "New to shared-actions? Start with the [Quickstart Guide](docs/quickstart.md)." + +# Commit changes +git add docs/quickstart.md docs/README.md AGENTS.md +git commit -m "Add quickstart guide for shared-actions (6.2.1) + +Provides scenario-driven onboarding for new users, including: +- Rust binary build and release example +- Python PyPI publishing example +- Coverage measurement example +- Dependabot auto-merge walkthrough + +All examples are validated against action.yml files and pass action-validator." + +# Push and create PR +git push -u origin 6-2-1-write-quickstart-guide +``` + +Expected: Commit message, successful push, PR created. + +--- + +## Validation and acceptance + +### Observable behaviour + +After following the guide, a new user should be able to: + +1. **Understand the purpose:** Run through "What's Inside" section and correctly +identify that setup-rust + rust-build-release is the path for Rust binary +projects. + +2. **Run a complete example:** Copy the Scenario A YAML into their repository at +`.github/workflows/release.yml`, adjust project-specific fields (binary name, +target platforms), push to GitHub, and see a successful build and release +workflow execute. + +3. **Understand dependencies:** Read the "Understanding Action Dependencies" +section and predict which actions must run in sequence and which are optional. + +4. **Find deeper docs:** Locate links to individual action READMEs and +architectural deep-dives for topics they want to customize (e.g., Cargo +features, nFPM configuration). + +### Quality criteria + +### Content + +- [ ] Guide is between 600–1,200 lines (excluding code examples). +- [ ] Includes at least 4 distinct use-case scenarios (Rust, Python, coverage, + Dependabot). +- [ ] All YAML examples pass `make lint`. +- [ ] All internal doc links are correct and point to existing files. +- [ ] Glossary covers at least 5 key terms. + +### Examples + +- [ ] Each YAML example is copy-paste runnable (no manual edits beyond + project-specific fields like binary name). +- [ ] Examples are validated against corresponding action.yml files and + existing workflows (ci.yml). +- [ ] At least one Rust example and one Python example are present. +- [ ] Examples include realistic input values and comments explaining how to + customize. + +### Validation + +- [ ] `make lint` passes (no markdown or YAML errors). +- [ ] Peer review by a maintainer unfamiliar with shared-actions is complete + with no unresolved clarity issues. +- [ ] No broken internal or external links. +- [ ] Platform-specific notes (if any) are clearly marked. + +### Integration + +- [ ] Root README.md updated to link to quickstart. +- [ ] AGENTS.md updated to reference quickstart in "Getting Started" or + similar section. +- [ ] New file is discoverable (linked from README.md and AGENTS.md). +- [ ] All CI/CD gates pass with the new file added. + +### Commands to verify success + +```bash +# Verify file exists and has content +wc -l docs/quickstart.md +# Expected: > 600 lines + +# Verify YAML examples pass validation +make lint +# Expected: All checks pass + +# Verify links are correct +grep -o '\[.*\](.*\.md)' docs/quickstart.md | head -5 +# Expected: Links to AGENTS.md, developers-guide.md, etc. + +# Verify GitHub Actions syntax +grep -c '```yaml' docs/quickstart.md +# Expected: >= 3 (at least 3 YAML examples) +``` + +### Acceptance definition + +Delivery is complete when: + +1. `docs/quickstart.md` exists with 600–1,200 lines of content. +2. All YAML examples pass `make lint` without errors. +3. Peer review is complete and no major clarity issues remain. +4. Root README.md and AGENTS.md are updated with links to the quickstart. +5. All CI/CD gates pass. +6. A draft PR is open with a reference to this execplan and the lody session. + +## Idempotence and recovery + +All steps in this plan are idempotent. If a phase fails or is incomplete: + +- **Phase 1–2:** Delete and recreate the files (dependency-matrix.txt, skeleton +docs/quickstart.md). No external systems are affected. + +- **Phase 3:** Rewrite the prose or YAML sections. Validation in Phase 4 will +catch errors. + +- **Phase 4:** Re-run validation steps. Link checks and YAML validation are +re-runnable. + +- **Phase 5:** If commit or push fails, fix the issue and retry. If PR creation +fails, create manually. + +If a tolerance threshold is breached (e.g., YAML fails validation), stop +immediately and escalate rather than working around the error. + +## Artifacts and notes + +Key artifacts produced: + +1. **docs/quickstart.md** — The main deliverable. Scenario-driven guide with 4 +examples, ~800–1,000 lines. + +2. **docs/execplans/6-2-1-dependency-matrix.txt** — Clarity on which actions are +always-required vs optional. + +3. **Updated README.md** — Added link to quickstart. + +4. **Updated AGENTS.md** — Added reference in "Getting Started" section. + +5. **Draft PR** — (6.2.1) Write Quickstart Guide for shared-actions, linking to +this execplan. + +## Interfaces and dependencies + +### External dependencies + +- GitHub Actions: Users' workflows will `uses` the actions defined in + `.github/actions/`. +- Documentation: Links to existing docs (AGENTS.md, developers-guide.md, + individual action READMEs). + +No new interfaces or dependencies are created by this plan. + +### Validation tools + +- `make lint` — Runs action-validator to check YAML syntax. +- `act` (local execution tool) — Optional, for smoke testing workflows + locally before CI. +- `grep` / link checkers — For validating documentation links. + +--- + +## Status: DRAFT + +This ExecPlan is ready for review and approval. It addresses the 6 expert review +gaps and provides a clear roadmap for writing a scenario-driven quickstart guide +that reduces friction for new users of shared-actions while preserving deep +reference documentation for advanced customization. + +To proceed, the user must: + +1. Review this execplan and approve or request revisions. +2. Confirm the 4-action scope and Rust-primary positioning. +3. Identify the maintenance owner for quarterly reviews. +4. Approve proceeding to Phase 1. + +Upon approval, implementation will proceed milestone-by-milestone with clear +go/no-go gates at each phase. diff --git a/docs/execplans/auto-merge-on-unstable.md b/docs/execplans/auto-merge-on-unstable.md index 5428e749..ccc61fbe 100644 --- a/docs/execplans/auto-merge-on-unstable.md +++ b/docs/execplans/auto-merge-on-unstable.md @@ -1,8 +1,8 @@ # Enable Dependabot Auto-Merge When Merge State Is UNSTABLE -This execution plan (ExecPlan) is a living document. The sections `Constraints`, `Tolerances`, -`Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, and -`Outcomes & Retrospective` must be kept up to date as work proceeds. +This execution plan (ExecPlan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, +and `Outcomes & Retrospective` must be kept up to date as work proceeds. Status: COMPLETE @@ -16,9 +16,10 @@ the workflow enables GitHub auto-merge earlier (while checks are pending or not yet green), and GitHub then performs the final merge only after branch protection and required checks are satisfied. -Success is observable in workflow logs: an eligible Dependabot pull request (PR) with -`automerge_merge_state=UNSTABLE` should now emit `automerge_status=enabled` -instead of `automerge_status=skipped` with `merge-state-unstable`. +Success is observable in workflow logs: an eligible Dependabot pull request +(PR) with `automerge_merge_state=UNSTABLE` should now emit +`automerge_status=enabled` instead of `automerge_status=skipped` with +`merge-state-unstable`. ## Constraints @@ -94,9 +95,9 @@ silently broadening scope. - Discovery: The current repository has no `PLANS.md`, so this ExecPlan is governed by the `execplans` skill format and `AGENTS.md` instructions. -- Discovery: Qdrant memory Model Context Protocol (MCP) tools (`qdrant-find`, `qdrant-store`) are not - exposed in this execution environment; no remote project-memory lookup was - possible from this session. +- Discovery: Qdrant memory Model Context Protocol (MCP) tools (`qdrant-find`, + `qdrant-store`) are not exposed in this execution environment; no remote + project-memory lookup was possible from this session. - Discovery: The existing unit suite already has a dedicated test asserting `UNSTABLE` is skipped (`merge_state_unstable_skips`), which will need to be inverted rather than adding entirely new harness plumbing. diff --git a/docs/execplans/dependabot-auto-merge.md b/docs/execplans/dependabot-auto-merge.md index aa161242..87e7d46d 100644 --- a/docs/execplans/dependabot-auto-merge.md +++ b/docs/execplans/dependabot-auto-merge.md @@ -73,8 +73,10 @@ local validation path via `pytest` + `act`. - [x] (2026-01-12 00:00Z) Add unit tests for the helper script. - [x] (2026-01-12 00:00Z) Add workflow integration test via `act` harness. - [x] (2026-01-12 00:00Z) Update documentation with usage guidance. -- [x] (2026-01-12 00:00Z) Reordered dry-run validation to require event payload before PR number resolution. -- [x] (2026-01-12 00:00Z) Adjusted linux-packages worktree repo-root test to accept `.git` files. +- [x] (2026-01-12 00:00Z) Reordered dry-run validation to require event + payload before PR number resolution. +- [x] (2026-01-12 00:00Z) Adjusted linux-packages worktree repo-root test to + accept `.git` files. - [x] (2026-01-12 00:00Z) Ran format, lint, typecheck, and test gates. - [x] (2026-01-12 00:00Z) Committed implementation changes. diff --git a/docs/execplans/refactor-validate-packages-modules.md b/docs/execplans/refactor-validate-packages-modules.md index e24666e9..9679a3e2 100644 --- a/docs/execplans/refactor-validate-packages-modules.md +++ b/docs/execplans/refactor-validate-packages-modules.md @@ -6,7 +6,6 @@ and `Outcomes & Retrospective` must be kept up to date as work proceeds. Status: COMPLETE - ## Purpose / big picture Improve cohesion and maintainability of the validate-linux-packages action by @@ -17,41 +16,54 @@ versus diagnostic formatting versus path validation. The public API remains unchanged, all tests pass without modification, and imports from validate_packages continue to work via re-exports. - ## Constraints Hard invariants that must hold throughout implementation: - Public API stability: all functions currently in `__all__` of + validate_packages.py must remain importable from validate_packages after the refactoring (via re-export). + - Test compatibility: all existing tests must pass without modification. Tests + import via `validate_packages_module` fixture which loads validate_packages.py dynamically. + - Backward compatibility: any code importing from validate_packages (including + validate_cli.py and tests) must continue working without changes. + - No functional changes: this is a pure refactoring; behaviour must remain + identical. + - Type safety: all type hints must remain valid; typecheck must pass. - The duplicate `_trim_output()` function must be resolved (two definitions at - lines 148 and 385 with different signatures). + lines 148 and 385 with different signatures). ## Tolerances (exception triggers) - Scope: if refactoring requires changes to more than 7 files (5 new modules + + validate_packages.py + one test discovery), stop and escalate. + - Interface: if any function in `__all__` changes signature or behaviour, stop + and escalate. + - Dependencies: if new external dependencies are required, stop and escalate. - Iterations: if tests fail after initial refactoring and fixing imports takes + more than 3 attempts, stop and escalate. -- Time: if any module extraction takes more than 30 minutes, stop and escalate. +- Time: if any module extraction takes more than 30 minutes, stop and escalate. ## Risks - Risk: The duplicate `_trim_output()` functions have different signatures (one + at line 148 with line_limit and char_limit parameters, one at line 385 without). The second definition overrides the first, so some call sites expecting the multi-line variant may not work correctly. @@ -61,18 +73,19 @@ Hard invariants that must hold throughout implementation: that preserves the intended behaviour for all callers. - Risk: Tests use dynamic module loading via fixture; imports from new modules + might not be properly discovered. Severity: low Likelihood: low Mitigation: Verify re-exports work correctly with the fixture-based loading. - Risk: Circular import dependencies may emerge when extracting modules. + Severity: medium Likelihood: low Mitigation: Keep imports unidirectional; validate_packages imports from the new modules, not vice versa. - ## Progress - [x] (2025-02-17) Audit `_trim_output()` duplicate and resolve signature conflict. @@ -87,10 +100,10 @@ Hard invariants that must hold throughout implementation: - [x] (2025-02-17) Run `make lint` and verify no errors. - [x] (2025-02-17) Run `make check-fmt` and verify formatting is correct. - ## Surprises & discoveries - Observation: The duplicate `_trim_output()` functions serve different purposes. + The first (line 148) preserves multi-line structure with line limits, while the second (line 385) normalises to a single line for logging. Both are needed. Evidence: Call sites at lines 174, 184, 191, 283, 286 use multi-line variant @@ -99,15 +112,16 @@ Hard invariants that must hold throughout implementation: rather than delete it, preserving both behaviours. - Observation: The `_PATH_CHECK_TIMEOUT_SECONDS` constant is used in + validate_sandbox_diagnostics.py but logically belongs with path checks. Evidence: Used in `_execute_diagnostic_command()` for path diagnostic commands. Impact: Duplicated the constant in validate_sandbox_diagnostics.py to avoid circular import dependency while keeping path check timeout logic together. - ## Decision log - Decision: Rename second `_trim_output()` to `_trim_output_single_line()` instead + of consolidating into one function. Rationale: The two functions serve distinctly different purposes - multi-line diagnostic formatting vs single-line logging. Combining them would require @@ -116,6 +130,7 @@ Hard invariants that must hold throughout implementation: Date/Author: 2025-02-17 / Agent - Decision: Create modules in dependency order: formatters, diagnostics, path_checks, + arch, locators. Rationale: Avoids circular imports by ensuring each module only imports from modules created earlier or from existing validate_* modules. Formatters has no @@ -123,12 +138,14 @@ Hard invariants that must hold throughout implementation: Date/Author: 2025-02-17 / Agent - Decision: Move PolytheneSession imports to TYPE_CHECKING blocks in new modules. + Rationale: Ruff lint rule TC002 requires third-party imports used only for type hints to be in TYPE_CHECKING blocks. This reduces runtime import overhead and makes type-only dependencies explicit. Date/Author: 2025-02-17 / Agent - Decision: Keep validate_packages.py as orchestration layer only. + Rationale: Retained `_exec_with_diagnostics()`, `_install_and_verify()`, `_validate_package()`, `_MetadataValidators`, and public entry points `validate_deb_package()` and `validate_rpm_package()` in validate_packages.py. @@ -136,41 +153,48 @@ Hard invariants that must hold throughout implementation: orchestration layer. Date/Author: 2025-02-17 / Agent - ## Outcomes & retrospective Successfully refactored validate_packages.py from 734 lines to 377 lines by extracting 5 focused modules. All 635 tests pass, all commit gates pass. Modules created: + - validate_formatters.py (51 lines): Output trimming and stderr extraction - validate_arch.py (70 lines): Architecture mapping and platform detection -- validate_locators.py (49 lines): Package discovery (locate_deb, locate_rpm, ensure_subset) -- validate_sandbox_diagnostics.py (176 lines): Diagnostic collection for sandbox troubleshooting +- validate_locators.py (49 lines): Package discovery (locate_deb, locate_rpm, + ensure_subset) +- validate_sandbox_diagnostics.py (176 lines): Diagnostic collection for + sandbox troubleshooting - validate_path_checks.py (151 lines): Path validation with Python fallback logic What worked well: + - Dependency-ordered module creation prevented circular imports - Renaming `_trim_output()` variants instead of consolidating preserved clarity -- Re-exporting public functions via `__all__` in validate_packages.py maintained - backward compatibility - no test changes required +- Re-exporting public functions via `__all__` in validate_packages.py + maintained backward compatibility - no test changes required - TYPE_CHECKING blocks for type-only imports satisfied lint rules What could be improved: + - The `_PATH_CHECK_TIMEOUT_SECONDS` constant ended up duplicated between - validate_sandbox_diagnostics.py and validate_path_checks.py. A better approach - might be a shared constants module, but this would have added complexity for - marginal benefit. + validate_sandbox_diagnostics.py and validate_path_checks.py. A better + approach might be a shared constants module, but this would have added + complexity for marginal benefit. Lessons learned: + - When encountering duplicate functions, investigate usage patterns before - assuming consolidation is correct. Sometimes duplicates serve distinct purposes. + assuming consolidation is correct. Sometimes duplicates serve distinct + purposes. - Module extraction benefits from clear dependency analysis upfront. Creating a dependency graph before extraction would have made the ordering decision more obvious. + - Tests using dynamic module loading (via fixtures) work seamlessly with - refactored imports as long as re-exports are maintained. + refactored imports as long as re-exports are maintained. ## Context and orientation @@ -202,22 +226,29 @@ multi-line variant's signature (with line_limit and char_limit parameters). This must be resolved by consolidating into a single canonical implementation that preserves the intended behaviour for all call sites. - ## Plan of work Stage A: Audit and resolve the `_trim_output()` duplicate - Read lines 148-174 and 385-391 to understand both signatures and their + intended use cases. + - Search all call sites in validate_packages.py to determine which signature + each caller expects. + - Consolidate into a single canonical `_trim_output()` implementation that + preserves the multi-line variant's signature (line 148) with line_limit and char_limit parameters, since this provides the most flexibility. The single-line variant (line 385) can be expressed as a call to the multi-line variant with appropriate parameters. + - Update all call sites of the single-line variant to use the consolidated + implementation. + - Verify behaviour remains identical and tests pass before proceeding. Stage B: Create the five new modules @@ -244,8 +275,11 @@ Stage C: Update validate_packages.py - Add imports from the five new modules. - Update `__all__` to re-export public functions from new modules. - Ensure internal functions like `_exec_with_diagnostics`, + `_install_and_verify`, `_validate_package`, `_MetadataValidators` remain. + - Keep public entry points `validate_deb_package()` and + `validate_rpm_package()`. Stage D: Validation @@ -255,7 +289,6 @@ Stage D: Validation - Run `make lint` and fix any violations. - Run `make check-fmt` and fix formatting. - ## Concrete steps ### Stage A: Resolve _trim_output() duplicate @@ -284,6 +317,7 @@ Stage D: Validation Create `validate_formatters.py`: Functions to extract: + - _trim_output (line 148, multi-line variant) - _trim_output_single_line (line 385, renamed single-line variant) - _extract_process_stderr (line 166) @@ -291,6 +325,7 @@ Create `validate_formatters.py`: Create `validate_sandbox_diagnostics.py`: Functions to extract: + - _execute_diagnostic_command (line 177) - _build_path_diagnostic_commands (line 195) - _format_path_diagnostics (line 221) @@ -301,6 +336,7 @@ Create `validate_sandbox_diagnostics.py`: Create `validate_path_checks.py`: Constants and functions to extract: + - _PYTHON_FALLBACK_SCRIPT (line 27) - _PYTHON_FALLBACK_INTERPRETERS (line 30) - _PATH_CHECK_TIMEOUT_SECONDS (line 31) @@ -314,6 +350,7 @@ Create `validate_path_checks.py`: Create `validate_arch.py`: Functions to extract: + - _HOST_ARCH_ALIAS_MAP (line 57) - _host_architectures (line 72) - _should_skip_sandbox (line 81) @@ -323,6 +360,7 @@ Create `validate_arch.py`: Create `validate_locators.py`: Functions to extract: + - locate_deb (line 119) - locate_rpm (line 129) - ensure_subset (line 139) @@ -403,7 +441,6 @@ Expected output: "All checks passed!" Expected output: "173 files already formatted" - ## Validation and acceptance Quality criteria: @@ -420,7 +457,6 @@ Quality method: cd /home/user/project make check-fmt && make lint && make typecheck && make test - ## Idempotence and recovery All steps are idempotent: @@ -432,37 +468,46 @@ All steps are idempotent: Recovery: if a stage fails, revert the changes to that stage only and retry with corrections. Use git to track intermediate states. - ## Artifacts and notes (To be filled during implementation with key transcripts and observations) - ## Interfaces and dependencies New modules will have these public interfaces (exported via `__all__`): `validate_formatters.py`: + - `_trim_output(output: str, *, line_limit: int = 5, char_limit: int = 400) -> str` - `_extract_process_stderr(error: BaseException | None) -> str | None` `validate_arch.py`: + - `acceptable_rpm_architectures(arch: str) -> set[str]` - `rpm_expected_architecture(arch: str) -> str` `validate_locators.py`: -- `locate_deb(package_dir: Path, package_name: str, version: str, release: str) -> Path` -- `locate_rpm(package_dir: Path, package_name: str, version: str, release: str) -> Path` -- `ensure_subset(expected: Collection[str], actual: Collection[str], label: str) -> None` + +- `locate_deb(package_dir: Path, package_name: str, version: str, + release: str) -> Path` +- `locate_rpm(package_dir: Path, package_name: str, version: str, + release: str) -> Path` +- `ensure_subset(expected: Collection[str], actual: Collection[str], + label: str) -> None` `validate_sandbox_diagnostics.py`: + - All functions are internal (prefixed with `_`); no public exports needed. `validate_path_checks.py`: + - All functions are internal (prefixed with `_`); no public exports needed. Dependencies flow: + - validate_packages imports from all five new modules. - New modules import from existing modules (validate_exceptions, + validate_helpers, validate_metadata, validate_polythene). + - No circular dependencies. diff --git a/docs/rfc-ortho-config-env-aliases.md b/docs/rfc-ortho-config-env-aliases.md new file mode 100644 index 00000000..97f40a1b --- /dev/null +++ b/docs/rfc-ortho-config-env-aliases.md @@ -0,0 +1,752 @@ +# RFC: Environment Variable Aliases for ortho-config + +## Title + +Environment Variable Aliases for ortho-config: Support Dynamic Aliasing and +Precedence-Based Fallback + +## Status + +Draft + +## Date + +2026-06-18 + +## Summary + +This RFC proposes adding support for environment variable aliases in +ortho-config, allowing multiple names to resolve to the same configuration +parameter. The design supports both compile-time alias declaration (via +procedural derive macros) and runtime alias registration, with a clear +precedence model that respects canonical names over aliases. This feature +enables smoother migration for projects like vk and podbot that need to support +legacy variable names while standardizing on canonical ones. + +## Motivation + +### Use Case: vk + +The vk project maintains configuration across multiple deployment environments. +As vk evolves, environment variable naming conventions may change or +consolidate. For example: + +- Old convention: `VK_DATABASE_HOST`, `VK_DB_HOST` (inconsistent naming) +- New convention: `VK_DATABASE_HOST` (single canonical name) + +Without alias support, vk must either: + +1. Hardcode checks for multiple variable names throughout the codebase, +creating maintenance burden +2. Perform manual pre-processing of environment variables at startup +3. Maintain two parallel configuration systems indefinitely + +With aliases, vk can declare `VK_DB_HOST` as an alias for +`VK_DATABASE_HOST`, automatically falling back to the old name when the +canonical name is unset. This enables gradual migration without code +duplication. + +### Use Case: podbot + +Podbot integrates ortho-config for multi-tenant audio processing configuration. +As the platform matures: + +- Original var names used underscores: `PODBOT_AUDIO_SAMPLE_RATE_HZ` +- Standardization effort switches to kebab-case in configuration: `podbot.audio.sample-rate-hz` +- Internal library names differ from external configuration names + +Aliases allow podbot to: + +1. Define the production canonical name in configuration +2. Declare aliases for legacy environment variables, documentation names, and +integration points +3. Support multiple deployment scenarios (legacy CI/CD, containers, Kubernetes) +without code changes + +### Core Problem + +Both projects face this pattern: environment-first systems need to support +**backward compatibility** while **migrating to new naming schemes**. Current +solutions are ad-hoc and spread throughout application code. A standardized +mechanism in ortho-config would: + +- Centralize alias definitions at the configuration struct definition site +- Enable zero-cost aliases (checked at compile time where possible) +- Provide clear semantics for precedence and fallback behavior +- Reduce maintenance burden and human error + +## Proposed Solution + +### Derive Macro Syntax + +```rust +use ortho_config::Config; + +#[derive(Config)] +#[ortho_config(env_prefix = "VK_")] +struct DbConfig { + /// Primary database hostname + #[ortho_config( + env_name = "DATABASE_HOST", + aliases = ["DB_HOST", "DBHOST"] + )] + database_host: String, + + /// Connection pool size; legacy var name still accepted + #[ortho_config( + env_name = "POOL_SIZE", + aliases = ["CONNECTION_POOL_SIZE"], + default = "10" + )] + pool_size: u32, +} +``` + +Expansion semantics: + +- `DATABASE_HOST` (canonical) is checked first +- If unset, `DB_HOST` is checked +- If unset, `DBHOST` is checked +- If still unset, the default is used + +### Runtime API + +For dynamic alias registration: + +```rust +use ortho_config::{Config, Aliases}; + +// Register aliases at startup +let aliases = Aliases::new() + .alias("PODBOT_AUDIO_SR", "PODBOT_AUDIO_SAMPLE_RATE_HZ")? + .alias("LEGACY_MODE", "PODBOT_MODE")?; + +// When creating config, provide aliases +let config = PodBotConfig::from_env_with_aliases(&aliases)?; + +// Or use a builder pattern +let config = PodBotConfig::builder() + .with_aliases(aliases) + .from_env()?; +``` + +### Precedence Rules + +Aliases follow a strict precedence hierarchy: + +1. **Canonical environment variable** (highest priority) +2. **Aliases in declaration order** (left to right as listed in `aliases = [...]`) +3. **Default value** (if provided) +4. **Error** (if no default and no env vars set) + +Example walkthrough: + +```rust +#[ortho_config(env_name = "FOO", aliases = ["BAR", "BAZ"])] +field: String, +``` + +Resolution with environment state: + +| State | Resolved Value | Source | +| -------------------------- | -------------- | --------------------- | +| `FOO=a, BAR=b, BAZ=c` | `a` | Canonical (highest) | +| `BAR=b, BAZ=c` | `b` | First alias | +| `BAZ=c` | `c` | Second alias | +| `(none)` + `default="d"` | `d` | Default value | +| `(none)` + no default | Error | No source found | + +## Detailed Design + +### Macro Levels + +#### Compile-Time Macro Expansion + +The `#[ortho_config(...)]` attribute macro generates code that: + +1. **Statically generates the resolution chain** during compilation +2. **Unrolls the precedence checks** into individual environment lookups +3. **Emits constants** listing all possible names for documentation/debugging + +Example generated code: + +```rust +// Original: +#[ortho_config(env_name = "DATABASE_HOST", aliases = ["DB_HOST"])] +database_host: String, + +// Generates approximately: +const DATABASE_HOST_NAMES: &[&str] = &["DATABASE_HOST", "DB_HOST"]; + +// Within from_env() expansion: +let database_host = env::var("VK_DATABASE_HOST") + .or_else(|_| env::var("VK_DB_HOST")) + .context("DATABASE_HOST configuration")?; +``` + +#### Runtime Registration + +For scenarios where aliases cannot be known at compile time, a +`#[ortho_config(runtime_aliases)]` mode allows registration: + +```rust +#[derive(Config)] +#[ortho_config(env_prefix = "APP_", runtime_aliases = true)] +struct Config { + #[ortho_config(env_name = "MODE")] + mode: String, +} + +// At startup: +let mut aliases = Aliases::builder(); +if uses_legacy_system() { + aliases.alias("LEGACY_MODE", "APP_MODE")?; +} +let config = Config::from_env_with_aliases(aliases.build())?; +``` + +### Runtime API Details + +#### Alias Builder + +```rust +pub struct AliasesBuilder { /* ... */ } + +impl AliasesBuilder { + /// Register that `alias_name` is an alias for `canonical_name` + /// Both names are tested; canonical is tried first + pub fn alias( + mut self, + alias_name: &str, + canonical_name: &str, + ) -> Result { + // Validation: neither can be empty + // Validation: alias != canonical + // Validation: no cycles (A->B, B->A) + Ok(self) + } + + /// Build the immutable Aliases struct + pub fn build(self) -> Aliases { + Aliases { /* ... */ } + } +} + +pub struct Aliases { + // Internal: map of alias -> canonical + map: HashMap, +} + +impl Aliases { + pub fn resolve(&self, name: &str) -> &str { + // If name is an alias, return canonical name + // Otherwise, return name unchanged + self.map.get(name).map(|s| s.as_str()).unwrap_or(name) + } + + pub fn all_names(&self, canonical: &str) -> Vec<&str> { + // Return canonical and all aliases pointing to it + } +} +``` + +#### Integration with Config Trait + +```rust +pub trait Config: Sized { + /// Standard from_env() - uses only compile-time aliases + fn from_env() -> Result; + + /// With runtime aliases + fn from_env_with_aliases(aliases: &Aliases) -> Result; + + /// Merge aliases into resolution chain + fn resolve_env_var(name: &str, aliases: &Aliases) -> Result; +} +``` + +### Detailed Resolution Algorithm + +The resolution engine applies aliases at multiple levels: + +1. **Field-level resolution** (within a struct field) +2. **Nested struct resolution** (for composite configs) +3. **Array/list resolution** (for repeated config blocks) + +For each field: + +```rust +fn resolve_field(field_name, field_spec, aliases): + // field_spec = {env_name, aliases, default, ...} + + canonical = env_prefix + field_spec.env_name + candidates = [canonical] + [env_prefix + a for a in field_spec.aliases] + + for candidate in candidates: + if runtime_aliases provided: + candidate = runtime_aliases.resolve(candidate) + if env_var_exists(candidate): + return parse(env_var(candidate)) + + if field_spec.default: + return field_spec.default + else: + return Error +``` + +### Testing Strategy + +#### Unit Tests + +```rust +#[cfg(test)] +mod tests { + use ortho_config::*; + + #[test] + fn test_canonical_takes_precedence() { + // Set both canonical and alias + // Verify canonical is used + } + + #[test] + fn test_alias_used_when_canonical_absent() { + // Set only first alias + // Verify it's used + } + + #[test] + fn test_alias_order_respected() { + // Set second alias but not first + // Verify second alias is used + } + + #[test] + fn test_default_used_when_all_absent() { + // Don't set any env vars + // Verify default is used + } + + #[test] + fn test_error_when_required_absent() { + // Don't set any env vars, no default + // Verify error is returned with helpful message + } + + #[test] + fn test_cycle_detection() { + // Try to register A->B, B->A + // Verify error during build + } + + #[test] + fn test_nested_struct_aliases() { + // Config with nested struct containing aliases + // Verify aliases work transitively + } + + #[test] + fn test_runtime_override_of_compile_time_aliases() { + // Register runtime alias that shadows compile-time + // Verify correct precedence + } +} +``` + +#### Integration Tests + +```rust +#[cfg(test)] +mod integration_tests { + #[test] + fn test_vk_migration_path() { + // Simulate vk migration from VK_DB_HOST to VK_DATABASE_HOST + // Verify old vars still work in backward-compat mode + // Verify new vars take precedence + } + + #[test] + fn test_podbot_multi_naming_scenario() { + // Simulate podbot with legacy, current, and future naming + // Verify all resolve correctly with right precedence + } +} +``` + +#### Documentation Generation + +Aliases are included in generated documentation: + +```rust +#[derive(Config)] +struct MyConfig { + #[ortho_config(env_name = "FOO", aliases = ["BAR", "BAZ"])] + /// Main configuration + field: String, +} + +// Auto-generates: +// - docs/generated/config-reference.md listing all names per field +// - config help message showing acceptable names +// - migration guide for deprecated aliases +``` + +### Error Messages + +When configuration fails, errors clearly indicate all names tried: + +```text +Error: failed to load configuration field 'database_host' + +Tried in order: + + 1. VK_DATABASE_HOST (canonical) + 2. VK_DB_HOST (alias) + 3. VK_DBHOST (alias) + +No value found and no default set. + +Suggestion: Set one of the above environment variables. + +Learn more: +[https://docs.example.com/vk/config#database_host](https://docs.example.com/vk/config#database_host) +``` + +## Migration Path + +### Phase 1: vk Migration (Month 1) + +1. **Tag canonical variables** in vk's current code + - Define `VK_DATABASE_HOST` as the canonical name + - List all legacy names used historically + +2. **Declare aliases** in config structs + + ```rust + #[ortho_config(env_name = "DATABASE_HOST", aliases = ["DB_HOST", "DBHOST"])] + database_host: String, + ``` + +3. **Test backward compatibility** + - Run with old env var names only + - Verify old scripts still work + +4. **Document migration** + - Release notes: "Both `VK_DB_HOST` and new `VK_DATABASE_HOST` work" + - Deprecation timeline: "Legacy names supported through vX.Y" + +### Phase 2: podbot Migration (Month 2) + +1. **Inventory naming inconsistencies** + - Old: `PODBOT_AUDIO_SAMPLE_RATE_HZ` + - New canonical: `PODBOT_AUDIO_SAMPLE_RATE_HZ` (standardize) + - External/legacy: `PODBOT_AUDIO_SR`, `LEGACY_MODE`, etc. + +2. **Implement runtime alias registration** + + ```rust + let mut aliases = Aliases::builder(); + + // Legacy external integrations + aliases.alias("PODBOT_AUDIO_SR", "PODBOT_AUDIO_SAMPLE_RATE_HZ")?; + aliases.alias("LEGACY_MODE", "PODBOT_MODE")?; + + // Container/Kubernetes secrets using different scheme + if is_container_env() { + aliases.alias("AUDIO_RATE", "PODBOT_AUDIO_SAMPLE_RATE_HZ")?; + } + + let config = PodBotConfig::from_env_with_aliases(aliases.build())?; + ``` + +3. **Gradual deployment** + - Stage 1: Support both old and new in staging + - Stage 2: Require new names in production, warn on old ones + - Stage 3: Remove old names after grace period + +### Phase 3: Community Adoption + +vk and podbot publish: + +- Migration guides +- Before/after comparisons +- Case studies showing maintenance reduction + +Other projects adopt aliases for their own configurations. + +## Alternatives Considered + +### Alternative 1: Raw Environment Scanning at Startup + +**Approach**: Projects scan for multiple variable names and rename them before +initialization. + +```rust +// Anti-pattern: scattered throughout codebase +fn load_config() -> Result { + // Manual renaming + if let Ok(v) = env::var("VK_DB_HOST") { + env::set_var("VK_DATABASE_HOST", v); + } + if let Ok(v) = env::var("DBHOST") { + env::set_var("VK_DATABASE_HOST", v); + } + Config::from_env() +} +``` + +**Pros**: + +- No framework changes needed + +**Cons**: + +- Duplicated across projects +- Error-prone (easy to miss names) +- Hidden logic outside config struct definition +- Hard to document +- Can't optimize (multiple env lookups) +- Imperative, not declarative + +### Alternative 2: Separate Configuration Layer + +**Approach**: Add a "pre-processing" layer that normalizes environment before +passing to ortho-config. + +```rust +struct LegacyEnvMapper { + mappings: HashMap<&str, &str>, +} + +impl LegacyEnvMapper { + fn apply(&self) { + for (old, new) in &self.mappings { + if let Ok(v) = env::var(old) { + env::set_var(new, v); + } + } + } +} +``` + +**Pros**: + +- Loosely coupled to ortho-config +- Could be reused in other contexts + +**Cons**: + +- Requires explicit invocation before `from_env()` +- Easy to forget or misconfigure +- Still duplicates logic across projects +- Less efficient (double environment reads) +- Doesn't provide documentation benefits + +### Alternative 3: Environment Variable Configuration File + +**Approach**: Use a dotenv-like file listing aliases. + +```env +# aliases.env +ALIAS VK_DB_HOST -> VK_DATABASE_HOST +ALIAS VK_DBHOST -> VK_DATABASE_HOST +ALIAS PODBOT_AUDIO_SR -> PODBOT_AUDIO_SAMPLE_RATE_HZ +``` + +**Pros**: + +- Centralized +- Can be version-controlled + +**Cons**: + +- Another file to manage and synchronize +- Breaks single source of truth (separate from config struct) +- Requires parsing at runtime for every lookup +- Harder to discover (hidden in file, not visible in code) +- No type safety or compile-time validation + +### Why This RFC is Better + +The proposed approach: + +1. **Colocates** alias definitions with field definitions (single source of +truth) +2. **Provides** compile-time validation and optimization +3. **Supports** both compile-time and runtime aliases (flexible) +4. **Generates** documentation automatically +5. **Ensures** type safety through Rust's type system +6. **Reduces** boilerplate and human error + +## Drawbacks + +### Compilation Time Impact + +The macro expansion adds non-trivial code to each field with aliases: + +```rust +// Small for one alias, ~1 line of generated code per alias +// For 50-field config with ~2 aliases per field, adds ~100 lines +// Negligible impact on compile time, but worth benchmarking +``` + +**Mitigation**: + +- Profile macro expansion +- Offer `lazy = true` option if needed (defer to runtime resolution) + +### Documentation Burden + +Projects must maintain alias lists in two places: + +1. Source code (as attributes) +2. User documentation (manually) + +**Mitigation**: + +- Auto-generate `config-reference.md` from macro attributes +- Include generated docs in CI validation + +### Potential for Misuse + +Developers might declare excessive aliases, creating confusion: + +```rust +#[ortho_config( + env_name = "FOO", + aliases = ["BAR", "BAZ", "QUX", "QUUX", "CORGE", "GRAULT"] // Too many! +)] +field: String, +``` + +**Mitigation**: + +- Lint warning when >3 aliases per field +- Documentation recommends max 2-3 +- Migration guide emphasizes retiring old names + +### Runtime Alias Registration Complexity + +Runtime aliases add complexity to initialization: + +```rust +// More verbose than simple from_env() +let config = Config::from_env_with_aliases( + Aliases::builder() + .alias("A", "B")? + .alias("C", "D")? + .build() +)?; +``` + +**Mitigation**: + +- Provide builder method: `Config::with_runtime_aliases(|a| a.alias(...)?)` +- Document common patterns +- Examples for typical scenarios + +## Unresolved Questions + +### 1. Case Sensitivity + +Should aliases be **case-sensitive**? + +- **Current proposal**: Yes, case-sensitive (standard for Unix env vars) +- **Alternative**: Case-insensitive matching option + +**Open**: Should we offer a `case_insensitive = true` attribute? + +### 2. Deprecation Tracking + +How should deprecated aliases be tracked over time? + +```rust +#[ortho_config( + env_name = "NEW_NAME", + aliases = [ + ("OLD_NAME", "1.0", "Use NEW_NAME instead"), + ] +)] +``` + +**Open**: Should we add deprecation metadata and emit warnings? + +### 3. Environment Variable Precedence vs Aliases + +When both runtime and compile-time aliases exist for the same field, what wins? + +- **Current proposal**: Compile-time aliases tried first, then runtime aliases +- **Alternative**: Runtime aliases override compile-time + +**Open**: Should precedence be customizable or fixed? + +### 4. Array/List Handling + +For repeated config elements (e.g., multiple database replicas): + +```rust +#[ortho_config( + env_name = "REPLICA_URLS", + aliases = ["REPLICA_HOSTS", "DB_REPLICAS"] +)] +replicas: Vec, +``` + +**Open**: Do aliases apply to the entire list, or individually? + +### 5. Nested Struct Aliases + +When a nested struct has its own aliases, and the parent also does: + +```rust +#[derive(Config)] +struct Parent { + #[ortho_config(aliases = ["OLD_DB"])] + db: DbConfig, // DbConfig itself has aliases on fields +} +``` + +**Open**: Should nested aliases be merged, stacked, or isolated? + +### 6. Performance Profiling + +Before stabilizing, we need to measure: + +- Macro expansion overhead +- Runtime lookup performance vs. simple `env::var()` +- Memory overhead of Aliases struct + +**Open**: What are acceptable performance targets? + +## Next Steps + +1. **Implementation phase**: + - Write macro expansion code with comprehensive tests + - Build Aliases runtime API + - Integrate into ortho-config crate + +2. **Review phase**: + - Get feedback from vk and podbot teams + - Performance profiling against targets + - Test with real migration scenarios + +3. **Documentation phase**: + - Write user guide with examples + - Create migration playbooks for vk and podbot + - Generate reference documentation + +4. **Stabilization**: + - Gather feedback from early adopters + - Resolve unresolved questions + - Version bump and release + +## References + +- [12-Factor App Config](https://12factor.net/config) - environment-first + principle +- [Rust RFC 2407 Anonymous struct fields](https://rust-lang.github.io/rfcs/2407-anonymous-fields.html) + - related feature design +- [dotenv-rs](https://github.com/dotenv-rs/dotenv-rs) - prior art in Rust env + handling +- [Kubernetes environment variables](https://kubernetes.io/docs/tasks/configure-pod-container/define-environment-variable-container/) + - real-world naming patterns diff --git a/docs/scripting-standards.md b/docs/scripting-standards.md index 73c0167c..40b34351 100644 --- a/docs/scripting-standards.md +++ b/docs/scripting-standards.md @@ -108,7 +108,9 @@ def main( # Lists (whitespace/newline separated by default) formats: list[str] | None = None, - man_paths: Annotated[list[Path] | None, Parameter(env_var="INPUT_MAN_PATHS")] = None, + man_paths: Annotated[ + list[Path] | None, Parameter(env_var="INPUT_MAN_PATHS") + ] = None, deb_depends: list[str] | None = None, rpm_depends: list[str] | None = None, ): @@ -158,7 +160,9 @@ Guidance: - Per‑parameter environment names can be pinned for backwards compatibility: ```python - config_out: Annotated[Optional[Path], Parameter(env_var="INPUT_CONFIG_PATH")] = None + config_out: Annotated[ + Optional[Path], Parameter(env_var="INPUT_CONFIG_PATH") + ] = None ``` ## plumbum: command calling and pipelines @@ -256,7 +260,9 @@ f.write_text("1.2.3\n", encoding="utf-8") version = f.read_text(encoding="utf-8").strip() # Atomic write pattern (tmp → replace) -with tempfile.NamedTemporaryFile("w", delete=False, dir=f.parent, encoding="utf-8") as tmp: +with tempfile.NamedTemporaryFile( + "w", delete=False, dir=f.parent, encoding="utf-8" +) as tmp: tmp.write("new-contents\n") tmp_path = Path(tmp.name) diff --git a/docs/windows-gnullvm-build.md b/docs/windows-gnullvm-build.md index dcc070dc..0c63fe96 100644 --- a/docs/windows-gnullvm-build.md +++ b/docs/windows-gnullvm-build.md @@ -1,15 +1,33 @@ # Building for Windows `*-pc-windows-gnullvm` -This document describes adapting the `rust-build-release` action to build for Windows `gnullvm` targets. The configuration produces MinGW-style artefacts on a Windows runner using an MSVC host toolchain while linking against LLVM's MinGW CRT provided by `llvm-mingw`, thereby removing any dependency on `libgcc`. Both `x86_64-pc-windows-gnullvm` and `aarch64-pc-windows-gnullvm` are supported. +This document describes adapting the `rust-build-release` action to build for +Windows `gnullvm` targets. The configuration produces MinGW-style artefacts on a +Windows runner using an MSVC host toolchain while linking against LLVM's MinGW +CRT provided by `llvm-mingw`, thereby removing any dependency on `libgcc`. Both +`x86_64-pc-windows-gnullvm` and `aarch64-pc-windows-gnullvm` are supported. -The process is orchestrated through Python scripts within the action, which replace the PowerShell examples from the original recommendation. +The process is orchestrated through Python scripts within the action, which +replace the PowerShell examples from the original recommendation. ## Strategy Overview -1. **Host vs. Target**: The build runs on a `windows-latest` GitHub Actions runner, which uses the `stable-x86_64-pc-windows-msvc` toolchain as the _host_. This ensures any build scripts (`build.rs`) compile with MSVC and do not introduce GCC runtime dependencies. The _target_ is one of the `*-pc-windows-gnullvm` triples (currently `x86_64` or `aarch64`), which instruct `rustc` to produce a MinGW-style binary. -2. **`cross` Local Backend**: Instead of using Docker (which is unavailable on Windows runners for Linux containers), `cross` is instructed to use its "local backend" via the `CROSS_NO_DOCKER=1` environment variable. This uses the host's toolchains directly. -3. **LLVM Linker**: The key is to tell `rustc` how to link the `gnullvm` target. `clang` and `lld` from the `llvm-mingw` project are used, configured via a dynamically generated `.cargo/config.toml` file. -4. **Python Orchestration**: All setup steps—downloading `llvm-mingw`, creating the Cargo config, and setting environment variables—are handled by a dedicated Python script within the action, ensuring cross-platform consistency and maintainability. +1. **Host vs. Target**: The build runs on a `windows-latest` GitHub Actions + runner, which uses the `stable-x86_64-pc-windows-msvc` toolchain as the + _host_. This ensures any build scripts (`build.rs`) compile with MSVC and do + not introduce GCC runtime dependencies. The _target_ is one of the + `*-pc-windows-gnullvm` triples (currently `x86_64` or `aarch64`), which + instruct `rustc` to produce a MinGW-style binary. +2. **`cross` Local Backend**: Instead of using Docker (which is unavailable on + Windows runners for Linux containers), `cross` is instructed to use its + "local backend" via the `CROSS_NO_DOCKER=1` environment variable. This uses + the host's toolchains directly. +3. **LLVM Linker**: The key is to tell `rustc` how to link the `gnullvm` + target. `clang` and `lld` from the `llvm-mingw` project are used, + configured via a dynamically generated `.cargo/config.toml` file. +4. **Python Orchestration**: All setup steps—downloading `llvm-mingw`, creating + the Cargo config, and setting environment variables—are handled by a + dedicated Python script within the action, ensuring cross-platform + consistency and maintainability. ## Implementation within `rust-build-release` @@ -17,7 +35,9 @@ The following changes adapt the recommendation for the existing action. ### 1. GitHub Actions Workflow (`action.yml`) -A conditional step is added to `rust-build-release/action.yml` to trigger the setup whenever a Windows `gnullvm` target is requested. The target triple is passed through to the setup script. +A conditional step is added to `rust-build-release/action.yml` to trigger the +setup whenever a Windows `gnullvm` target is requested. The target triple is +passed through to the setup script. ```yaml # .github/actions/rust-build-release/action.yml @@ -28,28 +48,54 @@ A conditional step is added to `rust-build-release/action.yml` to trigger the se working-directory: ${{ inputs.project-dir }} run: | set -euo pipefail - uv run --script "$GITHUB_ACTION_PATH/src/setup_gnullvm.py" --target "${{ inputs.target }}" + uv run --script "$GITHUB_ACTION_PATH/src/setup_gnullvm.py" \ + --target "${{ inputs.target }}" - name: Build release # ... ``` -The composite action installs `uv` earlier via the "Setup uv" step, so the snippet -does not need to install it explicitly. +The composite action installs `uv` earlier via the "Setup uv" step, so the +snippet does not need to install it explicitly. ### 2. Setup Script (`setup_gnullvm.py`) -The Python helper automates the setup process previously handled by PowerShell. It performs the following actions: - -- **Downloads `llvm-mingw`**: Fetches the specified release archive from GitHub, extracts it to a temporary runner directory, and adds its `bin` directory to the `GITHUB_PATH`. On GitHub’s Windows runners (`x86_64` hosts), the setup defaults to the `ucrt-x86_64` archive, which bundles cross-compilers for both the `x86_64` and `aarch64` Windows targets. The `RBR_LLVM_MINGW_VARIANT` environment variable can override this default. When the requested variant is unavailable or lacks a registered checksum for the chosen release, the helper falls back to `ucrt-x86_64` so the job can proceed on x86_64 runners. -- **Creates `.cargo/config.toml`**: Generates the necessary configuration to instruct Cargo how to link the requested `gnullvm` target using the matching `*-w64-mingw32-clang` frontend. -- **Sets Environment Variables**: Writes the `CROSS_NO_DOCKER=1` flag and the target-scoped `CC_*`, `CXX_*`, `LD_*`, `AR_*`, and `RANLIB_*` variables to `GITHUB_ENV` for the subsequent build step to use. -- **Supports Overrides**: Optional `RBR_LLVM_MINGW_VERSION`, `RBR_LLVM_MINGW_VARIANT`, and `RBR_LLVM_MINGW_SHA256` environment variables allow selecting alternative releases while keeping checksum verification enabled. - -The default archive variant is `ucrt-x86_64`, matching the upstream `llvm-mingw` distribution that links against the UCRT and provides cross-compilers for multiple targets. The setup script supports variants that share the upstream archive layout, including `ucrt-i686`, `msvcrt-x86_64`, and `msvcrt-i686`. When overriding `RBR_LLVM_MINGW_VARIANT`, set `RBR_LLVM_MINGW_SHA256` to the checksum for the chosen archive to keep checksum validation enabled; otherwise the script automatically falls back to the default archive when a checksum is unavailable. +The Python helper automates the setup process previously handled by PowerShell. +It performs the following actions: + +- **Downloads `llvm-mingw`**: Fetches the specified release archive from + GitHub, extracts it to a temporary runner directory, and adds its `bin` + directory to the `GITHUB_PATH`. On GitHub's Windows runners (`x86_64` hosts), + the setup defaults to the `ucrt-x86_64` archive, which bundles + cross-compilers for both the `x86_64` and `aarch64` Windows targets. The + `RBR_LLVM_MINGW_VARIANT` environment variable can override this default. + When the requested variant is unavailable or lacks a registered checksum for + the chosen release, the helper falls back to `ucrt-x86_64` so the job can + proceed on x86_64 runners. +- **Creates `.cargo/config.toml`**: Generates the necessary configuration to + instruct Cargo how to link the requested `gnullvm` target using the matching + `*-w64-mingw32-clang` frontend. +- **Sets Environment Variables**: Writes the `CROSS_NO_DOCKER=1` flag and the + target-scoped `CC_*`, `CXX_*`, `LD_*`, `AR_*`, and `RANLIB_*` variables to + `GITHUB_ENV` for the subsequent build step to use. +- **Supports Overrides**: Optional `RBR_LLVM_MINGW_VERSION`, + `RBR_LLVM_MINGW_VARIANT`, and `RBR_LLVM_MINGW_SHA256` environment variables + allow selecting alternative releases while keeping checksum verification + enabled. + +The default archive variant is `ucrt-x86_64`, matching the upstream +`llvm-mingw` distribution that links against the UCRT and provides +cross-compilers for multiple targets. The setup script supports variants that +share the upstream archive layout, including `ucrt-i686`, `msvcrt-x86_64`, and +`msvcrt-i686`. When overriding `RBR_LLVM_MINGW_VARIANT`, set +`RBR_LLVM_MINGW_SHA256` to the checksum for the chosen archive to keep +checksum validation enabled; otherwise the script automatically falls back to +the default archive when a checksum is unavailable. ### 3. Build Script (`main.py`) -The main build script, `src/main.py`, is modified to recognise when the `cross` local backend should be used, even when no container runtime (Docker/Podman) is detected. +The main build script, `src/main.py`, is modified to recognise when the `cross` +local backend should be used, even when no container runtime (Docker/Podman) is +detected. ```python # src/main.py (excerpt) @@ -61,11 +107,14 @@ The main build script, `src/main.py`, is modified to recognise when the `cross` # ... ``` -This ensures that `cross` is invoked correctly when the setup script has prepared the environment for a local `gnullvm` build. +This ensures that `cross` is invoked correctly when the setup script has +prepared the environment for a local `gnullvm` build. ## How to Use -To build for a Windows `gnullvm` target, set the `target` input in a workflow that uses the `rust-build-release` action on a `windows-latest` runner. The action performs the setup automatically. +To build for a Windows `gnullvm` target, set the `target` input in a workflow +that uses the `rust-build-release` action on a `windows-latest` runner. The +action performs the setup automatically. ```yaml # .github/workflows/example-workflow.yml @@ -85,5 +134,5 @@ jobs: target: ${{ matrix.target }} # ... other inputs like project-dir, version, etc. -The matrix workflow above validates that both Windows `gnullvm` targets are configured correctly. -``` +The matrix workflow above validates that both Windows `gnullvm` targets are +configured correctly. diff --git a/rust-toy-app/README.md b/rust-toy-app/README.md index 8cde6ea4..7fb397c2 100644 --- a/rust-toy-app/README.md +++ b/rust-toy-app/README.md @@ -6,15 +6,15 @@ ## Purpose -This simple Command Line Interface (CLI) application demonstrates various Rust testing patterns for +This simple CLI application demonstrates various Rust testing patterns for coverage collection validation: | Test Type | Framework | File(s) | Harness | -|-----------|-----------|---------|---------| -| Unit tests | [rstest](https://crates.io/crates/rstest) | `src/lib.rs` | Standard | -| Integration tests | rstest + assert_cmd | `tests/cli.rs` | Standard | -| Behaviour-Driven Development (BDD) (Gherkin) | [cucumber-rs](https://cucumber-rs.github.io/cucumber/current/) | `tests/cucumber.rs`, `tests/features/*.feature` | Custom (`harness = false`) | -| BDD (rstest-style) | rstest | `tests/bdd.rs` | Standard | +| --- | --- | --- | --- | +| Unit | [rstest](https://crates.io/crates/rstest) | `src/lib.rs` | Standard | +| Integration | rstest + assert_cmd | `tests/cli.rs` | Standard | +| BDD (Gherkin) | [cucumber-rs](https://cucumber-rs.github.io/cucumber/current/) | `tests/features/*.feature` | Custom | +| BDD (rstest) | rstest | `tests/bdd.rs` | Standard | The test suite validates that the `generate-coverage` action correctly: @@ -97,7 +97,7 @@ The test suite is designed to be invoked via the `generate-coverage` action: ### Action Inputs Used | Input | Value | Description | -|-------|-------|-------------| +| --- | --- | --- | | `with-cucumber-rs` | `true` | Enables cucumber-rs coverage collection | | `cucumber-rs-features` | `tests/features` | Path to Gherkin feature files |