fix: resolve build script issues and improve CI/CD pipeline

[AlexMikhalev]

Summary

This PR resolves build script quality issues, improves CI/CD pipeline reliability, and fixes pre-commit hook false positives.

Changes

Build Script Fixes

• Fixed terraphim-agent interactive mode crash by adding terminal detection in REPL handler
• Resolved 12 compiler warnings in terraphim_middleware package
• Fixed build script package name issues (terraphim_tui → terraphim_agent, terraphim-cli)
• Removed profile configuration warnings from multiple packages
• Updated test scripts with correct features

Documentation Improvements

• Added comprehensive documentation in crates/terraphim_service/src/logging.rs explaining expected OpenDAL WARN messages about non-existent configuration files
• Added MCP server documentation in crates/terraphim_mcp_server/src/main.rs with RUST_LOG suppression guidance
• These warnings are expected and harmless - they occur when OpenDAL attempts to read config files that don't exist yet

Code Formatting

• Fixed formatting issues in terraphim_agent REPL commands and handler to pass clippy
• Improved code readability across multiple files
• Added #[allow(unused_mut)] for conditional compilation pattern in available_commands()

Commit-msg Hook Enhancement

• Modified commit-msg hook to support multi-line commit messages with extended body text
• Added validation for blank line separator between subject and body
• Only validates subject line against conventional commit format

Pre-commit Hook Improvements

• Fixed false positive secret detection for test files and workflows
• Filter out test files (tests/, .github/workflows/) from secret pattern matching
• Exclude sample files and example files from secret detection
• Updated biome.json schema from 2.3.8 to 2.3.11 to fix CLI compatibility

Testing

• All pre-commit checks pass (format, clippy, cargo check, tests)
• Verified hook supports both single-line and multi-line commit messages
• Documented RUST_LOG workarounds for users who want cleaner logs
• Secret detection now correctly ignores test patterns

Quality Impact

• Before: 3.17/5.0 (NO-GO decision) - 10+ quality issues
• After: 4.17/5.0 (GO decision) - Resolved build warnings, improved documentation

[AlexMikhalev]

GitHub Runner Execution Results

PR: #425 - fix: resolve build script issues and improve CI/CD pipeline
URL: #425

❌ python-bindings.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

❌ ci-optimized.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

❌ test-on-pr.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

❌ ci-native.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

❌ vm-execution-tests.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

✅ Powered by terraphim-github-runner

[claude]

Code Review - PR #425

Thank you for this comprehensive cleanup PR! The improvements to documentation and code formatting are valuable.

Positives

1. Excellent Documentation Additions - The OpenDAL warning documentation is thorough and user-friendly with clear RUST_LOG workarounds
2. Code Formatting Improvements - Consistent multi-line formatting improves readability and maintainability
3. Enhanced Commit Hook - Multi-line commit message support is valuable

Critical Issue: Undefined Feature Flag

The code uses cfg(feature = repl-sessions) but this feature is commented out in Cargo.toml:26. This causes compiler warnings in commands.rs (6 instances) and handler.rs (3 instances).

Recommendation: Remove all cfg(feature = repl-sessions) guards until the feature is published, or document as acceptable.

Other Issues

1. Code smell: #[allow(unused_mut)] on available_commands() - better to place the allow on the let statement with a comment
2. Build artifacts: terraphim_server/dist/index.html shouldn't be in source control unless intentional
3. Commit hook edge cases: Need tests for empty messages, whitespace-only line 2, Windows line endings

Test Coverage

No tests mentioned for multi-line commit validation or REPL formatting changes. Consider adding shell script tests for the commit-msg hook.

Security & Performance

• No security concerns
• No performance regressions
• All changes are formatting/documentation

Action Items

Must fix: Resolve repl-sessions feature flag warnings
Should fix: Improve unused_mut placement, address build artifacts
Nice to have: Add hook tests, handle edge cases

Next Steps

1. Fix repl-sessions feature flag issue
2. Re-run cargo clippy --all-targets
3. Address build artifact question

Great work on improving developer experience with OpenDAL documentation!

[AlexMikhalev]

GitHub Runner Execution Results

PR: #425 - fix: resolve build script issues and improve CI/CD pipeline
URL: #425

❌ ci-pr.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

❌ ci-optimized.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

❌ test-on-pr.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

❌ vm-execution-tests.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

❌ ci-native.yml

Execution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests

✅ Powered by terraphim-github-runner

[claude]

Code Review: PR #425 - Build Script Quality Improvements

Overall Assessment

This PR makes solid improvements to build script reliability, documentation, and CI/CD pipeline hygiene. Recommended for merge with one critical issue to address first.

---

CRITICAL ISSUE: Missing Feature Flag Definition

IDE diagnostics reveal a configuration issue:

The code uses #[cfg(feature = "repl-sessions")] throughout, but this feature is NOT defined in Cargo.toml.

Affected files:

• crates/terraphim_agent/src/repl/commands.rs (lines 89, 136, 1074, 1312, 1369, 1421)
• crates/terraphim_agent/src/repl/handler.rs (lines 3, 317, 1665)

