Commit Graph
4 Commits
Author SHA1 Message Date
27e21fdabe feat: add pre-push git hook with delta lint mode (#833)
* feat: add pre-push git hook with delta lint mode

Add pre-push hook and CI quality gate scripts:
- .githooks/pre-push: runs quality gate before push
- scripts/ci/quality_gate.sh: baseline fmt + clippy correctness + tests
- scripts/ci/delta_lint.sh: clippy warnings filtered to changed lines only
- Updated dev-setup.sh to install pre-push hook

Supports environment-gated modes:
- IRONCLAW_STRICT_LINT=1: deny all clippy warnings
- IRONCLAW_STRICT_DELTA_LINT=1: deny warnings only on changed lines

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: use git rev-parse for SCRIPT_DIR, add python3 check

- Fix SCRIPT_DIR resolution in pre-push hook to work correctly
  with symlinks by using git rev-parse --show-toplevel
- Add python3 availability check in delta_lint.sh

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: delta lint stderr handling, --locked flag, path normalization

- Stop suppressing clippy stderr; capture it and show compilation
  errors if clippy produces no JSON output
- Add --locked flag to clippy for lockfile consistency
- Use repo root (via git rev-parse) for path normalization instead
  of os.getcwd() which may differ from repo root

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: dynamically detect upstream base branch in delta_lint.sh

Instead of hard-coding `origin/main`, derive the base ref by checking
`refs/remotes/origin/HEAD`, then falling back to `origin/main` and
`origin/master`. If none can be resolved, skip delta lint gracefully
with a warning and exit 0.

Addresses PR #833 review feedback.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* chore: re-trigger CI after adding skip-regression-check label

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR #833 review feedback for delta lint

- Pass remote name ($1) from pre-push hook to delta_lint.sh
- Accept optional remote name arg, fall back to dynamic detection
- Treat error-level diagnostics as always blocking
- Check span overlap [line_start, line_end] vs changed ranges
- Handle +++ /dev/null (file deletions) in parse_diff
- Catch git merge-base failure with graceful skip
- Add CLIPPY_STDERR to EXIT trap cleanup

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: drop -D warnings from delta lint, scope pre-push tests to --lib

1. Remove `-D warnings` from the clippy invocation in delta_lint.sh.
   With -D warnings, all warnings are promoted to error level in JSON
   output, which bypasses the delta filter entirely (errors are always
   blocking). The Python filter already handles the blocking decision
   for warnings based on changed-line overlap.

2. Scope pre-push tests to `cargo test --lib` (unit tests only) instead
   of the full test suite. Full integration tests can take minutes and
   will train developers to use --no-verify. The full suite runs in CI.
   Skip tests entirely with IRONCLAW_PREPUSH_TEST=0.

Addresses zmanian's review feedback on PR #833.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-15 05:41:29 +00:00
3b57d5bec9 chore: add reviewer-feedback guardrails (CLAUDE.md, pre-commit hook, skill) (#665)
* chore: add reviewer-feedback guardrails (CLAUDE.md, pre-commit hook, skill)

Analysis of ~50 PRs from the past week identified 10 recurring themes
in Copilot and Gemini code review comments. This change addresses them
at development time through three layers:

1. CLAUDE.md additions (7 new rules):
   - Transaction safety for multi-step DB operations
   - UTF-8 string safety (no byte-index slicing)
   - Case-insensitive comparisons for paths/media types
   - Decorator/wrapper trait method delegation
   - Sensitive data redaction in logs/SSE
   - tempfile crate for test temporary files
   - Trust boundaries for worker container data

2. Pre-commit hook (scripts/pre-commit-safety.sh):
   Mechanical checks for unsafe byte slicing, case-sensitive
   extension comparisons, hardcoded /tmp paths, unredacted
   tool parameter logging, and non-transactional DB operations.
   Installed via dev-setup.sh alongside existing commit-msg hook.

3. Review checklist skill (skills/review-checklist/SKILL.md):
   Activates on "review"/"merge" keywords. Covers the judgment-based
   items that can't be linted: transaction safety, SSRF validation,
   approval checks, decorator delegation, test quality, and doc accuracy.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review feedback on pre-commit-safety.sh

- Cache diff output in variable to avoid ~10 redundant git diff calls (Gemini)
- Add early exit when no .rs files are changed (Gemini)
- Fix header comment: list all 5 checks, not just 4 (Copilot)
- Fix check 2 comment: only mentions file extensions, not media types (Copilot)
- Add resolve_base_ref() with fallback candidates instead of hardcoded
  origin/main for standalone mode (Copilot)
- TX check: use -W (function context) to reduce false positives, honor
  // safety: suppression, print triggering lines (Copilot)

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-07 21:20:37 +00:00
f60c91e9a7 ci: enforce regression tests for fix commits (#517)
* ci: enforce regression tests for fix commits

Add a commit-msg hook and CI workflow that require test changes
alongside bug fix commits, ensuring every fix includes a regression
test that would have caught the bug.

- scripts/commit-msg-regression.sh: local git hook (blocks fix commits
  without test changes; exempts static/docs-only; bypass via
  [skip-regression-check] marker)
- .github/workflows/regression-test-check.yml: CI mirror on PRs
  (checks title + commit messages; skip via label)
- scripts/dev-setup.sh: install hook in step 6
- .github/scripts/create-labels.sh: add skip-regression-check label
- CLAUDE.md: document regression test policy

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review feedback on regression test enforcement

- Use here-strings instead of echo|grep to avoid misinterpreting
  special characters in variables
- Use git diff -W (whole-function context) to detect edits inside
  existing test functions, not just new #[test] attributes
- Honor [skip-regression-check] in commit messages in CI (not just
  the PR label)
- Use git rev-parse --git-path hooks for worktree-safe hook install

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Update .github/workflows/regression-test-check.yml

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Copilot <[email protected]>
2026-03-04 04:35:54 +00:00
ffb1cc9be8 refactor: architecture improvements for contributor velocity (#198)
* refactor: split large files and consolidate test stubs for contributor velocity

- Extract 7 Database sub-traits (ConversationStore, JobStore, SandboxStore,
  RoutineStore, ToolFailureStore, SettingsStore, WorkspaceStore) with Database
  as a supertrait combining them all
- Split libsql_backend.rs (2769 lines) into src/db/libsql/ directory with
  one file per sub-trait implementation
- Split config.rs (1753 lines) into src/config/ directory with 16 domain files
- Consolidate 3 duplicate test LLM stubs into shared StubLlm in src/testing.rs
- Split server.rs handlers into src/channels/web/handlers/ directory
- Extract main.rs init phases into AppBuilder (src/app.rs)
- Add developer setup script (scripts/dev-setup.sh)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: move heartbeat test from examples/ to tests/

Convert standalone example binary into a proper #[ignore] integration
test, matching the convention of the other integration tests.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: fix rustfmt formatting for CI

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review comments from Copilot

- tunnel.rs: replace .ok().flatten() with ? to propagate env var errors
- secrets.rs: remove misleading "process-wide cache" comment
- database.rs: use uppercase "DATABASE_URL" in error key
- testing.rs: gate harness tests with #[cfg(feature = "libsql")]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Illia Polosukhin <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-18 23:05:47 +00:00