test(harness): shared test harness, coverage baseline, and case-insensitive name uniqueness - #18
Merged
Merged
Conversation
…ect names Issue #12: adds idx_environments_name_nocase (project_id, name COLLATE NOCASE) and retrofits the same pre-check to idx_projects_name_nocase, so an install that already holds a case-colliding pair (e.g. MyApp/myapp, production/Production) is deterministically deduped (lowest id keeps its name, losers get -2/-3 suffixes, audit persisted under settings key env_name_dedup_v1) instead of bricking VaultDb::open. Unique-constraint violations are now detected via sqlx's is_unique_violation() and mapped to a stable "conflict:" sentinel string instead of substring-matching sqlx's error text, closing a raw-SQL leak in the 500 fallback path. project::resolve_environment now rejects ambiguous case-insensitive project/environment matches instead of silently taking the first one (SQLite NOCASE folds ASCII only, so non-ASCII collisions like PRODUCCIÓN/producción survive the index and need this Unicode-aware check) — the HTTP layer maps that to 409 AMBIGUOUS_SCOPE. New src-tauri/tests/environment_naming.rs covers T1-T8/T11 from the plan plus a projects-side dedup case; sqlx added to [dev-dependencies] to seed pre-collision state that's otherwise unreachable through the public API once the index exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…overage baseline; drop stale Postman collection Implements docs/plans/issue-11-test-coverage-and-postman.md: a crate::test_support harness (TestVault fixture + router()/req() oneshot helpers) so every module's private/pub(crate) logic is unit-testable without a Tauri runtime or live socket, plus 119 new test cases across api/, project/, vault/, db/, share/, the CLI's scope resolver, and the MCP binary's environment-picking logic (17 -> 136 total). Two pure refactors enable the harness, both behavior-preserving: - api::build_router / ApiState::new extracted from start_server (pub(crate), not pub — zero external API widening). - vault::set_item_global's fork logic extracted the same way create_project_item already was, so the Tauri command becomes a thin delegating wrapper. - crypt-env-mcp's resolve_environment_id split into network-fetching + pure pick_environment_id, mirroring the plan's resolve_environment_id split. Deletes the stale Postman collection (asserted a removed /health field, every request 422'd without the mandatory scope params) and replaces it with curl examples + the executed api::tests::* suite in docs/reference.md. Adds .github/workflows/test.yml running cargo test on push/PR with an informational cargo-llvm-cov summary step. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lity, sentinel visibility
Addresses adversarial review of the issue-12 case-insensitive-uniqueness
work:
- Critical: next_free_{project,environment}_name folded the rename
candidate with Rust's to_lowercase() (full Unicode) but compared it
against SQLite's own LOWER(name) (ASCII-only). For a non-ASCII base
this made the two folds disagree, so a real collision against a
pre-existing suffixed sibling (e.g. producciÓn-2) was missed, the
"free" candidate was accepted, and the following UPDATE then hit the
table's exact-match UNIQUE constraint -- init_schema returned Err and
the vault could not be opened at all. Fixed by comparing via
LOWER(name) = LOWER(?) so both sides are folded by SQLite consistently.
Added a regression test reproducing the exact crash shape.
- env_name_dedup_v1 (the sole reversal path for an otherwise
irreversible rename) is now persisted per-rename, inside the dedup
loops themselves, instead of batched and written only after both
CREATE UNIQUE INDEX statements succeed -- a renamed row can no longer
land in the DB without also being recorded, even if a later migration
step fails or the process is killed mid-run.
- PROJECT_NAME_CONFLICT bumped from private to pub (matching
ENVIRONMENT_NAME_CONFLICT, also bumped from pub(crate) to pub): each
src/bin/* target is its own crate separate from crypt_env_lib, so
pub(crate) was invisible cross-crate. Exported
project::AMBIGUOUS_MATCH_PREFIX so the ambiguity sentinel is a named
constant instead of a bare string literal at every match site.
- persist_rename_report no longer silently discards prior rename
history on a JSON parse failure -- logs to stderr and preserves the
raw value verbatim under env_name_dedup_v1_corrupt before starting a
fresh list.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # src-tauri/Cargo.toml
The five branches in this batch add four integration targets under src-tauri/tests/ (environment_naming, path_containment, project_relay, vault_integration). `cargo test --lib --bins` builds none of them, so they would have been invisible to CI while appearing to be covered. Refs #11
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lands the two foundational pieces the rest of this batch builds on: a shared HTTP/vault test harness with a coverage baseline, and case-insensitive uniqueness for environment and project names.
Closes #11
Closes #12
Type of Change
What Changed
#11 —
test_support::TestVaultbuilds an unlocked temp vault + boundaxum::Routervia the sameApiState::newthe real server uses.api/mod.rsgains abuild_routersplit so tests andstart_serverconstruct state identically. 136 test functions, up from 17. The stale Postman collection is deleted: it asserted a removedGET /healthfield and none of its 15 requests carried the now-mandatory scope, so all 15 would 422 — nothing in CI ever ran it.#12 —
idx_environments_name_nocase, plus the same pre-check retrofitted toidx_projects_name_nocase(which could brickVaultDb::openon installs already holding a colliding pair). Colliding rows are deterministically renamed before the index is created, with a report underenv_name_dedup_v1. Write path returns 409CONFLICTwith no SQL text in the body; ambiguous name resolution returns 409AMBIGUOUS_SCOPEinstead of guessing.Dedup name-probing compares via SQLite's own
LOWER()on both sides rather than pre-folding in Rust — mixing Rust's full-Unicode fold with SQLite's ASCII-only one made non-ASCII collisions miss the check and then trip the table's unique constraint, turning a routine dedup into a hardinit_schemafailure. Regression test included.CI —
cargo testgains--tests; the four suites undersrc-tauri/tests/were not being built or run.Known gaps (deliberate, tracked)
TestVaultis reachable only from the lib crate;src/bin/crypt-envandcrypt-env-mcpare separate targets and use in-file tests.crypt-env-mcp'spick_environment_idand the CLI'sscope.rs/project.rsstill take the first name match rather than returningAMBIGUOUS_SCOPE. The DB index prevents ASCII collisions from existing; non-ASCII pairs remain resolvable first-match on those paths. Out of scope here.Testing
cargo checkpassescargo test --lib --bins --tests— 221 passed, 0 failedpnpm tauri dev(not run — Linux/WSL, no Windows toolchain this session)Security Implications
No crypto changes. #12 hardens an error path:
handle_save_project/handle_save_environmentno longer echo the underlying error, which could carry raw sqlx text (table/column/index names), into API responses — the detail is logged server-side instead. Rejections never echo the submitted name.