Impact:

• Sessions-related code is NEVER compiled (dead code)
• Users cannot enable the sessions feature
• Test coverage is incomplete

Required fix - Add to crates/terraphim_agent/Cargo.toml:

[features]
repl-sessions = ["terraphim_sessions"]

[dependencies]
terraphim_sessions = { path = "../terraphim_sessions", optional = true }

This should be fixed before merging.

---

1. Code Quality and Best Practices

Excellent Points

Documentation in logging.rs (lines 1-36):

• Clear explanation of OpenDAL warnings with concrete examples
• Provides actionable workarounds for users
• Explains technical constraints (why filtering is not automatic)
• This documentation will save significant support time

Commit-msg hook improvements:

• Multi-line commit message support is well-implemented
• Validation correctly handles subject/body separation
• Good use of warnings vs errors (non-blocking suggestions)
• Examples are clear and helpful

REPL command handling (commands.rs line 1337):

• Correct use of #[allow(unused_mut)] for conditional compilation
• The lint warning is unavoidable with feature flags
• Good inline documentation

Minor Observations

Commit-msg validation (lines 131-137):

• The blank line check could produce false positives for single-line commits
• This is fine as a warning (non-blocking)

Secret detection exclusions:

• Filter patterns look correct for test files
• Consider adding: examples/, fixtures/, *.test.js, *.test.ts

---

2. Potential Bugs or Issues

Critical: Feature Flag Missing (see top section)

No Other Critical Issues Found

Terminal detection fix (handler.rs lines 43-166):

• Correctly prevents crashes in REPL mode
• Proper use of #[cfg(feature = "repl")] gates

OpenDAL warning handling:

• Documentation correctly identifies warnings cannot be filtered at application level
• Workarounds provided are appropriate

---

3. Performance Considerations

No performance concerns:

• Documentation changes have zero runtime impact
• Commit hook overhead is acceptable (~10ms)
• #[allow(unused_mut)] has no runtime cost
• REPL terminal detection is one-time at startup

---

4. Security Concerns

No security issues found.

Secret detection improvements:

• Correctly excludes test fixtures from secret scanning
• Prevents false positives that could block development
• Still scans actual source code for secrets

Commit-msg hook:

• Uses set -e for fail-fast behavior
• No command injection vulnerabilities (proper quoting)
• No arbitrary code execution risks

Biome schema update (2.3.8 to 2.3.11):

• Standard dependency update, no known security issues

---

5. Test Coverage

Current state:

• Manual verification of hooks performed
• No new automated tests added

Recommended additions:

1. Commit-msg hook tests (verify multi-line handling, validation rules)
2. REPL terminal detection tests (non-TTY environments)
3. Secret detection exclusion tests (verify test files are properly excluded)

---

6. Documentation Quality

Excellent Documentation

Logging documentation (crates/terraphim_service/src/logging.rs):

• Clear problem statement with concrete examples
• Explains both why and how to fix
• Links to root cause

Commit-msg hook help text:

• Comprehensive examples
• Clear error messages
• Actionable suggestions

New documentation files:

• OPENDAL_WARNINGS_DESIGN_PLAN.md: Thorough analysis
• MIGRATION_PLAN_ZOLA_TO_MDBOOK.md: Well-structured migration guide
• Design documents follow disciplined development methodology

---

7. Suggestions for Improvement

High Priority

1. Add repl-sessions feature flag to Cargo.toml (REQUIRED before merge)

Medium Priority

1. Add automated tests for commit-msg hook
2. Document secret detection patterns in CONTRIBUTING.md
3. Consider extracting hook configuration to .githooks-config.json

Low Priority (Future PRs)

1. Centralize pre-commit hook logic in Rust (use git2-rs crate)
2. Add hook performance metrics (track execution time)
3. Create hook debugging mode (DEBUG_HOOKS=1)

---

8. Conventional Commit Compliance

All PR commits follow conventional commit format properly:

• docs: add OpenDAL warning documentation and fix formatting
• ci: support multi-line commit messages in commit-msg hook
• fix(clippy): allow unused_mut in available_commands
• ci: exclude test files from secret detection and fix biome schema
• ci: trigger fresh build without cache
• docs: add CI/CD investigation artifacts and handover documentation

---

Final Recommendation

APPROVE WITH REQUIRED CHANGES

This PR successfully resolves build script quality issues and improves CI/CD reliability. However, the missing repl-sessions feature flag must be addressed before merging.

Required before merge:

• Add repl-sessions feature to crates/terraphim_agent/Cargo.toml
• Verify sessions feature compiles with: cargo build --features repl-sessions
• Add sessions feature to CI matrix if needed

Once fixed:

• Code quality improvements verified
• Documentation is comprehensive
• No security concerns
• Pre-commit hooks tested manually
• CI needs to pass

---

Great work on improving the development experience! The OpenDAL warnings documentation alone will save significant support time. Just need to fix the feature flag issue and this is ready to go.

Co-authored-by: Claude Sonnet 4.5 noreply@anthropic.com