Skip to content

Commit 2e17567

Browse files
ammarioammario
authored
feat: Implement line processor mode (--proc) (#55)
This implements #52 by adding a new RuleEngine that implements a JSON-line based protocol with a persistent process. Performance improvements: - Single persistent process instead of spawn-per-request - Enables stateful filtering (caching, rate limiting) - Better support for interpreted languages
1 parent 66d547c commit 2e17567

File tree

18 files changed

+1728
-613
lines changed

18 files changed

+1728
-613
lines changed

‎CLAUDE.md‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,23 @@ for significantly faster build times while still providing reasonable performanc
2525

2626
When writing tests, prefer pure rust solutions over shell script wrappers.
2727

28+
**Important:** Shell script based tests (`.sh` files) should NOT be committed to the repository. They are acceptable for transient testing during development (WIP), but all submitted tests must be written in Rust as either integration or unit tests. This ensures consistency, maintainability, and proper CI integration.
29+
30+
**Test Philosophy:** Write terse, minimal tests that cover the essential behavior. Avoid verbose test suites with many similar test cases. Each test should have a clear, specific purpose. Prefer 1-2 focused tests over 5-10 comprehensive tests. This keeps the test suite fast and maintainable.
31+
32+
### Debugging Test Failures
33+
34+
**Prefer logging over temporary scripts for debugging.** Tests automatically have tracing enabled via `ctor` initialization in `tests/common/logging.rs`. To debug test failures:
35+
36+
1. Run tests with `RUST_LOG=debug cargo test` to see detailed logging
37+
2. Add `tracing::debug!()` or `tracing::info!()` statements rather than `println!` or temporary test files
38+
3. The logging setup uses `try_init()` so it won't panic if already initialized
39+
40+
This approach ensures:
41+
- Debugging information is available in CI logs
42+
- No temporary files clutter the repository
43+
- Debugging aids can be left in place without affecting normal test runs
44+
2845
When testing behavior outside of the strong jailing, use `--weak` for an environment-only
2946
invocation of the tool. `--weak` works by setting the `HTTP_PROXY` and `HTTPS_PROXY` environment
3047
variables to the proxy address.
@@ -69,6 +86,7 @@ This ensures fast feedback during development and prevents CI timeouts.
6986

7087
Each jail operates in its own network namespace (on Linux) or with its own proxy port, so most tests can safely run concurrently. This significantly reduces total test runtime.
7188

89+
7290
## Cargo Cache
7391

7492
Occasionally you will encounter permissions issues due to running the tests under sudo. In these cases,
@@ -141,8 +159,71 @@ The CI workspace is located at `/home/ci/actions-runner/_work/httpjail/httpjail`
141159
./scripts/wait-pr-checks.sh 47 coder/httpjail # Specify PR and repo explicitly
142160
```
143161

162+
### PR Review Scripts
163+
164+
Two helper scripts are available for managing GitHub PR code review comments:
165+
166+
#### Getting PR Comments
167+
168+
```bash
169+
# Get all resolvable code review comments for the current PR
170+
./scripts/get-pr-comments.sh
171+
172+
# Get raw output (useful for extracting comment IDs)
173+
./scripts/get-pr-comments.sh --raw
174+
175+
# Get compact output (one line per comment)
176+
./scripts/get-pr-comments.sh --compact
177+
178+
# Get comments for a specific PR
179+
./scripts/get-pr-comments.sh 54
180+
```
181+
182+
The script shows comments with line numbers in the format:
183+
```
184+
username [CID=12345678] path/to/file.rs#L42: Comment text
185+
```
186+
187+
#### Replying to PR Comments
188+
189+
```bash
190+
# Reply to a specific comment (auto-marks as automated)
191+
./scripts/reply-to-comment.sh <COMMENT_ID> <MESSAGE>
192+
193+
# Examples:
194+
./scripts/reply-to-comment.sh 2365688250 "Fixed in commit abc123"
195+
./scripts/reply-to-comment.sh 2365688250 "Thanks for the feedback - addressed this issue"
196+
```
197+
198+
To find comment IDs, use:
199+
```bash
200+
./scripts/get-pr-comments.sh --raw | grep CID
201+
```
202+
203+
**Best Practices for Replies:**
204+
- **IMPORTANT**: Push your commits to the remote branch BEFORE replying with commit hashes. GitHub needs the commits to be on the remote to create clickable links.
205+
```bash
206+
git push origin branch-name # Push commits first
207+
./scripts/reply-to-comment.sh CID "Fixed in abc123" # Then reply
208+
```
209+
- Always include the commit hash that fixes the issue (e.g., "Fixed in a5813f3")
210+
- If the commit includes multiple unrelated changes, include a diff snippet showing just the relevant fix:
211+
```
212+
Fixed in commit a5813f3. Relevant change:
213+
\`\`\`diff
214+
- old line
215+
+ new line
216+
\`\`\`
217+
```
218+
- Use `git log --oneline -n 5` to find recent commit hashes
219+
- Use `git show <hash> -- <file>` to get the diff for a specific file
220+
221+
**Note:** The reply script automatically marks all messages with "🤖 Automated 🤖" to indicate the response was generated with AI assistance. This script only works with resolvable code review comments (comments on specific lines of code), not general PR comments.
222+
144223
### Manual Testing on CI
145224

225+
**Important:** When debugging on ci-1, prefer using `scp` to sync individual files rather than creating commits for every small edit. This avoids polluting the git history with debugging commits.
226+
146227
```bash
147228
# Set up a fresh workspace for your branch
148229
BRANCH_NAME="your-branch-name"
@@ -153,7 +234,11 @@ gcloud --quiet compute ssh root@ci-1 --zone us-central1-f --project httpjail --
153234
git checkout $BRANCH_NAME
154235
"
155236

156-
# Sync local changes to the test workspace
237+
# Sync individual files during debugging (preferred over commits)
238+
gcloud compute scp src/rules/proc.rs root@ci-1:/tmp/httpjail-$BRANCH_NAME/src/rules/ --zone us-central1-f --project httpjail
239+
gcloud compute scp tests/json_parity.rs root@ci-1:/tmp/httpjail-$BRANCH_NAME/tests/ --zone us-central1-f --project httpjail
240+
241+
# Or sync entire directories if many files changed
157242
gcloud compute scp --recurse src/ root@ci-1:/tmp/httpjail-$BRANCH_NAME/ --zone us-central1-f --project httpjail
158243
gcloud compute scp Cargo.toml root@ci-1:/tmp/httpjail-$BRANCH_NAME/ --zone us-central1-f --project httpjail
159244

‎Cargo.lock‎

Lines changed: 31 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ filetime = "0.2"
3838
ctrlc = "3.4"
3939
url = "2.5"
4040
v8 = "129"
41+
serde = { version = "1.0", features = ["derive"] }
42+
serde_json = "1.0"
4143
simple-dns = "0.7"
4244

4345
[target.'cfg(target_os = "macos")'.dependencies]
@@ -53,6 +55,7 @@ tempfile = "3.8"
5355
assert_cmd = "2.0"
5456
predicates = "3.0"
5557
serial_test = "3.0"
58+
ctor = "0.2"
5659

5760
[package.metadata.cargo-udeps.ignore]
5861
dev = ["serial_test"]

0 commit comments

Comments
 (0)