You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Implements the improvement plan from docs/superpowers/2026-09-10-licet-improvements.md (audit: docs/2026-09-10-licet-audit.md), plus follow-ups found in review.
docs: attribute vendored REUSE Specification v3.3 (CC-BY-SA-4.0) — the vendored spec copy lacked the attribution CC-BY-SA requires; adds FSFE notice, SPDX tag, source link. (Upstream is ShareAlike, not plain CC-BY.)
feat!: rename default config license.toml to licet.toml — names containing license trip license-detection heuristics (GitHub licensee, REUSE tooling). Default is now <root>/licet.toml via config::CONFIG_FILENAME. Migration is mv license.toml licet.toml; a lone legacy file fails exit 2 naming the rename, and explicit --config still reads any name. 3 new tests; 393 green (24 differential, comparator-enforced).
docs/REUSE_Specification_v3.3.md is a verbatim copy of the FSFE REUSE
Specification, whose upstream reuse-docs repository is licensed
CC-BY-SA-4.0 (not CC-BY-4.0). Add the required attribution: FSFE
copyright notice, SPDX identifier, source link, and a note that no
REUSE-tool source (GPL-3.0-or-later) is included in this project.
Names containing 'license' are claimed by license-detection heuristics
(GitHub licensee, REUSE tooling), which misread the declarative config
as a license text. The default config path is now <root>/licet.toml
everywhere (check/apply/lint/add-license discovery, init output,
--config help), via config::CONFIG_FILENAME.
Migration: 'mv license.toml licet.toml'. A lone legacy license.toml is
never picked up by default - check/apply fail exit 2 naming the rename,
and init refuses to write a competing default next to one. An explicit
--config path still reads any filename as an escape hatch.
Covers default load, legacy hint, and init paths with three new
enforce tests; snapshots, living docs, and contracts updated.
393 tests passing (24 differential vs reuse 6.2.0).
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request implements a comprehensive overhaul of the scanning engine and CLI to achieve REUSE 3.3 compliance. However, the PR is currently not up to standards due to a significant increase in cyclomatic complexity across core modules and a failure to meet quality thresholds, evidenced by 15 new issues and over 300 detected code clones.
A critical risk exists in the high-complexity logic found in src/walk/mod.rs, src/reuse/atomic.rs, and src/reconcile/mod.rs. These files handle sensitive filesystem operations and file selection but currently lack sufficient automated test coverage to safely support their high cyclomatic complexity (CCN > 25). Additionally, the consolidation of 12 distinct tasks into a single update significantly hampers reviewability and increases the likelihood of side effects in the new atomic writer or git-index snapshotting logic.
About this PR
The Pull Request combines 12 major independent tasks and a breaking rename into a single update. This significantly increases review complexity and the risk of regression compared to delivering these improvements incrementally.
Test suggestions
Verify that atomic_write rejects writing through a symlink destination or ancestor.
Ensure check --staged reads bytes from the Git index even when the working tree has unstaged changes.
Confirm that apply appends a superseding override annotation to REUSE.toml instead of editing existing stanzas in-place.
Verify that the init command pins root-level files as ./filename to prevent them from matching deeper files of the same name.
Check that lint reports missing_copyright errors independently of whether the file matches its declared license intent.
Ensure the engine detects and inventories snippets located deep in a large file (> 8 KiB).
Verify that check --explain provides winning/losing rules and metadata provenance without scanning the full tree.
Confirm the offline-guard CI job prevents network-capable crates from entering the dependency tree.
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
The primary safety helper is too complex (CCN 29) for a component lacking full test coverage, making it difficult to guarantee that security checks (like symlink ancestors) are applied correctly. Refactor atomic_write to extract destination state classification and temporary file staging logic.
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
High complexity (CCN 32) in reconciliation planning combined with low test coverage increases the risk of source file corruption during apply runs. Refactor plan_file to extract copyright policy evaluation and anchored insertion point calculation into dedicated private methods.
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
This function is overly complex (CCN 35) and lacks sufficient test coverage, making it difficult to audit for the subtle correctness requirements of staged vs worktree reads.
Refactor the prepare function to extract the logic for Selection::Staged and Selection::Changed into dedicated private functions, and separate the configuration reading logic from the file marking logic.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
File length and mixed responsibilities make oob.rs difficult to maintain. The implementation of Task 4 should have included splitting this into a module directory (src/reuse/oob/) with dedicated files for parsing, lookup resolution, and batch writing. Note: This file is currently identified as high-complexity with insufficient coverage.
| `writes` | list of write records | Independent of file states: one record per planned/applied/failed/blocked write with destination, kind, outcome, covered files, and exact text for text writes (FR-021). |
| `summary` | `{ pass, partial, complete, before_pass, projected_pass, counts }` | `counts` holds per-`DriftClass` totals **plus** `conflicts` and `contradictions`; `partial` (FR-021) and `pass` (FR-012a, SC-009) live here too. `complete` is false when final verification did not run; `before_pass` is the pre-apply gate; dry-run `pass` means `projected_pass`. Drives the exit code. This is the canonical shape; `report.schema.json` matches it. |
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Suggestion: Avoid hard-coding std::process::exit in utility functions. It is better to propagate the error via Result and handle process exit logic in the main dispatch or a higher-level handler to improve testability.
A regular code review gets triggered (for example, "Codex (@codex) review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
Security: Resolve Git outside the repository search path
On Windows, running any licet command from an attacker-controlled checkout can execute a committed git.exe. Every command now reaches this call during root discovery, and Command::new("git") uses the platform executable search path, where the current directory can precede the system Git. The -C argument and status checks happen only after the planted image starts, so it runs with the maintainer/CI job's privileges. Resolve a trusted absolute Git path or use an in-process API; fix the new unqualified curl helper similarly.
Every licet command runs with the evaluated repository as its working
directory, and OS executable search can consult the CWD (notably on
Windows, where it precedes PATH entries). A checkout containing a
planted git.exe/curl could therefore supply the helper binary. New
crate::tool::resolve scans PATH explicitly, ignoring empty entries
(CWD by convention) and relative entries (CWD-relative), requires a
regular executable file, and probes .exe on Windows. All git (3 sites)
and curl (fetch path, already --disable/proto-pinned) spawns go
through it; unresolvable tools fail exit 2 naming the PATH lookup.
Covered by 5 hermetic resolver unit tests plus an integration test
running check with an empty PATH.
Answer the Codacy findings with behavior-preserving extractions,
each covered by the existing suites:
- atomic_write: extract classify_destination (destination-state
classification without following symlinks)
- plan_file: extract plan_missing_header and resolve_target_block
(target validation stays refusal-only, never clamping)
- prepare: extract the common Assembly/assemble tail shared by all
five selection arms (also removes the duplicated packing)
- lint: extract explicit --config validation (check_explicit_config)
- reuse/oob.rs (1828 lines) -> reuse/oob/{mod,parse,lookup,write,tests};
public paths unchanged via re-exports
- dep5: drop the redundant strip_suffix (every value path trims;
verified by mutation) and pin CRLF-tolerance with a regression test
- docs: blank line around lint list (cli.md), headings + code-span
spacing (data-model.md)
Deliberately not changed: emit_stdout's quiet EPIPE exit(0) is the
tested contract (broken_stdout_pipe_exits_quietly), not a utility
accident.
Main's manifest lost the serde_json dependency while src (report,
add-license, lint) still uses serde_json::json!, so the rebased tree
did not compile. Re-add serde_json = "1" (lock resolves to 1.0.150,
matching the stale lock entry main carried).
Lockfile: apply Deprecation version bumps, prune gix/sha2 subtrees
removed from the manifest. Verified: 400 tests pass, clippy -D warnings
clean, fmt clean, MSRV 1.89 check ok, 24/24 REUSE differential, offline
guard shows no network crates.
- reuse/atomic: file_attributes() on fs::Metadata comes from
MetadataExt, not FileAttributesExt (which is for fs::File).
- walk/git: gate the OsString import on unix, its only use site
(bytes_to_path) is cfg(unix); on Windows it trips -D warnings.
Verified with a local x86_64-pc-windows-msvc lib check; native gates
still green (400 tests, clippy -D warnings, fmt).
…nings
The non_executable test ended its cfg(not(unix)) branch with return,
making the shared assert unreachable on Windows (deny unreachable_code).
Split into a cfg(unix) negative test and a cfg(not(unix)) positive test
that also pins the accept-a-plain-file semantics.
…pendent
globset disables backslash escapes by default where backslash is a
path separator (Windows), turning the escaped literals our REUSE
pattern compiler emits (for ?[]{} and backslash escapes) into path
separators. Force backslash_escape(true) for both REUSE.toml and dep5
patterns; candidates are already forward-slash normalized, so matching
is now identical on every OS. No-op on Unix (already the default).
Git reports the repo root in long form (C:/Users/runneradmin/...) while
the process cwd can carry 8.3 short-name aliases (C:\Users\RUNNER~1\...),
so lexical strip_prefix rejected in-root --files inputs as outside the
project root (exit 2). Compare both sides through a spelling-normalized
form on Windows (canonicalize, with nearest-ancestor fallback for missing
inputs); Unix keeps the untouched lexical path. Also applied to explicit
--config resolution, which shares the comparison.
Rebased onto de522d2 (remote tip). The rebase surfaced three latent issues, all fixed in this branch:
Dep reconciliation (da14ca3) — the merged manifest had lost the serde_json dependency while src still uses serde_json::json!, so the tree did not compile. Re-added serde_json = "1" (lock resolves to 1.0.150); lockfile takes the Deprecation bumps with the removed gix/sha2 subtrees pruned. Note: main@de522d2 itself does not compile as committed for the same reason.
Windows CI (red on main, now green here) — four fixes, all verified by the matrix:
a3e1c2d: MetadataExt, not FileAttributesExt, provides file_attributes() on fs::Metadata; gate the OsString import on unix (its only use is cfg(unix)).
4a79d93: split the platform-divergent resolver test so no cfg(not(unix)) branch ends in return (-D unreachable-code).
9888586: force backslash_escape(true) on both oob glob builders — globset disables escapes by default on Windows, which broke literal ?[]{} /`-escape matching (4 failing lookup tests).
cb6f476: --files compared the cwd-joined input lexically against git's root, but on Windows the cwd can carry 8.3 aliases (RUNNER~1) while git rev-parse reports the long form → bogus "outside the project root" (exit 2). Both sides now compare through a spelling-normalized form on Windows; Unix path handling is untouched.
Local gates on the pushed head: 400 tests pass, clippy -D warnings clean, fmt clean, MSRV 1.89 ok, 24/24 REUSE differential, offline guard clean. GitHub matrix (ubuntu/macos/windows + all jobs) is green.
One remaining red check: Codacy (13 new issues, gate max 0). 12 are cyclomatic-complexity findings (limit 12) on functions this diff touches — including three (explain_one, scan, generate) outside this diff entirely — plus 1 false positive (i18n complaint about tests/fixtures/table/sample.jsx, which is test data). This reads as a default-threshold gate mismatched to the codebase rather than actionable review feedback; fixing it properly means either a 12-function refactor campaign or a threshold/fixture-exclusion adjustment in Codacy. Leaving that call to the maintainer.
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
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.
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
Implements the improvement plan from docs/superpowers/2026-09-10-licet-improvements.md (audit: docs/2026-09-10-licet-audit.md), plus follow-ups found in review.
Stack (3 commits)
licensetrip license-detection heuristics (GitHub licensee, REUSE tooling). Default is now<root>/licet.tomlviaconfig::CONFIG_FILENAME. Migration ismv license.toml licet.toml; a lone legacy file fails exit 2 naming the rename, and explicit--configstill reads any name. 3 new tests; 393 green (24 differential, comparator-enforced).Verification (this branch head)
cargo test --locked --lib --bins --tests: 393 passed, 0 failed (19 suites)LICET_REQUIRE_REUSE=1 cargo test --test reuse_differential: 24/24 vs reuse 6.2.0cargo clippy --locked --all-targets -- -D warnings,cargo fmt --all --check,cargo test --doc, MSRV 1.89, offline-guard: cleanNotes
.specify/memory/constitution.md(gitignored agent memory) still sayslicense.toml; tracked contracts carry the rename.