Add ts dev lint domains + ts dev install-hooks - #733
Open
aram356 wants to merge 5 commits into
Open
Conversation
aram356
marked this pull request as draft
May 23, 2026 20:19
3 tasks
aram356
force-pushed
the
feature/check-domains-spec
branch
from
July 10, 2026 16:38
fcff9ed to
30f2cc1
Compare
Port the pure-Rust URL-host linter (`ts dev lint domains`) and the
pre-commit hook installer (`ts dev install-hooks`) onto main's
restructured CLI. Both are cross-host (all git access via gitoxide, no
`git` subprocess) and register as siblings of the macOS-only
`ts dev proxy`.
Adapt to main's layout:
- Move the feature under `src/commands/dev/{lint,install_hooks}` and add
the `Lint` / `InstallHooks` variants to `DevCommand`, replacing the
prior empty-enum handling.
- Reintroduce a small `CliError` (`Io`, `Json`, `EnvironmentError`,
`ViolationsFound`) plus `output::write_{stdout,stderr}_line` /
`write_json`; `output` becomes cross-host while `info` / `warn` stay
macOS-only.
- Map the linter's results to the exit contract (0 clean, 1 violations,
2 environment error) in `commands::dev::run`.
- Scope `error-stack` / `derive_more` cross-host and add `gix` /
`gix-config`; add `assert_cmd` / `predicates` / `temp-env` dev-deps.
The former `serve` subcommand is not restored — main replaced it with
`ts dev proxy` independently of this work.
aram356
force-pushed
the
feature/check-domains-spec
branch
from
July 15, 2026 00:30
30f2cc1 to
b0b6bd5
Compare
Pin a fixed `user.name` / `user.email` in the repo-local config of the git fixtures. `create_and_checkout_branch` writes a ref through `repo.reference(...)`, whose reflog needs a committer identity; CI machines have no ambient identity, so both `changed_vs` tests failed with `CreateOrUpdateRefLog(MissingCommitter)`. Developer machines passed only because they inherited a global git identity. Repair the "Resolved by the Phase 2 spike" list in the design spec. Its sub-bullets were glued onto preceding lines and its inline code spans were split across line breaks, so prettier re-indented the block deeper on every run and never converged, failing `format-docs`.
aram356
marked this pull request as ready for review
August 18, 2026 21:30
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Automated Review:
Summary
Reviewed the domain-lint and hook-installation changes. I found one confirmed self-exclusion defect, submitted inline.
Findings by priority
- P1: 1 inline finding.
CI
All currently reported PR checks are passing. git diff --check is clean.
Existing Reviews
No existing submitted reviews were returned when checked; this review does not duplicate prior feedback.
|
|
||
| /// The linter's own source file — excluded so its allowlist | ||
| /// constants and doc comments cannot self-flag. | ||
| const SELF_PATH: &str = "crates/trusted-server-cli/src/dev/lint/domains.rs"; |
Collaborator
There was a problem hiding this comment.
P1 – Correct the self-exclusion path
SELF_PATH omits the commands/ path component, so the component-aware Path::ends_with check cannot match this file's actual repository path. Consequently full, changed-vs-ref, staged, and explicit scans can report the linter's own intentionally disallowed-host fixtures; the installed staged hook can then reject commits that modify this source file. The regression test currently constructs the same incorrect src/dev/lint layout, so it does not cover the real path.
Suggested change
| const SELF_PATH: &str = "crates/trusted-server-cli/src/dev/lint/domains.rs"; | |
| const SELF_PATH: &str = "crates/trusted-server-cli/src/commands/dev/lint/domains.rs"; |
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
2 participants
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
ts dev lint domains— a pure-Rust source/config/docs linter that flags non-allowlisted URL hosts in four modes (--staged,--changed-vs <ref>, full-repo, explicit paths). All git operations go through gitoxide (no shelling out togit).ts dev install-hooks— installs a managedpre-commithook that runsts dev lint domains --staged, with foreigncore.hooksPathpreflight, unmanaged-clobber refusal, and--forcewith timestamped backup.ts devleaf into a subcommand group:ts dev servepreserves the prior surface;lintandinstall-hooksare siblings.Stacked on #669 — base branch is
feature/ts-cli. Merge that PR first.Design
Allowlists:
EXACT_HOSTS,SUBDOMAIN_HOSTS,REFERENCE_HOSTS, plus RFC 2606 reserved TLDs. Suppression marker:// allow-domain: host(and#,<!--,*comment forms). Scanned extensions cover Rust, TS/JS, configs, Markdown, CSS, HTML, env files, and Dockerfiles. Exit codes: 0 clean, 1 violations, 2 environment error, 130 cancelled.Test plan
cargo fmt --all -- --checkcargo clippy --workspace --exclude trusted-server-cli --all-targets --all-features -- -D warningscargo clippy --package trusted-server-cli --target <host> --all-targets -- -D warningscargo test --package trusted-server-cli --target <host>— 132 tests passcd crates/js/lib && npx vitest run— 291 tests passcd crates/js/lib && npm run formatcd docs && npm run formatcargo test --workspace --exclude trusted-server-cli— one pre-existing failure on the base (test_env_var_roundtrip_normalizes_integration_types) reproduces onorigin/feature/ts-cliwith zero files touched intrusted-server-core/on this branch; not a regression from this work.