fix(mcp): decode escape sequences in fprobe binary params - #24
Conversation
LLM text interfaces can't emit raw control characters — \x1b arrives as the literal 6-char string "\\x1b" through MCP JSON-RPC. Added unescapeBytes() to the MCP layer only, applied to fprobe's pattern, target, and replacement params. CLI literal-matching contract preserved. Fixes cosmetic binary patches failing with "replacement exceeds target" when ANSI escape codes inflate byte count through MCP transport.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds ANSI pretty renderers for fcase and fprobe, introduces slimming of structuredContent, enables decode_escapes for fprobe (decoding \xNN/\uNNNN into raw bytes with validation), updates CLI/data flow to use slimStructuredContent, bumps MCP version to 3.1.0, and propagates config_only handling across fs/fsearch tooling and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Client CLI
participant MCP as MCP Server (mcp/index.js)
participant Engine as fprobe-engine.py / fs-engine.py
participant FS as Filesystem
CLI->>MCP: send request (includes decode_escapes/config_only)
MCP->>MCP: slimStructuredContent(normalizeStructuredContent(...))
alt tool has pretty ANSI renderer
MCP->>CLI: return content[text] only (ANSI)
else structured JSON path
MCP->>CLI: return structuredContent
end
MCP->>Engine: invoke tool with args (hex args if decode_escapes)
Engine->>Engine: resolve_bytes / scan_pattern / patch_binary
Engine->>FS: read/replace file (os.replace for atomic write)
Engine-->>MCP: return result (structured or pretty)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 008e2f811b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcp/index.js`:
- Line 1083: The condition that currently uses a truthy check on the variable
context before calling args.push("--context", String(context)) drops valid
values like 0; update the check to explicitly test for undefined (e.g., only
skip when context === undefined) so that numeric zero and other falsy-but-valid
values are preserved, keeping the same call to args.push for non-undefined
context.
- Around line 1040-1045: The unescapeBytes function can decode "\x00" or
"\u0000" into a NUL char which later causes execFile to throw
ERR_INVALID_ARG_VALUE; update the code so that after decoding (or inside
unescapeBytes) you detect any NUL (\u0000) characters and throw a clear error
(or return an explicit failure) before arguments are passed to execFile;
specifically, modify unescapeBytes (or add a validateDecodedArgs helper) to
check for '\u0000' and reject/throw with a descriptive message referencing the
offending parameter, and ensure callers that previously passed unescapeBytes
output to execFile now handle or propagate that error instead of allowing
execFile to receive NUL bytes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7a2c4bbb-af78-47e7-a622-653a9a12a534
📒 Files selected for processing (1)
mcp/index.js
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-suite
|
@codex review Please review this PR for correctness, regressions, and test coverage before merge. Focus especially on the MCP-layer |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 008e2f811b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Rendered tools (ftree, fread, fmap, fcontent, fedit, fwrite, fsearch, fls) were returning only pretty ANSI text and dropping the parsed JSON structuredContent. Agents need the JSON for efficient token usage — the pretty output is for human display only. Now both are returned: content[text] has the ANSI rendering, and structuredContent has the parsed JSON. Agent-first, human-friendly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2414fd1000
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Added slimStructuredContent() that strips redundant fields from CLI JSON before returning to the model via structuredContent: - Removes tool/version/mode/backend repeated at every nesting level - Removes budget_seconds, budget_used_seconds, recon_depth, ignored - Removes size_human (keep size_bytes — agent can calculate) - Removes duration_ms from nested objects (keep top-level only) - Drops lines[] array when tree_json exists (duplicate data) - Flattens single-key wrapper objects Agent-first: the model sees minimal JSON. Human display (content[text]) is unchanged — still gets the full pretty ANSI rendering.
Adding structuredContent alongside pretty ANSI caused Claude Code to display the raw JSON instead of the ANSI rendering. Rendered tools (ftree, fread, fedit, fwrite, fmap, fcontent, fsearch, fls) now return only content[text] with pretty ANSI — matching pre-change behavior. Non-rendered tools (fprobe, fmetrics, fcase) keep getting slimmed structuredContent. Root cause: Claude Code prefers structuredContent over content[text] when both are present, overriding the pretty display.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 987bcabbb1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fcase: summary bar (N cases | N resolved | N open) + status-grouped listing with checkmark/bullet icons, priority colors (crit/high in red), and ctrl+o expand hint. Single case view shows goal + resolution. fprobe: purple hex offsets with pipe separators, bold yellow match highlighting for filter terms. Scan mode shows match count + offsets. Patch mode shows status badge (Patched/Dry run/Failed). Both tools now render pretty ANSI for humans while the agent gets slimmed structuredContent JSON (non-rendered path).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bca60e1367
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1. Remove unused `deleted` variable in renderFcaseResult (index.js) 2. Add EXIT trap for temp file cleanup in fprobe, remove explicit rm -f 3. Replace os.rename with os.replace in fprobe-engine.py for cross-filesystem safety Fixes generated by Groq gpt-oss-120b via Responses API, applied with fsuite fedit --lines.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6767f340b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1. slim() recursion: add MAX_DEPTH=20 guard to prevent stack overflow on pathological JSON input (CodeRabbit nitpick) 2. unescapeBytes: document BMP-only limitation — surrogate pairs intentionally produce broken output for binary patching use case Applied by Groq gpt-oss-120b via OpenCode headless mode, validated by Claude.
(cherry picked from commit 9e0b9aa8d27321655fd8e0bbf33d6905a1d4b8a5)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f6282247b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Droid (Groq gpt-oss-120b via Factory) + Claude review: 1. Version bump 2.3.0→3.1.0 in MCP server, package.json, test (Droid) 2. Remove fprobe/fcase from RENDERERS to preserve structuredContent (Droid) 3. Fix context=0 truthy check → context !== undefined (Droid) 4. Add decodeFprobeParam NUL byte guard for decode_escapes (Droid+Claude) 5. Fix fprobe filter read from array payloads (Droid) 6. Fix backup overwrite logic in fprobe-engine.py (always refresh .bak) Claude cleanup: moved decodeFprobeParam outside registerTool, removed duplicate pattern push, wired NUL guard through decode_escapes esc() gate, restored missing "fprobe" tool name in registerTool call.
The strings/items branch used `out +=` without declaring `let out`. Currently dead code (fprobe removed from RENDERERS to preserve structuredContent), but fixes latent ReferenceError if re-enabled. Also adds header line with match count. Found by Opus code review agent.
Reverts the unconditional .bak overwrite introduced in fdde546. The original `if not os.path.exists(backup_path)` guard ensures .bak preserves the truly original pre-patch binary, not an intermediate patched state. Without the guard, repeated patches make restore-to-original impossible. Found by superpowers code review.
1. MCP decode_escapes test: verifies decode_escapes=false searches literal \x1b (finds matches), decode_escapes=true decodes to ESC byte (no matches in literal text file) 2. Backup-once test: patches A→B→C, verifies .bak preserves original A (not intermediate B), confirming restore-to-original contract Closes test gaps found by superpowers code review. Written by Haiku sub-agent with fsuite tools.
Covers the exact original failure mode: 1. Patch with decode_escapes=true using \uNNNN escapes — verifies decoded replacement byte length matches target (succeeds) 2. Patch with decode_escapes=true where decoded length != target — verifies length mismatch is correctly rejected Closes the last test gap from superpowers review. Written by Haiku sub-agent.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6894a4a0b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1. Replace printable \u0048 (H) with \u001b (ESC) — proves real control byte survives MCP→execFile→CLI. Reads back patched file to verify 0x1b byte is present. 2. Replace wrong-reason mismatch test with NUL guard test — sends "A\x00B\x00C" through decode_escapes, asserts decodeFprobeParam rejects it before execFile sees the NUL bytes. Previous test passed for wrong reasons (target-not-found, not length). New test pins exact guard behavior with correct assertions. Found by superpowers code review (round 3).
…l tools With the safeParse binary patch applied, Claude Code correctly shows content[text] to the user and feeds structuredContent to the model. Changes: - cli() now attaches structuredContent alongside rendered pretty text - Re-added fcase and fprobe to RENDERERS map (pretty output restored) - Updated parity tests: all tools now assert structuredContent exists This means: - User sees pretty ANSI for every tool (fread, fedit, ftree, fcase, fprobe) - Agent gets slim JSON for every tool (zero token waste on ANSI) - Best of both worlds — no more choosing between display and efficiency Requires: Claude Code binary patch (safeParse fix) to work correctly. Without patch: Claude Code shows raw JSON instead of pretty text.
… name patch for 2.1.89
Claude Code's "early return blender" in mapToolResultToToolResultBlockParam
discards content[text] when structuredContent exists, showing raw JSON to
the user instead of pretty ANSI rendering. Fix: rendered tools (ftree, fread,
fmap, fcontent, fedit, fwrite, fls, fsearch, fcase, fprobe) now return
content[text] only. Non-rendered tools (fmetrics, freplay) keep
structuredContent for model consumption.
Also updates fpatch-claude-mcp for 2.1.89 variable name shift (${z} → ${A}).
Tests updated: 11/11 passing.
The export action returns {case, events} which the renderer was
colorizing with ANSI, breaking JSON.parse in downstream consumers.
Now renderFcaseResult returns null for payloads with .events,
falling through to raw JSON output.
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5823b578d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
@coderabbitai full review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baec28ed24
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fprobe-engine.py`:
- Around line 78-86: scan_pattern can get pattern_bytes=None (resolve_bytes
returns None) which will cause re.escape to TypeError; after calling
resolve_bytes(pattern, pattern_hex, "pattern") in scan_pattern add a guard that
checks if pattern_bytes is None and raise a clear ValueError (or return an empty
list) indicating the caller must provide at least one of pattern or pattern_hex.
Update the check near the start of scan_pattern so the error message references
the function and the parameters (pattern/pattern_hex) and prevents calling
re.escape on None.
In `@fsearch`:
- Around line 634-654: The bug is caused by adding both -name '.*' and -name
"$query" to hidden_cmd (so both must match); fix by removing the unconditional
-name '.*' from the hidden_cmd array and instead append the appropriate -name
test in the conditional: when match_mode == "name" append -name ".$query" (so it
matches hidden files like .filename) and otherwise append -name '.*' before
-print; update the branches that run "${hidden_cmd[@]}" accordingly (referencing
hidden_cmd and match_mode).
In `@mcp/index.js`:
- Around line 1252-1286: decodeFprobeParam currently returns Buffer.alloc(0) for
undefined/null which turns into an empty hex string and may pass empty CLI
flags; change decodeFprobeParam to return undefined when s is null/undefined
(instead of Buffer.alloc(0)), and update all call sites that consume it (e.g.,
places that do decodeFprobeParam(pattern).toString("hex")) to first check the
return value (const buf = decodeFprobeParam(...); if (buf)
args.push("--pattern-hex", buf.toString("hex"))), and similarly only push other
--*-hex flags when the returned buffer is truthy.
In `@mcp/structured-parity.test.mjs`:
- Around line 302-304: The test's NUL-byte error assertion is fragile because it
looks for the escaped string "\\x00" while decodeFprobeParam actually produces
an error with the real NUL character and wording like "contains \x00 which
cannot be passed via argv"; update the assertion for nulResult to check for the
actual NUL sequence or the real error phrase and/or the "argv" token (e.g.,
assert that textContent(nulResult) includes "contains \x00" or
.toLowerCase().includes("argv") or .includes("nul") ), referencing the failing
assertion variables nulResult and helper textContent and the decoder function
decodeFprobeParam so the test matches the real error message rather than the
escaped literal.
In `@tests/test_fprobe.sh`:
- Around line 397-453: In test_patch_backup_once(), avoid ShellCheck SC2155
warnings by separating local declarations from command substitutions: declare
the variables first (e.g., backup_content1, backup_content2, live_after_first,
live_final) with local, then assign them using the command substitutions on the
next line (e.g., backup_content1=$(cat "$backup_file")), leaving the test logic
in test_patch_backup_once unchanged.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d18ffce4-7e5f-4d49-a72d-3c2d7d4debfe
⛔ Files ignored due to path filters (1)
mcp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
fpatch-claude-mcpfprobefprobe-engine.pyfsfs-engine.pyfsearchmcp/index.jsmcp/structured-parity.test.mjstests/test_fprobe.shtests/test_fs.shtests/test_fsearch.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
fsearch
⚙️ CodeRabbit configuration file
fsearch: Focus on: pattern normalization (glob/extension heuristics), backend selection
(fd/fdfind vs find), and safe handling of user input (quoting, no word-splitting
regressions). Prefer correctness and stability over micro-optimizations.
Files:
fsearch
🧠 Learnings (6)
📚 Learning: 2026-03-11T09:17:10.463Z
Learnt from: CR
Repo: lliWcWill/fsuite PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-11T09:17:10.463Z
Learning: Prefer `-o json` for programmatic decisions in fsuite tools
Applied to files:
fstests/test_fs.sh
📚 Learning: 2026-03-11T09:17:10.463Z
Learnt from: CR
Repo: lliWcWill/fsuite PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-11T09:17:10.463Z
Learning: Prefer `-o paths` when piping fsuite output into another tool
Applied to files:
fs
📚 Learning: 2026-03-11T09:17:10.463Z
Learnt from: CR
Repo: lliWcWill/fsuite PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-11T09:17:10.463Z
Learning: Use `-q` flag for existence checks and silent control flow in fsuite tools
Applied to files:
fstests/test_fs.sh
📚 Learning: 2026-03-11T09:17:10.463Z
Learnt from: CR
Repo: lliWcWill/fsuite PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-11T09:17:10.463Z
Learning: Use `fsuite` for suite-level guidance first, then use operational tools for filesystem reconnaissance before opening files blindly or spawning broad exploration loops
Applied to files:
fs
📚 Learning: 2026-03-11T09:17:10.463Z
Learnt from: CR
Repo: lliWcWill/fsuite PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-11T09:17:10.463Z
Learning: Route results to `stdout` and errors to `stderr` in fsuite tools
Applied to files:
fs
📚 Learning: 2026-03-11T09:17:10.463Z
Learnt from: CR
Repo: lliWcWill/fsuite PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-11T09:17:10.463Z
Learning: Run one narrowing pass with `fsearch` to identify candidate files before broad reads
Applied to files:
fsearch
🪛 Shellcheck (0.11.0)
tests/test_fprobe.sh
[warning] 418-418: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 425-425: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 439-439: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 446-446: Declare and assign separately to avoid masking return values.
(SC2155)
tests/test_fsearch.sh
[info] 357-367: This function is never invoked. Check usage (or ignored if invoked indirectly).
(SC2329)
[info] 369-377: This function is never invoked. Check usage (or ignored if invoked indirectly).
(SC2329)
[info] 379-387: This function is never invoked. Check usage (or ignored if invoked indirectly).
(SC2329)
🔇 Additional comments (23)
fsearch (2)
194-212: LGTM.build_config_roots()correctly identifies config-like directories and populates the globals for traversal.
137-138: Config-only plumbing looks correct. The flag propagates through argument parsing, backend selection, search execution, and JSON output consistently.Also applies to: 788-788, 830-834, 951-955, 962-968, 1009-1011
fs-engine.py (2)
99-101: Good addition. Hidden filename detection (^\.[\w.-]+$) correctly classifies dotfiles as file intent with high confidence.
201-206: Config-only propagation is complete. The parameter flows correctly from request parsing through tool invocation to output serialization.Also applies to: 216-226, 525-525, 565-580, 665-666
fs (1)
50-50: Config-only flag integration is correct. CLI parsing, JSON request construction, and pretty output rendering all handle the new flag consistently.Also applies to: 72-72, 88-88, 96-96, 172-172, 186-190, 229-229, 235-236
fprobe-engine.py (3)
21-30: Hex decoding implementation looks correct. Clear error message on invalid hex, and UTF-8 encoding witherrors="replace"is appropriate for binary tool input.
104-108: Good use ofos.replace(). Atomic on POSIX and handles the "destination exists" case on Windows, making the write-back more robust thanos.rename().Also applies to: 155-155
263-276: Validation and dispatch for patch command is correct. Ensures at least one of text/hex for both target and replacement before proceeding.fprobe (1)
230-232: Good cleanup pattern. EXIT trap ensures temp files are removed on all exit paths including errors.fpatch-claude-mcp (1)
90-90: Target pattern updated for new binary layout. The variable reference changed from${z}to${A}to track changes in the Claude Code bundle.Also applies to: 142-142, 147-147
tests/test_fprobe.sh (1)
529-529: Test registration is correct. New test added to patch section and cross-cutting tests renumbered appropriately.Also applies to: 533-534
tests/test_fsearch.sh (3)
33-36: Test fixture setup is complete. Creates the necessary directory structure and files for config-only testing.Also applies to: 59-63
357-387: Good coverage of --config-only scenarios. Tests verify:
- Config roots (.config, .local) are searched while siblings (visible/) are excluded
- Top-level hidden files (.toolrc) are included
- Top-level hidden dirs (.ssh) surface in nav mode
Note:
test_config_only_includes_top_level_hidden_files(line 371) searches for.toolrcwhich should work since it matches.*. However, if the query were something likeopencode.json, it would fail due to the double-namebug infsearch'srun_config_find().
1145-1147: Tests registered correctly in main().mcp/index.js (6)
1281-1285: NUL byte guard correctly implemented per previous review.The check at line 1282-1284 catches
\x00in decoded output before passing toexecFile, preventingERR_INVALID_ARG_VALUEruntime errors.
1325-1325: Previous review concern addressed:context=0now handled correctly.The check now uses
context !== undefinedinstead of a truthy check, socontext=0will be passed correctly.
857-876:cli()logic correctly omitsstructuredContentwhen renderer produces output.The early return at lines 869-870 returns only
content[text]when a renderer succeeds, matching the documented "early return blender" behavior for Claude Code. This is the intended fix for the PR.
801-849:slimStructuredContentrecursion depth limit of 20 is arbitrary but safe.The depth limit prevents stack overflow on deeply nested/cyclic structures. 20 levels is reasonable for typical tool output.
1321-1338: Hex flags are already supported by fprobe CLI.The
--pattern-hex,--target-hex, and--replacement-hexflags are defined in fprobe-engine.py and integrated with aresolve_bytes()helper that handles hex decoding. No breaking changes here.
1319-1340: 🧹 Nitpick | 🔵 TrivialInconsistent indentation and formatting in fprobe handler.
Lines 1318-1339 have inconsistent indentation (mixed 4-space and 2-space). This appears to be a merge artifact.
-} - const args = [action, file]; - if (action === "strings" && filter) args.push("--filter", filter); + } + const args = [action, file]; + if (action === "strings" && filter) args.push("--filter", filter);> Likely an incorrect or invalid review comment.tests/test_fs.sh (2)
792-792: No action needed — engine already acceptsconfig_onlyparameter.The actual
run_fsearch_jsoninfs-engine.py:216already has theconfig_only=Falseparameter in its signature and properly handles it by appending"--config-only"to args when True. The stub function signature matches exactly.
704-722: Test 7.13 assumes"file"field but may extract empty strings if fsearch outputs"path"instead.The test at line 707 extracts paths with
h.get("file", ""), but fs-engine.py line 481 shows defensive fallback:top_hit.get("file") or top_hit.get("path", ""). This suggests uncertainty about fsearch's JSON schema. If fsearch returns hits with"path"instead of"file", the test will collect only empty strings and fail. Verify fsearch's actual JSON output format for the-o jsonflag to confirm the field name.mcp/structured-parity.test.mjs (1)
240-261: Test logic is correct; \u0081 properly decodes to raw byte 0x81.The
decodeFprobeParamimplementation (lines 1252–1271) correctly parses\u0081as hex 0x81, validates it doesn't exceed 0xFF, and returns a single-byte Buffer. The fprobe patch flow (lines 1331–1337) converts this to--replacement-hexwhendecode_escapes: true. The Python CLI supports both--target-hexand--replacement-hexflags. Test assertion[0x41, 0x81, 0x42]is sound.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 5 file(s) based on 5 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 5 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32aab55ab3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Lines 286 to 290 in f7fa90f
Validate scan pattern before empty-file fast path
The new scan argument validation runs only in the non-empty-file branch, so fprobe scan <empty-file> without --pattern now returns [] with exit code 0 instead of reporting a missing required pattern. This makes malformed calls silently succeed depending on file size, which can mask caller bugs in scripts and MCP wrappers that rely on consistent argument validation.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41c3fa20e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba3406f7ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33ffd2fab7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…2.4.0; add Claude Design brief fsuite (the suite-level guide command): - 12 -> 14 tools (added fwrite MCP-only and fbash MCP-native) - fread blurb mentions image/PDF media engine - fmetrics blurb mentions learning loop - New per-tool descriptions for fwrite, fbash, fs, fls - Added missing per-tool blurbs that were dropping from print_tools docs/internal/readme-drafts/04-cheatsheet-changelog.md: - New v2.4.0 changelog entry consolidating PRs #24, #27, #30, #33, #34, #37, #38 - Test matrix updated: fread 42 -> 84, +memory_ingest, +mcp_parity, totals 516/16 - Header timestamp + cross-references to README + Episode 3 docs/internal/specs/2026-04-29-claude-design-site-brief.md (NEW): - 12-section brief for Claude Design to upgrade lliwcwill.github.io/fsuite - Scope: site/ only, must not touch tool source / MCP adapter / tests - Concrete deliverables, voice guide, proof points, source material to mine - Authorization scope: PR against master, no auto-merge
1 participant
Summary
This started as an MCP escape-decoding fix for
fprobeand grew into a broader MCP/search hardening pass driven by iterative Codex + CodeRabbit review.fprobeescape sequences at the MCP boundary while preserving CLI literal semantics--config-onlypreset tofsearchandfsfor config/dotfile lookupsfprobe,fsearch,fs, and MCP structured parity testsRoot Cause
LLM text interfaces cannot emit raw control bytes directly through MCP tool arguments. Sequences like
\x1borESC (U+001B)arrive as literal text unless the MCP adapter decodes them and hands the engine exact bytes.The fix keeps escape decoding in the MCP layer instead of the engine so the CLI continues to treat backslash escapes literally for predictable grep-style matching.
What Changed
MCP / rendering
fcaseandfprobefcase exportfprobedry-run counts, patch counts, scan labeling, and empty scan labelsfprobe/ binary escape handling\\xNNand\\uNNNNescapes in MCP only--pattern-hex,--target-hex, and--replacement-hexflags so high bytes, NULs, and control bytes survive the JS -> argv boundary unchangedfpatch-claude-mcpdisplay-name patch targeting for Claude Code 2.1.89Search /
--config-only--config-onlysupport tofsearchandfs.config,.local) plus top-level hidden entries for dotfile/config lookupsfs ..parent-path misclassification regression--config-onlynarrowing limited to file/nav flows so scoped content/symbol searches still work normallyReview-driven cleanup
3.1.0Verification
bash tests/test_fprobe.sh->30/30passingbash tests/test_fsearch.sh->71/71passingbash tests/test_fs.sh->96/96passingmcp/structured-parity.test.mjscoverage for render/raw-byte/scan/patch behaviorScope
Files touched across the PR:
fpatch-claude-mcpfprobefprobe-engine.pyfsfs-engine.pyfsearchmcp/index.jsmcp/package-lock.jsonmcp/structured-parity.test.mjstests/test_fprobe.shtests/test_fs.shtests/test_fsearch.sh