Coding standards, M2 review deck, and CI hardening#2
Merged
Conversation
Import and adapt CODING_STANDARDS.md from lucid-lint: the four pillars (Tidy First, CUPID & YAGNI, TDD + Reflect, Clean Code) re-domained to termherd's hexagonal core (App::apply, Event/Effect, ports, SessionId). Toolchain, clippy, layout and testing sections rewritten to match this repo's actual policy (pinned 1.95.0, workspace lints, proptest/nextest). AGENTS.md points to it and keeps precedence. Add the medium-effort review of the M0..M2 build-out as a Typst deck (docs/reviews/), with three CeTZ diagrams. The rendered PDF is a build artifact and is gitignored.
Pin every third-party action to a commit SHA (version in a trailing comment), add a concurrency group that cancels superseded runs, set least-privilege `permissions: contents: read`, and add `workflow_dispatch` plus `RUST_BACKTRACE`. Let dtolnay/rust-toolchain read the version from rust-toolchain.toml so the pinned 1.95.0 (Q10) stays single-source instead of being duplicated in CI. Install cargo-deny via taiki-e/install-action and run `cargo deny check`. Add an actionlint job (workflow lint, verified downloader) and a CodeQL workflow (whole-program SAST, security-and-quality suite, no path gate so it covers every commit on main). Skipped as not applicable / needing setup: coverage (CODECOV_TOKEN), scorecard, typos, lychee, mdBook docs, and lucid-lint's self-dogfood job.
After rebasing onto ce13614 (permission/attention status + sidebar badges), extend the review deck from 6 to 8 findings: - #7 Attention is sticky with no non-Busy escape, and fold_status discards the OSC 9 payload. Framed as contingent (MED?): the "false alarm" depends on Claude emitting non-permission OSC 9, which is not verified here — the verifiable part is the discarded payload + missing escape path. - #8 the status state machine is split across pty (stickiness) and core (Exited-is-terminal); a NOTE that reinforces #6. Adds a CeTZ state-machine diagram for #7 and updates the verdict table and recommended-order slides.
`cargo deny check licenses` reported Unicode-DFS-2016 and MPL-2.0 as not-encountered: allow-listed but used by no crate in the graph (all targets). Remove them so the allow-list mirrors the actual tree — a new dependency under either license now fails the check and forces a conscious decision rather than passing silently. Unicode-3.0 (the successor that is in use) stays. Re-adding is trivial if ever needed.
The pinned dtolnay/rust-toolchain commit requires an explicit `toolchain` input — omitting it (to read rust-toolchain.toml) made every Rust job and CodeQL fail instantly with "'toolchain' is a required input". Pass `toolchain: "1.95.0"` in all four steps, kept in lockstep with rust-toolchain.toml / Cargo.toml (Q10). actionlint failed on the cargo-dist-generated release.yml (shellcheck SC2086 info / SC2129 style, which we don't own). Set `SHELLCHECK_OPTS=--severity=warning` on the actionlint job so it still catches real warnings/errors in our own workflows but tolerates the generated file.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
guillaumejay
approved these changes
Jun 13, 2026
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
3 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.
Imports and adapts standards + CI from
lucid-lint, adds the M0→M2 code-reviewdeck, and trims the license allow-list. Four commits, scoped so each is
reviewable on its own.
Commits
CODING_STANDARDS.md(TidyFirst · CUPID & YAGNI · TDD+Reflect · Clean Code), re-domained to termherd's
hexagonal core and reconciled with this repo's actual toolchain/clippy/test
policy. Plus the review as a Typst deck (
docs/reviews/, CeTZ diagrams; PDFgitignored). AGENTS.md points to the standards and keeps precedence.
concurrency+least-privilege
permissions+workflow_dispatch, read the toolchain fromrust-toolchain.toml(single-source 1.95.0), addactionlint, and a newCodeQL SAST workflow. Skipped lucid-lint bits that don't apply (dogfood,
mdBook, coverage/scorecard needing secrets).
ce13614— extends the deck 6→8findings after rebasing onto the Attention-status feature. #7 is framed as
contingent (the OSC 9 "done-ping" premise is unverified); #8 is a NOTE.
Unicode-DFS-2016+MPL-2.0were allow-listed but used by no crate; removing them makes a future dep
under either license a conscious decision.
cargo deny check licensesnowreports
okwith 0 warnings.Reviewer notes
repo without Advanced Security it will error — happy to path-gate or drop it.