Skip to content

Coding standards, file-size limit, safe path output, stricter acceptance scanner - #96

Merged
jon-devlapaz merged 5 commits into
mainfrom
docs/coding-standards
Oct 5, 2026
Merged

jon-devlapaz merged 5 commits into
mainfrom
docs/coding-standards

Conversation

@jon-devlapaz

@jon-devlapaz jon-devlapaz commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Why the change

Reviews of tink keep flagging the same problems: one owner per act, whole renames, honest copy, about 25 Codex findings on unsafe file writes, hand-counted file sizes, and raw paths printed to the terminal. This writes the rules down and turns three of them into checks.

Special things to note

  • Merging triggers bump-release.yml (one patch release for all three changes).
  • File-size limit: five files are already over 1,000 lines and are allowed at their size as of this PR (tests/acceptance.rs 8,264, src/skillsets.rs 1,667, src/lib.rs 1,496, src/skills.rs 1,196, src/manifest.rs 1,060). The next acceptance test must first move code out of acceptance.rs.
  • Path output: clippy.toml now disallows Path::display; the 34 remaining src call sites go through output::display_path, which already served 105 sites. Output is unchanged for ordinary paths; control characters and backslashes are escaped (tink builds no Windows targets). Tests print with to_string_lossy(), so acceptance.rs gained only rustfmt rewrapping (+12 lines).
  • Acceptance scanner: tests/acceptance_traceability.rs now counts only #[test] functions as sensors (Codex found it accepted any ID-prefixed fn). All 180 current sensors qualify. With #[test] removed from G6's sensor, the old scanner passed and the new one fails.
  • Local test fix: init_repo now runs git init --initial-branch=master. Seven inspect acceptance tests failed on any machine with init.defaultBranch=main (fixtures are addressed as tree/master/...); CI passed only because its git defaults to master.
  • Local run on a main-default Mac: cargo fmt --check, cargo clippy --all-targets -D warnings and the full suite pass (211 acceptance tests).

Change outline

 AGENTS.md                                 # + read CODING_STANDARDS.md before writing or reviewing code
+CODING_STANDARDS.md                       # 10 structural rules
+docs/standards/filesystem-mutations.md    # checklist for code that writes files
+tests/file_size.rs                        # fail over 1,000 lines, or over an allowed file's size
+clippy.toml                               # disallowed-methods: Path::display -> output::display_path
 src/*.rs (11 files)
-  format!("... {}", path.display())
+  format!("... {}", output::display_path(&path))     # codemod, then cargo clippy --fix
 tests/acceptance_traceability.rs          # only fns marked #[test] count as sensors
 tests/acceptance.rs                       # init_repo pins master; path.display() -> to_string_lossy()
-  path.display()
+  path.to_string_lossy()

🤖 Generated with Claude Code

sb and others added 2 commits October 5, 2026 12:30
Encode the findings that recur across tink reviews (#7, #11 and about 25
Codex findings on filesystem writes) so reviewers and agents can cite a rule
instead of re-deriving it. AGENTS.md points to the new file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The 1k rule was a reviewer duty and tests/acceptance.rs had reached 8,252
lines. Five files already over the limit are allowed at their current size;
they may shrink, not grow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jon-devlapaz jon-devlapaz changed the title docs: coding standards and filesystem-mutation checklist Coding standards, filesystem checklist, and a 1,000-line file limit Oct 5, 2026
Add clippy disallowed-methods for Path::display so a filename from an
untrusted repository cannot inject terminal control sequences. Rewrite the
34 remaining src call sites mechanically (codemod: <expr>.display() ->
output::display_path(&<expr>), then cargo clippy --fix for needless borrows).
Tests print paths with to_string_lossy(). The file-size allowance records
each file's size after rustfmt rewrapped the longer calls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jon-devlapaz jon-devlapaz changed the title Coding standards, filesystem checklist, and a 1,000-line file limit Coding standards, file-size limit, and terminal-safe path output Oct 5, 2026
The traceability scanner accepted any ID-prefixed fn, so a helper that never
runs could satisfy an acceptance row. All 180 current sensors carry #[test].
Proved: with #[test] removed from G6's sensor, the old scanner passed and the
new one fails with 'acceptance row G6 names missing sensor G6'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jon-devlapaz jon-devlapaz changed the title Coding standards, file-size limit, and terminal-safe path output Coding standards, file-size limit, safe path output, stricter acceptance scanner Oct 5, 2026
Fixture URLs address repos as tree/master/..., but init_repo ran plain
'git init', so on machines with init.defaultBranch=main seven inspect
acceptance tests failed with 'Remote branch master not found'. CI passed only
because its git still defaults to master. Requires git >= 2.28.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jon-devlapaz
jon-devlapaz merged commit 1d95bd5 into main Oct 5, 2026
6 checks passed
@jon-devlapaz
jon-devlapaz deleted the docs/coding-standards branch October 6, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant