feat(receipt): add provider-free context guard companion - #283
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request adds the Context Guard Receipt package. It includes local CLI and MCP entry points, evidence assembly and expansion, secure storage, command capture, diagnostics, experimental state, package integrity checks, schemas, and extensive contract tests. ChangesContext Guard Receipt package
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
packages/context-guard-receipt/python/context_guard_receipt/cli.py-47-47 (1)
47-47: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe help text says
positive-decimalwhere the parser accepts only an integer.
_is_positive_integerrequires a canonical base-10 integer, so--timeout-seconds 1.5and--limit 10.0are rejected with exit 64. The help string labels those values<positive-decimal>for--timeout-seconds,--max-channel-bytes,--max-total-bytes, and--limit. A user can readdecimalas permitting a fractional value. Use<positive-integer>in the help text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/python/context_guard_receipt/cli.py` at line 47, Update the HELP usage string to label --timeout-seconds, --max-channel-bytes, --max-total-bytes, and --limit values as <positive-integer> instead of <positive-decimal>, matching the validation performed by _is_positive_integer.packages/context-guard-receipt/python/context_guard_receipt/cli.py-524-534 (1)
524-534: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winWrite the receipt before emitting the payload to stdout.
Line 524 writes the assembled payload to stdout. Line 532 then writes the receipt file, and a
CliIOErrorthere returns exit code 74. The caller therefore sees a full payload on stdout together with a failure exit code, and no receipt file. Write the receipt first, and emit the payload only after the receipt is durable. The refusal branch at line 519 already writes the receipt before it reports the outcome.🛠️ Proposed reordering
+ if type(receipt_path) is str: + try: + write_receipt(receipt_path, canonical_json_bytes(result.receipt)) + except CliIOError as error: + return emit_error("assemble", "error", error.code, 74) _emit_payload( result.output_bytes, operation="assemble", emit=emit, receipt=result.receipt, ) - if type(receipt_path) is str: - try: - write_receipt(receipt_path, canonical_json_bytes(result.receipt)) - except CliIOError as error: - return emit_error("assemble", "error", error.code, 74) return 0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/python/context_guard_receipt/cli.py` around lines 524 - 534, In the assemble flow, move the receipt-writing block guarded by type(receipt_path) is str—including its CliIOError handling—to execute before _emit_payload. Only emit result.output_bytes after write_receipt succeeds, while preserving the existing refusal-branch ordering and error return behavior.packages/context-guard-receipt/schemas/source-identity.schema.json-46-68 (1)
46-68: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winExclude
sourcein thepass_throughbranch.The
pass_throughbranch excludesselectionandsymbolat line 48. It does not excludesource. The three other dispositions each requiresource, sopass_throughis the only disposition wheresourcecarries no meaning.This lets a contradictory document validate.
sourcerequiresfile_typeto equal"regular"and requires acontent_sha256. Apass_throughdocument withreasonsource_missingorsource_not_regularcan still carry thatsourceblock and pass validation.🛡️ Proposed fix to exclude `source` from the pass_through branch
{ "if": {"properties": {"disposition": {"const": "pass_through"}}}, "then": { - "not": {"anyOf": [{"required": ["selection"]}, {"required": ["symbol"]}]}, + "not": { + "anyOf": [ + {"required": ["selection"]}, + {"required": ["source"]}, + {"required": ["symbol"]} + ] + }, "properties": {If producers do emit
sourceon apass_throughidentity, keep the current shape and confirm whichreasonvalues permit it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/schemas/source-identity.schema.json` around lines 46 - 68, Update the pass_through branch in the source-identity schema so its exclusion rule also rejects the source property alongside selection and symbol. Preserve the existing pass_through reason enum and other disposition requirements; do not change the source schema or reason values unless producer compatibility requires separately documenting allowed behavior.packages/context-guard-receipt/scripts/verify_protected_surfaces.py-94-103 (1)
94-103: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPass
-ztogit ls-filesso path comparison stays literal.
git ls-filesquotes any path that contains a non-ASCII or control byte, controlled bycore.quotePath, which defaults to enabled. It emits such a path as a C-quoted string, for example"res\303\251". The comparison at line 176 then finds no match for the literalpath_textand raisesprotected surface tracked status drifted, even though the path is tracked.Every manifest path is ASCII today, so this does not fire now. A future protected surface with a non-ASCII path would fail verification with a misleading reason.
-zdisables quoting and emits NUL-terminated records.🛠️ Proposed fix to read literal paths
def tracked_paths(paths: list[str], repo_root: Path = REPO_ROOT) -> set[str]: result = subprocess.run( - ["git", "ls-files", "--", *paths], + ["git", "ls-files", "-z", "--", *paths], cwd=repo_root, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, ) - return set(result.stdout.splitlines()) + return {record for record in result.stdout.split("\0") if record}The Ruff
S603andS607hints and the ast-grepsubprocess-from-requesthint on this call are false positives. The argv is fixed,shell=Trueis absent,--blocks option injection, and every element ofpathsalready passednormalized_repository_path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/scripts/verify_protected_surfaces.py` around lines 94 - 103, Update tracked_paths to invoke git ls-files with -z and parse stdout using NUL separators, preserving literal path names including non-ASCII and control bytes while retaining the existing fixed-argv and -- safeguards.Source: Linters/SAST tools
tests/test_context_guard_receipt_suite.py-33-34 (1)
33-34: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAssert a minimum discovered test count.
The regex captures the test count in group 1, but the assertion only checks that the match exists. The test therefore passes when discovery finds a single test.
Discovery can lose modules while the return code stays 0. A missing
__init__.pyin a new test subpackage, or a directory that thetest_*.pypattern no longer matches, reduces the discovered set silently. The PR reports 513 companion tests, so a floor makes this a regression guard rather than a smoke check.💚 Proposed fix to assert a count floor
match = re.search(r"Ran ([1-9][0-9]*) tests?", result.stderr) self.assertIsNotNone(match, result.stderr) + assert match is not None + self.assertGreaterEqual(int(match.group(1)), 500, result.stderr)The ast-grep
subprocess-from-requesthint on this call is a false positive. The argv is fixed, andshell=Trueis absent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_context_guard_receipt_suite.py` around lines 33 - 34, Update the test-count assertion in the relevant test method to convert regex group 1 to an integer and require it to meet the expected minimum companion-test count of 513, while preserving the existing match-presence assertion and stderr diagnostic.Source: Linters/SAST tools
packages/context-guard-receipt/tests/contract/test_g006_tool_schemas.py-260-260 (1)
260-260: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPrefix the unused unpacked variable.
payloadis never read in this test. The test recomputesdeferred_rawandinline_rawfromcatalogat lines 264 and 269. Ruff reports RUF059 here, so this can fail the lint gate.🧹 Proposed fix
- raw, payload = descriptor(catalog, [item(priority=2), item(priority=1)]) + raw, _payload = descriptor(catalog, [item(priority=2), item(priority=1)])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g006_tool_schemas.py` at line 260, Update the unpacking in the test around descriptor(catalog, [item(priority=2), item(priority=1)]) to prefix the unused payload variable with an underscore, while preserving raw for subsequent assertions and leaving the test behavior unchanged.Source: Linters/SAST tools
packages/context-guard-receipt/dev/packaged_acceptance.py-1045-1055 (1)
1045-1055: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
--scenariohas no effect.
main()parses--scenarioand then always callsdistribution().argumentsis never read, so--scenario distributionand--scenario allbehave identically. When a second scenario is added, it will silently not run. Dispatch on the parsed value, or remove the option.🐛 Proposed fix
def main() -> int: parser = argparse.ArgumentParser() parser.add_argument("--scenario", choices=("distribution", "all"), default="all") arguments = parser.parse_args() try: - distribution() + if arguments.scenario in ("distribution", "all"): + distribution() except (OSError, RuntimeError, json.JSONDecodeError, tarfile.TarError) as exc:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/dev/packaged_acceptance.py` around lines 1045 - 1055, Update main() to use arguments.scenario when dispatching scenarios instead of always calling distribution(). Preserve distribution-only behavior for --scenario distribution and execute every configured scenario for --scenario all, using the existing exception handling and result reporting.packages/context-guard-receipt/tests/contract/test_g005_cli.py-146-172 (1)
146-172: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe no-state assertion is vacuous.
state_diris never passed torun_cli, so the CLI cannot create it. The assertion at line 172 also runs after thewith tempfile.TemporaryDirectory()block deletes the whole tree, sostate_dir.exists()is alwaysFalse. The test cannot detect state creation without opt-in, which is the break the docstring claims to catch.Pass
--state-dirwithout--persistand assert inside thewithblock.💚 Proposed fix
state_dir = base / "private-state" response = run_cli( "assemble", "--kind", "evidence", "--descriptor", "-", "--root", str(root), + "--state-dir", + str(state_dir), "--emit", "bytes", input_bytes=evidence_descriptor(payload, "source.bin"), ) + self.assertFalse(state_dir.exists()) self.assertEqual(response.returncode, 0, response.stderr) self.assertEqual(response.stdout, payload) self.assertEqual(response.stderr, b"") - self.assertFalse(state_dir.exists())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g005_cli.py` around lines 146 - 172, Update test_nonpersistent_stdin_assembly_is_exact_binary_pass_through to pass the state_dir path via --state-dir without enabling --persist, and move the state_dir.exists() assertion inside the TemporaryDirectory context before cleanup. Preserve the existing binary output and empty-stderr assertions.packages/context-guard-receipt/tests/contract/test_g009_diagnostics.py-88-97 (1)
88-97: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe test does not reach the 999 basis-point boundary it names.
With
input_bytes = 3_000, a predicted cost of 2_701 gives 299 bytes of savings. Integer basis points are then299 * 10_000 // 3_000 = 996, not 999. At an input of 3_000 no integer saving maps to 999, because that would require savings in the range [299.7, 300.0).The test still separates "below the threshold" from "at the threshold", so the direction is covered. It does not prove the boundary is inclusive at exactly 999. Either choose an input length where 999 is reachable, or rename the test to state the pair it actually exercises.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g009_diagnostics.py` around lines 88 - 97, Update test_999_and_1000_basis_point_boundaries_use_integer_math so its name and cases match the actual basis-point values produced by the 3,000-byte input, or change the input and predicted sizes to reach exactly 999 basis points and verify the inclusive threshold. Preserve assertions for both below-threshold and threshold behavior.packages/context-guard-receipt/tests/contract/test_g007_sanitizer.py-368-387 (1)
368-387: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winStrengthen the import-time I/O assertion.
importlib.reloadloads source without callingbuiltins.open, soopened == []does not detect import-time file I/O. Use broader I/O instrumentation or a subprocess audit hook that excludes the loader’s source read.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g007_sanitizer.py` around lines 368 - 387, Strengthen test_module_reload_opens_nothing_and_emits_no_logs so it detects import-time file I/O beyond builtins.open, using broader I/O instrumentation or a subprocess audit hook that excludes the module loader’s own source read. Preserve the existing no-log assertion and verify that sanitizer reload performs no additional file access.packages/context-guard-receipt/tests/e2e/test_g012_mcp_stdio.py-88-99 (1)
88-99: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDrain
stderrbefore waiting for the installed process.
close()callsself.process.wait(timeout=20)whilestderris still an undrained pipe. If the child writes more than the pipe buffer tostderr, the child blocks on write andwaitnever returns. The test then fails withTimeoutExpiredinstead of reporting the child's diagnostics.waitcan also raise before Lines 93-96 run, which leaves the pipes open.Use
communicate()so the remaining output is read while the process exits.🛠️ Proposed fix
def close(self) -> str: if self.process.stdin is not None and not self.process.stdin.closed: self.process.stdin.close() - self.process.wait(timeout=20) - stderr = self.process.stderr.read() if self.process.stderr is not None else "" - if self.process.stdout is not None: - self.process.stdout.close() - if self.process.stderr is not None: - self.process.stderr.close() + try: + _stdout, stderr = self.process.communicate(timeout=20) + except subprocess.TimeoutExpired: + self.process.kill() + self.process.communicate() + raise + stderr = stderr or "" if self.process.returncode != 0: raise AssertionError(f"installed MCP exited {self.process.returncode}") return stderr🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/e2e/test_g012_mcp_stdio.py` around lines 88 - 99, Update the close method to use self.process.communicate() instead of calling wait before reading stderr, ensuring remaining stdout/stderr are drained while the child exits. Preserve the returned stderr diagnostics and nonzero-returncode assertion, and ensure cleanup still occurs if process completion raises.Source: Linters/SAST tools
packages/context-guard-receipt/tests/contract/test_g012_mcp_cli.py-103-113 (1)
103-113: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBound and reap every child process started with pipes. Three test helpers start a child with
PIPEstreams but do not guarantee that the child terminates. Each one can block the suite or leave a process running after the test ends. Apply one convention: pass a timeout, drain the pipes withcommunicate, and kill any survivor in afinally.
packages/context-guard-receipt/tests/contract/test_g012_mcp_cli.py#L103-L113: replaceprocess.wait(timeout=20)and the manualstderr.read()withprocess.communicate(timeout=20), and callprocess.kill()followed by a secondcommunicateonsubprocess.TimeoutExpired. The current code waits whilestdoutandstderrremain undrained, so a child that fills the stderr pipe deadlocks.packages/context-guard-receipt/tests/contract/test_g010_cli.py#L148-L164: addstdin=subprocess.DEVNULLandtimeout=30to thesubprocess.runcall inrun_mcp. Without them the stdio MCP server inherits the test process stdin andserve_stdioblocks on read until EOF, with no timeout to break the wait.packages/context-guard-receipt/tests/contract/test_g010_twin.py#L723-L743: wrap thepool.map(...communicate(raw, timeout=10)...)call intry/finallyand kill both entries ofprocessesthat still returnNonefrompoll(). Each surviving child holds the twin lock and can block a later test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g012_mcp_cli.py` around lines 103 - 113, Ensure every piped child process is bounded, drained, and reaped: in packages/context-guard-receipt/tests/contract/test_g012_mcp_cli.py lines 103-113, replace process.wait and manual stderr reading with process.communicate(timeout=20), killing the process and communicating again on subprocess.TimeoutExpired; in packages/context-guard-receipt/tests/contract/test_g010_cli.py lines 148-164, update run_mcp’s subprocess.run call to use stdin=subprocess.DEVNULL and timeout=30; in packages/context-guard-receipt/tests/contract/test_g010_twin.py lines 723-743, wrap the pool.map communicate call in try/finally and kill each process in processes whose poll() remains None.packages/context-guard-receipt/tests/contract/test_g008_runner.py-2087-2123 (1)
2087-2123: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid closing a descriptor number that the runner already closed.
Line 2117 asserts that the runner closed
descriptor. Thefinallyblock then callsos.close(descriptor)and swallows theOSError. If the OS reuses that descriptor number after the runner closed it, thisos.closecloses an unrelated descriptor in the test process. Track whether the descriptor is still owned by the test and close it only in that case.🐛 Proposed fix
with self.subTest(error_number=error_number), Harness(self) as harness: descriptor = os.open("/dev/null", os.O_RDONLY) + owned = True spawn_calls = [] try: @@ self.assertFalse((Path(harness.root) / "target-marker").exists()) - with self.assertRaises(OSError): - os.fstat(descriptor) + try: + os.fstat(descriptor) + except OSError: + owned = False + self.assertFalse(owned, "the runner must close the probe descriptor") finally: - try: + if owned: os.close(descriptor) - except OSError: - pass🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g008_runner.py` around lines 2087 - 2123, Update the descriptor cleanup in this test around the `descriptor` setup and `finally` block: track whether the test still owns the descriptor, mark it as no longer owned after the runner closes it, and only call `os.close(descriptor)` when ownership remains. Preserve the existing `os.fstat(descriptor)` assertion verifying the runner closed it.packages/context-guard-receipt/python/context_guard_receipt/store.py-1135-1136 (1)
1135-1136: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPreserve the specific error code in
_revalidate_anchors.
except StoreError: _raise(StoreErrorCode.UNSAFE_STATE)collapses every inner code. The helpers inside the block deliberately produce distinct codes:_bounded_namesat Line 1109 raisesRECOVERY_REQUIRED, and_validate_auxiliary_compartmentat Line 1118 raisesRECOVERY_REQUIREDorSTORE_CORRUPT._ensure_initializedreports the same on-disk conditions with those specific codes, so the code a caller sees depends on which path observed the state.Re-raise the original
StoreErrorand map only unexpected failures toUNSAFE_STATE.🐛 Proposed fix
- except StoreError: - _raise(StoreErrorCode.UNSAFE_STATE) + except StoreError: + raise + except OSError: + _raise(StoreErrorCode.UNSAFE_STATE)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/python/context_guard_receipt/store.py` around lines 1135 - 1136, Update the exception handling in _revalidate_anchors to re-raise the original StoreError unchanged, preserving specific codes such as RECOVERY_REQUIRED and STORE_CORRUPT from _bounded_names and _validate_auxiliary_compartment. Map only non-StoreError exceptions to StoreErrorCode.UNSAFE_STATE.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/context-guard-receipt/python/context_guard_receipt/identity.py`:
- Around line 860-894: Update identify_source to retain the confirmed snapshot
pair produced by the first _snapshot_with_open_root call and reuse it in the
later comparison instead of invoking a fresh pair. Preserve
_snapshot_with_open_root’s two-snapshot stability check and the existing
_observe_source_index behavior, so each identification performs three snapshots
rather than four.
In `@packages/context-guard-receipt/python/context_guard_receipt/mcp.py`:
- Around line 1149-1173: Protect the entire request-processing flow in MCP
server handle with a shared lock so all request-scoped mutations are serialized.
Ensure updates and checks involving _requests, _tool_calls, _seen_ids, and
_state—including the duplicate-ID and initialization gates—occur while that lock
is held, while preserving the existing validation, dispatch, and error behavior.
In
`@packages/context-guard-receipt/python/context_guard_receipt/reference_expiry.py`:
- Around line 1381-1400: The active-record update logic in the inspection flow
must continue persisting the maximum observed timestamp as a durable clock
high-water mark. Do not skip writes when the clock rolls back; ensure a later
is_inaccessible call, such as after inspect observed 90 then 50, still detects
the rollback and does not leave the reference active. Preserve expiry and
generation updates in the existing record publication path.
In `@packages/context-guard-receipt/python/context_guard_receipt/store.py`:
- Around line 1429-1444: Update _scan so payload.bin is opened, read, and
digest-verified only for the entry matching return_payload_for; continue
validating metadata and MACs for all entries. For non-selected entries, derive
payload_bytes from the MAC-validated byte_length field, while preserving the
existing selected payload and length consistency checks.
- Around line 1212-1215: Add an operator-invocable cleanup operation for the
non-empty staging directory under tmp, using the store’s existing locking
mechanism and recovery/error conventions; expose it through the appropriate CLI
or store API so operators can remove abandoned issue_batch state safely, and
document when and how to invoke it. Keep normal issuance behavior unchanged and
preserve RECOVERY_REQUIRED until the locked cleanup is explicitly performed.
In
`@packages/context-guard-receipt/schemas/diagnostic-ledger-inspection.schema.json`:
- Line 25: Update the entries.items.$ref in the diagnostic ledger inspection
schema to use the diagnostic-ledger-entry schema’s declared $id,
diagnostic-ledger-entry-v1.json, ensuring reference resolution matches the
registered schema identity.
In `@packages/context-guard-receipt/tests/contract/test_g010_cli.py`:
- Around line 148-164: Update run_mcp to provide closed stdin input and a finite
timeout when invoking subprocess.run, matching the safeguards used by run_cli
and run_node_cli. Preserve its existing command, environment, output capture,
and return behavior while ensuring serve_stdio cannot block indefinitely on
inherited stdin.
In `@packages/context-guard-receipt/tests/e2e/test_g001_offline_distribution.py`:
- Around line 34-59: The subprocess helpers currently allow child processes to
block indefinitely. In
packages/context-guard-receipt/tests/e2e/test_g001_offline_distribution.py,
update run_command and run_binary_command to accept the same bounded timeout and
pass it to subprocess.run; apply the equivalent change to run and run_binary in
packages/context-guard-receipt/dev/packaged_acceptance.py, preserving their
existing behavior otherwise.
---
Minor comments:
In `@packages/context-guard-receipt/dev/packaged_acceptance.py`:
- Around line 1045-1055: Update main() to use arguments.scenario when
dispatching scenarios instead of always calling distribution(). Preserve
distribution-only behavior for --scenario distribution and execute every
configured scenario for --scenario all, using the existing exception handling
and result reporting.
In `@packages/context-guard-receipt/python/context_guard_receipt/cli.py`:
- Line 47: Update the HELP usage string to label --timeout-seconds,
--max-channel-bytes, --max-total-bytes, and --limit values as <positive-integer>
instead of <positive-decimal>, matching the validation performed by
_is_positive_integer.
- Around line 524-534: In the assemble flow, move the receipt-writing block
guarded by type(receipt_path) is str—including its CliIOError handling—to
execute before _emit_payload. Only emit result.output_bytes after write_receipt
succeeds, while preserving the existing refusal-branch ordering and error return
behavior.
In `@packages/context-guard-receipt/python/context_guard_receipt/store.py`:
- Around line 1135-1136: Update the exception handling in _revalidate_anchors to
re-raise the original StoreError unchanged, preserving specific codes such as
RECOVERY_REQUIRED and STORE_CORRUPT from _bounded_names and
_validate_auxiliary_compartment. Map only non-StoreError exceptions to
StoreErrorCode.UNSAFE_STATE.
In `@packages/context-guard-receipt/schemas/source-identity.schema.json`:
- Around line 46-68: Update the pass_through branch in the source-identity
schema so its exclusion rule also rejects the source property alongside
selection and symbol. Preserve the existing pass_through reason enum and other
disposition requirements; do not change the source schema or reason values
unless producer compatibility requires separately documenting allowed behavior.
In `@packages/context-guard-receipt/scripts/verify_protected_surfaces.py`:
- Around line 94-103: Update tracked_paths to invoke git ls-files with -z and
parse stdout using NUL separators, preserving literal path names including
non-ASCII and control bytes while retaining the existing fixed-argv and --
safeguards.
In `@packages/context-guard-receipt/tests/contract/test_g005_cli.py`:
- Around line 146-172: Update
test_nonpersistent_stdin_assembly_is_exact_binary_pass_through to pass the
state_dir path via --state-dir without enabling --persist, and move the
state_dir.exists() assertion inside the TemporaryDirectory context before
cleanup. Preserve the existing binary output and empty-stderr assertions.
In `@packages/context-guard-receipt/tests/contract/test_g006_tool_schemas.py`:
- Line 260: Update the unpacking in the test around descriptor(catalog,
[item(priority=2), item(priority=1)]) to prefix the unused payload variable with
an underscore, while preserving raw for subsequent assertions and leaving the
test behavior unchanged.
In `@packages/context-guard-receipt/tests/contract/test_g007_sanitizer.py`:
- Around line 368-387: Strengthen
test_module_reload_opens_nothing_and_emits_no_logs so it detects import-time
file I/O beyond builtins.open, using broader I/O instrumentation or a subprocess
audit hook that excludes the module loader’s own source read. Preserve the
existing no-log assertion and verify that sanitizer reload performs no
additional file access.
In `@packages/context-guard-receipt/tests/contract/test_g008_runner.py`:
- Around line 2087-2123: Update the descriptor cleanup in this test around the
`descriptor` setup and `finally` block: track whether the test still owns the
descriptor, mark it as no longer owned after the runner closes it, and only call
`os.close(descriptor)` when ownership remains. Preserve the existing
`os.fstat(descriptor)` assertion verifying the runner closed it.
In `@packages/context-guard-receipt/tests/contract/test_g009_diagnostics.py`:
- Around line 88-97: Update
test_999_and_1000_basis_point_boundaries_use_integer_math so its name and cases
match the actual basis-point values produced by the 3,000-byte input, or change
the input and predicted sizes to reach exactly 999 basis points and verify the
inclusive threshold. Preserve assertions for both below-threshold and threshold
behavior.
In `@packages/context-guard-receipt/tests/contract/test_g012_mcp_cli.py`:
- Around line 103-113: Ensure every piped child process is bounded, drained, and
reaped: in packages/context-guard-receipt/tests/contract/test_g012_mcp_cli.py
lines 103-113, replace process.wait and manual stderr reading with
process.communicate(timeout=20), killing the process and communicating again on
subprocess.TimeoutExpired; in
packages/context-guard-receipt/tests/contract/test_g010_cli.py lines 148-164,
update run_mcp’s subprocess.run call to use stdin=subprocess.DEVNULL and
timeout=30; in packages/context-guard-receipt/tests/contract/test_g010_twin.py
lines 723-743, wrap the pool.map communicate call in try/finally and kill each
process in processes whose poll() remains None.
In `@packages/context-guard-receipt/tests/e2e/test_g012_mcp_stdio.py`:
- Around line 88-99: Update the close method to use self.process.communicate()
instead of calling wait before reading stderr, ensuring remaining stdout/stderr
are drained while the child exits. Preserve the returned stderr diagnostics and
nonzero-returncode assertion, and ensure cleanup still occurs if process
completion raises.
In `@tests/test_context_guard_receipt_suite.py`:
- Around line 33-34: Update the test-count assertion in the relevant test method
to convert regex group 1 to an integer and require it to meet the expected
minimum companion-test count of 513, while preserving the existing
match-presence assertion and stderr diagnostic.
---
Nitpick comments:
In `@packages/context-guard-receipt/bin/launcher.cjs`:
- Around line 616-620: Update launch to guard the fs.realpathSync(entryFilename)
call before deriving packageRoot, catching resolution failures and returning
launcherError('integrity_failure', 70). Preserve the existing validatePackage
flow for successfully resolved entry files so all failures emit the structured
launcher response.
- Around line 182-184: Remove the unused canonicalJson function entirely from
the launcher module, since there are no repository call sites and its
implementation is incorrect for nested object keys.
- Around line 99-168: Add a prepublish validation that compares shared entries
from package-files.json against TRUSTED_PAYLOAD_DIGESTS, excluding
bin/launcher.cjs because it is intentionally absent. Ensure prepublish_check.py
invokes the package check, and document the command used to regenerate
TRUSTED_PAYLOAD_DIGESTS when package contents change.
In `@packages/context-guard-receipt/dev/package_check.py`:
- Around line 153-163: Extract the shared {"0644": {"0600", "0640", "0644"},
"0755": {"0700", "0750", "0755"}} validation into a helper such as
check_portable_mode, then update both portable_source_mode and
bounded_regular_content to use it. Preserve each caller’s existing error
message/context, including bounded_regular_content’s subject-specific message,
so both paths enforce the same portability rules.
In `@packages/context-guard-receipt/dev/packaged_acceptance.py`:
- Around line 1049-1053: Update the exception handler in main() around
distribution() to also catch IndexError and KeyError, preserving the existing
packaged acceptance failed message and return code for these malformed
process-output cases.
In `@packages/context-guard-receipt/python/context_guard_receipt/assembly.py`:
- Around line 388-399: Update the pass-through flow by adding a helper alongside
_pass_through that accepts the already computed costs and decision and uses them
when building the exact-payload result. Replace the _pass_through calls in
assemble_evidence, assemble_evidence_pack, and assemble_blueprint with this
helper where real route data is available, preserving each caller’s existing
payload, kind, and reason.
In `@packages/context-guard-receipt/python/context_guard_receipt/cli_io.py`:
- Around line 25-26: Update the `_raise` return annotation from `None` to
`NoReturn`, importing `NoReturn` from `typing` as needed, so type checkers
recognize that the function never returns and correctly narrow callers such as
the `chunk` and `raw` flows.
In `@packages/context-guard-receipt/python/context_guard_receipt/cli.py`:
- Around line 669-682: Update _parse_run_invocation to return the validated
timeout_seconds, channel_bytes, and total_bytes values alongside its existing
result, then modify _run to read options["timeout_seconds"],
options["channel_bytes"], and options["total_bytes"] when constructing
RunnerLimits. Remove the duplicate defaults and integer conversions from _run so
RunnerLimits always uses the values validated by _parse_run_invocation.
- Around line 330-341: Update _option_values to handle a trailing non-flag
option without raising IndexError, returning None for the malformed argument
list; adjust its return annotation and callers as needed to propagate this
result. Preserve existing parsing for valid flag and option-value pairs, and
document the validation invariant if callers still rely on pre-validation.
In `@packages/context-guard-receipt/python/context_guard_receipt/contracts.py`:
- Around line 34-36: Update canonical_json to match canonical._encode_validated
by using UTF-8 characters directly with ensure_ascii=False, preserving its
compact sorted output and trailing newline; also set allow_nan=False so
non-standard numeric tokens are rejected and output remains parseable by
parse_canonical_json_bytes.
In
`@packages/context-guard-receipt/python/context_guard_receipt/diagnostic_ledger.py`:
- Around line 984-988: Remove the redundant _AUXILIARY_TEMP_NAME.fullmatch test
from _root_names and collapse the unknown-name handling to a single
RECOVERY_REQUIRED raise, preserving the existing behavior for all unknown names.
- Around line 1327-1352: Update DiagnosticLedger._operation so self._thread_lock
is held only while validating descriptors and incrementing _active_operations,
then released before yielding to the operation body; retain the finally cleanup
under the lock and preserve close-request handling when the active-operation
count reaches zero, matching the surrounding ExecutionTwin._operation and
ReferenceExpiryRegistry._operation patterns.
In `@packages/context-guard-receipt/python/context_guard_receipt/diagnostics.py`:
- Around line 254-257: Update the zip call in the reused calculation to pass
strict=False explicitly, preserving the intended truncation when current_hashes
and previous_hashes have different lengths and resolving Ruff B905.
In
`@packages/context-guard-receipt/python/context_guard_receipt/execution_twin.py`:
- Around line 551-560: In the predicate handling around the kind branches,
consolidate the duplicated _validate_relative_path call and ExecutionTwinError
rejection into a single validation step shared by path_absent and the else path.
Keep the branch-specific regular_file_equals checks applied afterward where
required, preserving existing behavior for all predicate kinds.
- Around line 416-424: Extract the repeated seven-field stat tuple construction
into one module-level helper, such as _status_identity, near the filesystem
helpers. Replace the identity lambda in _read_named_file, the status_identity
lambda in _scan_committed, and the _PathObserver._status_identity implementation
with calls or delegation to this shared helper, preserving the existing tuple
contents and behavior.
In `@packages/context-guard-receipt/python/context_guard_receipt/expansion.py`:
- Around line 390-391: Add a module-level logger in expansion.py and update both
Exception handlers around the store-unavailable refusals to log the caught
exception type at debug level before returning _refused("store_unavailable").
Keep the refusal value and externally visible error details unchanged.
- Around line 31-34: Centralize capability-format validation in store.py by
exporting one validator that owns the prefix, length, alphabet, and
prefix-length derivation. Remove the duplicated constants and hardcoded slice
length from expansion.py, and update its validation path and mcp.py to call the
shared validator instead of defining or reimplementing the format locally.
In `@packages/context-guard-receipt/python/context_guard_receipt/identity.py`:
- Around line 921-926: Rename the identity module function
_repository_exclusion_snapshot to repository_exclusion_snapshot, expose the new
name through __all__, and update store.py to import and use
repository_exclusion_snapshot so the cross-module contract references the public
symbol.
- Around line 1215-1282: The candidate validation logic in
_complete_symbol_range is too large and mixed with unrelated checks; extract the
candidate block into a _validated_candidates(...) helper. Move candidate
envelope, field, digest, UTF-8, identity, occurrence-order, and
matching-candidate checks into that helper, returning either the existing status
string or the match count, then have _complete_symbol_range preserve the current
status handling and require exactly one match.
In `@packages/context-guard-receipt/python/context_guard_receipt/mcp.py`:
- Around line 1251-1263: Remove both _revalidate_root calls from the outer
dispatch wrapper surrounding the receipt_assemble, receipt_expand,
receipt_tool_select, and _inspect handler dispatch. Preserve the existing pre-
and post-operation validation brackets inside each handler, without adding
replacement snapshots or digest walks in this wrapper.
- Around line 990-1006: Update _record_tool_references so _tool_references
cannot grow without bound: remove entries whose capabilities no longer resolve
using the existing capability-resolution mechanism, and enforce MAX_ARTIFACTS
when adding catalog and deferred references. Preserve valid references while
evicting stale or excess entries.
In
`@packages/context-guard-receipt/python/context_guard_receipt/reference_expiry.py`:
- Line 815: Remove the redundant self._closed = False assignment from
_open_axis, leaving open responsible for the closed-state transition after the
limits comparison. Preserve the existing descriptor cleanup behavior when that
comparison raises so the state remains consistent with the established open
flow.
- Line 1: Update the module docstring in reference_expiry.py to replace
“removable expiry” with wording that accurately describes expiry records as
retained, including expired and revoked records that remain counted toward
max_references. Do not imply that unregistering or pruning is supported.
In `@packages/context-guard-receipt/python/context_guard_receipt/sanitizer.py`:
- Around line 1181-1184: Rename the loop variable in the `_remote_url_values`
parsing loop so it no longer shadows the imported `dataclasses.field`; update
its references in the value extraction and append logic while preserving the
existing tuple output.
- Around line 1356-1371: Document in the sanitization summary definition that
invalid_utf8_bytes and escaped_control_characters are counted from the
pre-redaction payload, while output_bytes reflects the post-render emitted
payload. Update the relevant SanitizationSummary docstring or add a concise
comment near the counter calculation, without changing the existing counting
behavior.
In `@packages/context-guard-receipt/python/context_guard_receipt/store.py`:
- Around line 826-832: Update the constructor __init__ to initialize
_state_path, _state_fd, _lock_fd, _key, _namespace_id, _limits, _store_fd,
_commits_fd, _temp_fd, and all five *_anchor attributes to None, alongside the
existing fields. Then update _close_descriptors and _operation to use these
initialized attributes directly instead of getattr-based existence checks,
preserving their existing partial-state guards.
- Around line 659-671: Update _mac_document to avoid mutating the
caller-provided value dict: create a separate document copy, remove any existing
integrity_hmac_sha256 from that copy, assign the computed MAC there, and
canonicalize and return the copy. Preserve the existing MAC calculation and
limits handling.
- Around line 207-222: Update the `_raise` helper’s return annotation to
`NoReturn`, adding the necessary typing import if needed, so type checkers
recognize that `_descriptor_status` and `_physical_directory_ancestry` terminate
on invalid states. Do not change the descriptor validation or address the
separate object-to-int mismatch.
In `@packages/context-guard-receipt/python/context_guard_receipt/tool_schemas.py`:
- Around line 190-191: Update _reject in
packages/context-guard-receipt/python/context_guard_receipt/tool_schemas.py at
lines 190-191 to return NoReturn and import NoReturn from typing; update
_reject_capability in
packages/context-guard-receipt/python/context_guard_receipt/mcp.py at lines
298-299 likewise, so type checkers recognize both helpers never return.
In `@packages/context-guard-receipt/schemas/evidence-pack.schema.json`:
- Line 40: Update the payload_b64u schema property to use the grouped base64url
pattern already defined by diagnostics-request.schema.json, rejecting unpadded
strings whose length is 1 modulo 4 while preserving the existing character and
maxLength constraints.
In `@packages/context-guard-receipt/schemas/expansion-envelope.schema.json`:
- Around line 23-27: Update the schema’s byte-offset integer definitions for
start_byte, end_byte, occurrence, the range selection branch, evidence offsets,
payload_start_byte, and payload_end_byte to include the established package
maxima: use 900000 where offsets are capped at that value and 899999 where
sibling segment schemas use that exclusive-end bound. Preserve the existing
minimum and integer constraints, and apply the same bounds consistently across
all named fields.
In `@packages/context-guard-receipt/schemas/tool-schema-receipt.schema.json`:
- Around line 6-29: Update the top-level schema’s disposition/reason validation
to use oneOf branches that bind deferred only to beneficial, refused only to
secret or refuse, and pass_through to all remaining emitted reasons. Preserve
the existing disposition and reason enums while ensuring each valid
disposition-reason pairing is enforced.
In `@packages/context-guard-receipt/schemas/typed-blueprint.schema.json`:
- Around line 41-62: Update the obligations item schema to compose the shared
fields from `#/`$defs/reference via allOf instead of redeclaring byte_length,
capability, content_sha256, and subject_identity_sha256. Remove
additionalProperties: false from the referenced definition and use
unevaluatedProperties: false on the composed item so the reference and
obligation-specific properties are both accepted while unknown properties remain
rejected.
In `@packages/context-guard-receipt/scripts/verify_protected_surfaces.py`:
- Around line 180-190: Update verify_stage2_artifact_integrity to return the
hash-verified artifact bytes, then pass those bytes into
verify_unsupported_semantics instead of reading the semantic records from disk
again. Parse the returned bytes for host-observability.json and
verification-record.json while preserving the existing semantic expectations and
verification errors.
In `@packages/context-guard-receipt/tests/adversarial/test_g012_mcp_limits.py`:
- Around line 322-335: Update the receipt_tool_select baseline in the test loop
to use a valid descriptor accepted without forbidden fields, and add a baseline
call assertion confirming success before iterating over forbidden. Keep the
existing invalid_arguments assertions so failures specifically demonstrate
rejection of root, state_dir, and receipt.
In
`@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py`:
- Around line 322-334: Update copy_runtime_with_current_launcher to call
require_distribution() before copying or otherwise using the runtime, ensuring
the three direct subprocess.Popen tests, including
test_launcher_reports_closed_stdout_as_bounded_delivery_failure, fail with the
established Node.js availability assertion instead of constructing Path(NODE)
when NODE is absent.
In `@packages/context-guard-receipt/tests/contract/test_g002_canonical.py`:
- Around line 345-346: Update the subtest context in the operations loop to
label each case with its stable iteration index rather than the operation lambda
object. Preserve the existing operation execution and use the index as the
subtest identifier so failure output is deterministic and identifies the failing
case.
In `@packages/context-guard-receipt/tests/contract/test_g002_protection.py`:
- Around line 19-31: Move the shared EVIDENCE_BOUNDARY fixture into the contract
package’s __init__.py, then remove the duplicated definitions from
test_g002_protection.py, test_g003_identity.py, and test_g004_store.py and
import the centralized constant in each test module. Keep BOUNDARY_REQUIRED
derived from the imported EVIDENCE_BOUNDARY.
In `@packages/context-guard-receipt/tests/contract/test_g004_store.py`:
- Around line 638-647: Bind the enclosing loop variables in each closure using
keyword defaults: update observed_fstat with real_fstat, foreign_owner, and
target_identity in
packages/context-guard-receipt/tests/contract/test_g004_store.py lines 638-647;
update the retrieve lambda with issued at lines 849-855; update
fail_renamed_parent with failed_parent and real_fsync at lines 1236-1239; and
update the JSONLimits lambda with arguments in
packages/context-guard-receipt/tests/contract/test_g002_canonical.py lines
311-315, matching the existing idiom.
In `@packages/context-guard-receipt/tests/contract/test_g005_evidence_pack.py`:
- Around line 199-207: Introduce a small base64url padding helper in
test_g005_evidence_pack.py that computes the required padding from the encoded
string length, and use it for both payload_b64u decoding assertions in the
deferred-result checks. Replace the fixed "==" suffixes while preserving the
existing decoded-value assertions.
In `@packages/context-guard-receipt/tests/contract/test_g005_expansion.py`:
- Around line 125-143: Update ResolvingStore.resolve to compute and return the
actual SHA-256 digest of self.request.payload in payload_sha256 instead of the
constant value, preserving the existing fixture behavior otherwise. If digest
verification is part of the contract, add a separate mismatch fixture that
deliberately returns a digest inconsistent with its payload.
In `@packages/context-guard-receipt/tests/contract/test_g005_schemas.py`:
- Around line 103-104: Update the branch lookup in the test around
schema["$defs"]["selection"]["oneOf"] to locate the selection variant whose kind
property has the exact "symbol" const, instead of accessing oneOf[2]. Keep the
existing evidence assertion against that discriminator-selected private-symbol
branch.
In `@packages/context-guard-receipt/tests/contract/test_g006_tool_schemas.py`:
- Around line 70-76: Update the synthetic handle generation in RecordingStore to
keep every repeated character within the capability alphabet [A-Za-z0-9_-],
including when requests exceed 26; preserve the existing cgr1p_ prefix and
43-character handle length.
In `@packages/context-guard-receipt/tests/contract/test_g008_cli.py`:
- Around line 179-187: Move the if/then evaluation in schema_accepts to the
node-level logic alongside allOf and oneOf, so conditional schemas are checked
regardless of whether they declare object properties or required fields. Remove
the duplicated conditional block from the object-specific branch while
preserving the existing condition/consequence acceptance behavior.
In `@packages/context-guard-receipt/tests/contract/test_g008_expansion.py`:
- Around line 298-299: Update the zero_length and oversized_length fixtures in
the relevant contract test so each violates only its named validation rule: use
a zero declared length with no payload for zero_length, and a declared length of
4097 with exactly 4097 payload bytes for oversized_length. Preserve the
surrounding frame structure and assertions.
In `@packages/context-guard-receipt/tests/contract/test_g008_runner.py`:
- Around line 503-530: Update
test_signal_installation_restores_prior_handlers_after_injected_interrupt to
capture the existing SIGINT and SIGTERM handlers before guard.install(), then
assert after cleanup that signal.getsignal() matches those captured handlers
rather than fixed defaults. Apply the same prior-handler assertion change to the
duplicate assertion pair in the related test near the other occurrence.
In `@packages/context-guard-receipt/tests/contract/test_g010_cli.py`:
- Around line 328-344: Update the return-code assertions in the test method
containing the `calls` tuple to assert each named result individually rather
than using `calls[1:6]` and `calls[7]`. Explicitly validate `captured`,
`expanded`, `tool_assembled`, `tool_expanded`, `firewall`, `run_cli("inspect",
"boundary")`, `run_cli("inspect", "diagnostic-ledger", ...)`, and
`run_mcp(...)`; leave `assembled` and `served` unchecked only if intentional,
and document that exception with a comment.
In `@packages/context-guard-receipt/tests/contract/test_g010_twin.py`:
- Around line 216-223: Update the predicate construction to call the already
imported hashlib module directly instead of using __import__("hashlib") for the
expected_content_sha256 value, matching the existing usage in the test file.
In `@packages/context-guard-receipt/tests/contract/test_g011_cli.py`:
- Around line 29-33: Remove the local canonical_json helper and import
canonical_json_bytes from context_guard_receipt.canonical alongside the existing
package imports. Update all usages in this test file to call
canonical_json_bytes so the CLI tests use the implementation’s canonical
serialization contract.
In `@packages/context-guard-receipt/tests/contract/test_g011_reference_expiry.py`:
- Around line 698-702: In the test setup around original, displaced, and
replacement, remove the duplicate replacement path alias and reuse original
after os.rename(original, displaced) when creating the replacement directory.
In `@packages/context-guard-receipt/tests/contract/test_g012_mcp.py`:
- Around line 121-144: Extend
test_in_memory_capabilities_expire_monotonically_and_are_process_local with a
backward-clock case: after recording a later observed time, move observed[0]
below the high-water mark and assert that resolving the handle raises
StoreError, covering InMemoryCapabilityStore._now’s unsafe-state behavior.
In `@packages/context-guard-receipt/tests/contract/test_g013_package_audit.py`:
- Around line 296-298: Replace the dict(os.environ) setup in the affected test
cases with a minimal explicit environment containing only the required LANG,
PATH, and PYTHONDONTWRITEBYTECODE values, while preserving the explicit
CONTEXT_GUARD_RECEIPT_PYTHON assignment. Apply the same change to the repeated
environment setups near the other referenced launcher invocations, and add any
ambient variable such as HOME only if the launcher explicitly requires it.
- Around line 272-283: Update
test_restricted_source_modes_pack_through_a_normalized_real_staging_tree to skip
when npm is unavailable, using the module’s existing Node/npm availability
symbol or unittest skip mechanism before calling validate_npm_tarball. Apply the
same skip behavior to the related tests that currently call
assertIsNotNone(node), replacing their local node lookup with the shared NODE
value and skipIf guard as requested.
In `@packages/context-guard-receipt/tests/e2e/test_g001_offline_distribution.py`:
- Around line 829-830: Rename the loop variable in the iteration over
twin_result and twin_snapshot from payload to a distinct name such as
twin_payload, and update its evidence_boundary assertion accordingly so the
earlier payload evidence bytes binding remains intact.
In `@tests/test_contextguard_stage2_feasibility.py`:
- Around line 208-227: Update provider_free_changed_paths to catch
subprocess.CalledProcessError from the git merge-base invocation and raise an
AssertionError that identifies the unavailable base_ref, preserving the existing
successful merge-base validation.
- Around line 293-302: Make surface_inventory a pure builder that processes
every path in visible_paths without applying exclusion checks. Retain the
complete exclusion policy, including the receipt prefix, solely in
is_legacy_production_path, and ensure both callers continue filtering their
inputs before invoking surface_inventory.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| def _snapshot_with_open_root( | ||
| root_path: str, | ||
| root_status: os.stat_result, | ||
| git_executable: object, | ||
| limits: IdentityLimits, | ||
| ) -> dict[str, object]: | ||
| first = _snapshot_once(root_path, root_status, git_executable, limits) | ||
| second = _snapshot_once(root_path, root_status, git_executable, limits) | ||
| if ( | ||
| first["instance"] != second["instance"] | ||
| or first["logical_state"] != second["logical_state"] | ||
| ): | ||
| return _unresolved_snapshot(root_path, root_status, "git_state_changed") | ||
| return second | ||
|
|
||
|
|
||
| def snapshot_repository( | ||
| root: object, | ||
| git_executable: object = None, | ||
| limits: IdentityLimits = _DEFAULT_LIMITS, | ||
| ) -> dict[str, object]: | ||
| """Capture bounded Git metadata plus a local repository instance identity.""" | ||
|
|
||
| checked_limits = _require_limits(limits) | ||
| root_path = _root_path(root) | ||
| root_descriptor = _open_root(root_path) | ||
| try: | ||
| root_status = os.fstat(root_descriptor) | ||
| result = _snapshot_with_open_root( | ||
| root_path, root_status, git_executable, checked_limits | ||
| ) | ||
| _root_is_unchanged(root_path, root_status) | ||
| return result | ||
| finally: | ||
| os.close(root_descriptor) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Consider reducing repeated full-repository Git work per identification.
_snapshot_with_open_root runs _snapshot_once twice. _snapshot_once runs up to eight git processes for a worktree, including git status --porcelain=v1 -z --untracked-files=all and git diff --binary --full-index. identify_source calls _snapshot_with_open_root twice, at Line 1326 and Line 1393, and also calls _observe_source_index twice. One identify_source call therefore spawns on the order of 30 git processes and performs four full worktree scans.
On a large repository each scan reads the whole worktree. The MCP surface can drive this path repeatedly, so the cost is attacker-influenced in volume.
The double execution is the stability check, so do not delete it. Instead reuse results within one call: pass the confirmed post-read snapshot pair into the comparison at Line 1393 rather than recomputing a fresh pair, so the sequence performs three snapshots instead of four.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/context-guard-receipt/python/context_guard_receipt/identity.py`
around lines 860 - 894, Update identify_source to retain the confirmed snapshot
pair produced by the first _snapshot_with_open_root call and reuse it in the
later comparison instead of invoking a fresh pair. Preserve
_snapshot_with_open_root’s two-snapshot stability check and the existing
_observe_source_index behavior, so each identification performs three snapshots
rather than four.
| def handle(self, request: object) -> dict[str, object] | None: | ||
| self._requests += 1 | ||
| if self._requests > MAX_FRAMES: | ||
| return self._error(None, -32600, "Request limit reached") | ||
| try: | ||
| _walk_json(request) | ||
| except Exception: | ||
| return self._error(None, -32600, "Invalid Request") | ||
| if ( | ||
| type(request) is not dict | ||
| or set(request) - {"id", "jsonrpc", "method", "params"} | ||
| or request.get("jsonrpc") != "2.0" | ||
| or type(request.get("method")) is not str | ||
| or ("params" in request and type(request["params"]) is not dict) | ||
| ): | ||
| return self._error(None, -32600, "Invalid Request") | ||
| notification = "id" not in request | ||
| request_id = request.get("id") | ||
| if not notification: | ||
| if not _valid_id(request_id): | ||
| return self._error(None, -32600, "Invalid Request") | ||
| marker = (type(request_id), request_id) | ||
| if marker in self._seen_ids: | ||
| return self._error(request_id, -32600, "Duplicate request id") | ||
| self._seen_ids.add(marker) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the request-scoped server state with a lock.
handle mutates self._requests, self._seen_ids, self._state, and self._tool_calls (Line 1244) without synchronization. The non-blocking self._call_lock.acquire at Line 1248 shows that concurrent calls to handle are expected, and the counters are updated before that acquire. Concurrent callers then produce these effects:
self._tool_calls += 1andself._requests += 1are non-atomic read-modify-write operations, so lost updates let a caller exceedMAX_TOOL_CALLSandMAX_FRAMES.- The
_seen_idsmembership test and insert form a check-then-act, so two threads can both accept the same request id. - The
_statetest and assignment are also a check-then-act, so twoinitializerequests can both pass thePRE_INITgate.
InMemoryCapabilityStore holds its own RLock, so only the server state is affected. Take one lock for the whole handle body, or protect the counters and _seen_ids before the dispatch.
🔒 Proposed fix to serialize request-scoped state
self._seen_ids: set[tuple[type, object]] = set()
self._call_lock = threading.Lock()
+ self._state_lock = threading.Lock() def handle(self, request: object) -> dict[str, object] | None:
- self._requests += 1
- if self._requests > MAX_FRAMES:
- return self._error(None, -32600, "Request limit reached")
+ with self._state_lock:
+ self._requests += 1
+ if self._requests > MAX_FRAMES:
+ return self._error(None, -32600, "Request limit reached") if not notification:
if not _valid_id(request_id):
return self._error(None, -32600, "Invalid Request")
marker = (type(request_id), request_id)
- if marker in self._seen_ids:
- return self._error(request_id, -32600, "Duplicate request id")
- self._seen_ids.add(marker)
+ with self._state_lock:
+ if marker in self._seen_ids:
+ return self._error(request_id, -32600, "Duplicate request id")
+ self._seen_ids.add(marker)🧰 Tools
🪛 Ruff (0.16.1)
[warning] 1155-1155: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/context-guard-receipt/python/context_guard_receipt/mcp.py` around
lines 1149 - 1173, Protect the entire request-processing flow in MCP server
handle with a shared lock so all request-scoped mutations are serialized. Ensure
updates and checks involving _requests, _tool_calls, _seen_ids, and
_state—including the duplicate-ID and initialization gates—occur while that lock
is held, while preserving the existing validation, dispatch, and error behavior.
| for reference_id in sorted(scan.records): | ||
| current = scan.records[reference_id] | ||
| if current["status"] == "active": | ||
| previous_observed = int(current["updated_at_unix_ms"]) | ||
| clock_rolled_back = observed < previous_observed | ||
| effective_observed = max(observed, previous_observed) | ||
| if not clock_rolled_back and effective_observed == previous_observed: | ||
| continue | ||
| record = dict(current) | ||
| record["integrity_hmac_sha256"] = "" | ||
| record["updated_at_unix_ms"] = effective_observed | ||
| if ( | ||
| clock_rolled_back | ||
| or effective_observed >= current["expires_at_unix_ms"] | ||
| ): | ||
| record["generation"] = int(current["generation"]) + 1 | ||
| record["status"] = "expired" | ||
| updates[reference_id] = record | ||
| if updates: | ||
| scan = self._publish_records(updates, scan) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the reference-expiry contract tests for assertions on updated_at_unix_ms after inspect().
set -euo pipefail
fd -t f 'test_g011_reference_expiry.py' -x rg -n -C 6 'inspect\(|updated_at_unix_ms|reference_summaries' {}
fd -t f 'test_g011_cli.py' -x rg -n -C 6 'reference-expiry|updated_at_unix_ms' {}Repository: ictechgy/context-guard
Length of output: 21512
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=$(fd -t f 'reference_expiry.py' packages/context-guard-receipt/python)
printf '%s\n' "$file"
ast-grep outline "$file" --match 'class ReferenceExpiryRegistry' --view expanded
sed -n '1180,1435p' "$file"
test_file=$(fd -t f 'test_g011_reference_expiry.py' tests)
sed -n '330,455p' "$test_file"Repository: ictechgy/context-guard
Length of output: 13499
🏁 Script executed:
#!/bin/bash
set -euo pipefail
test_file=$(fd -t f 'test_g011_reference_expiry.py' . | head -n 1)
printf 'TEST_FILE=%s\n' "$test_file"
sed -n '330,455p' "$test_file"
printf '\nRelated high-water and inspect assertions:\n'
rg -n -C 8 'high.?water|non.?decreasing|inspect\(.*observed_at_unix_ms|updated_at_unix_ms' \
. -g '*.py' | head -n 320Repository: ictechgy/context-guard
Length of output: 45061
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
def current_inspect(updated, observed, expires):
rolled_back = observed < updated
effective = max(observed, updated)
if not rolled_back and effective == updated:
return updated, "active"
if rolled_back or effective >= expires:
return effective, "expired"
return effective, "active"
def proposed_inspect(updated, observed, expires):
rolled_back = observed < updated
effective = max(observed, updated)
expiring = rolled_back or effective >= expires
if not expiring:
return updated, "active"
return effective, "expired"
def is_inaccessible(updated, observed, expires):
rolled_back = observed < updated
effective = max(observed, updated)
return rolled_back or effective >= expires
updated_current, status_current = current_inspect(10, 90, 100)
updated_proposed, status_proposed = proposed_inspect(10, 90, 100)
print("current inspect(90):", updated_current, status_current)
print("proposed inspect(90):", updated_proposed, status_proposed)
print("current then is_inaccessible(50):",
is_inaccessible(updated_current, 50, 100))
print("proposed then is_inaccessible(50):",
is_inaccessible(updated_proposed, 50, 100))
assert updated_current == 90
assert updated_proposed == 10
assert is_inaccessible(updated_current, 50, 100) is True
assert is_inaccessible(updated_proposed, 50, 100) is False
PY
file=packages/context-guard-receipt/python/context_guard_receipt/reference_expiry.py
sed -n '1070,1195p' "$file"Repository: ictechgy/context-guard
Length of output: 5139
Preserve the persistent clock high-water mark.
The write amplification is real, but skipping active-record updates breaks rollback protection. After inspect(observed_at_unix_ms=90) follows registration at 10, is_inaccessible(observed_at_unix_ms=50) can incorrectly leave the reference active. Keep persisting forward high-water updates, or add an equivalent persistent mechanism before reducing these writes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/context-guard-receipt/python/context_guard_receipt/reference_expiry.py`
around lines 1381 - 1400, The active-record update logic in the inspection flow
must continue persisting the maximum observed timestamp as a durable clock
high-water mark. Do not skip writes when the clock rolls back; ensure a later
is_inaccessible call, such as after inspect observed 90 then 50, still detects
the rollback and does not leave the reference active. Preserve expiry and
generation updates in the existing record publication path.
| if _bounded_names( | ||
| temp_fd, 1, overflow=StoreErrorCode.RECOVERY_REQUIRED | ||
| ): | ||
| _raise(StoreErrorCode.RECOVERY_REQUIRED) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for a recovery command that clears the store staging directory.
set -euo pipefail
rg -n -C5 --type=py 'RECOVERY_REQUIRED|recovery_required|recover' packages/context-guard-receipt/python
rg -n -C3 --type=py '_TEMP_NAME|"tmp"' packages/context-guard-receipt/python
rg -n -C3 'recover' packages/context-guard-receipt/README.md || trueRepository: ictechgy/context-guard
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository files near the package and CLI/docs ---'
git ls-files | rg '(^|/)(README(\.md)?|pyproject\.toml|setup\.cfg|setup\.py|.*\.py|.*\.md)$' | rg 'context-guard-receipt|context_guard|README|docs|cli|command' | head -300
printf '%s\n' '--- recovery-related symbols and commands across the repository ---'
rg -n -i -C3 --glob '!*.lock' --glob '!*.jsonl' \
'RECOVERY_REQUIRED|recovery_required|recover|stale staging|staging director|temporary director|clear.*tmp|cleanup.*tmp|tmp.*clear' .
printf '%s\n' '--- receipt-store public methods and package exports ---'
rg -n -C3 \
'def (issue|inspect|summary|recover|cleanup|repair)|class .*Store|context_guard_receipt\.store|from .*store import|StoreSummary' \
packages/context-guard-receiptRepository: ictechgy/context-guard
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package README recovery and store instructions ---'
rg -n -i -C8 \
'recovery_required|recover|recovery|tmp|temporary|staging|inspect|issue|store' \
packages/context-guard-receipt/README.md
printf '%s\n' '--- receipt CLI command definitions and dispatch ---'
ast-grep outline packages/context-guard-receipt/python/context_guard_receipt/cli.py
rg -n -C6 \
'argparse|add_parser|add_subparsers|issue|inspect|summary|recover|repair|cleanup|Store\(' \
packages/context-guard-receipt/python/context_guard_receipt/cli.py
printf '%s\n' '--- package exports and store public API ---'
rg -n -C5 \
'Store|issue|inspect|summary|recover|repair|cleanup|__all__' \
packages/context-guard-receipt/python/context_guard_receipt/__init__.py \
packages/context-guard-receipt/python/context_guard_receipt/store.py
printf '%s\n' '--- tests that create temporary entries or assert recovery behavior ---'
rg -n -C8 \
'RECOVERY_REQUIRED|recovery_required|temp|tmp|temporary|staging|issue\(' \
packages/context-guard-receipt/tests/contract/test_g004_store.py \
packages/context-guard-receipt/tests/contract/test_g005_cli.py \
packages/context-guard-receipt/tests/contract/test_g006_cli.py \
packages/context-guard-receipt/tests/contract/test_g008_cli.py \
packages/context-guard-receipt/tests/contract/test_g009_cli.py \
packages/context-guard-receipt/tests/contract/test_g010_cli.py \
packages/context-guard-receipt/tests/contract/test_g011_cli.py \
packages/context-guard-receipt/tests/contract/test_g012_mcp_cli.pyRepository: ictechgy/context-guard
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README recovery references ---'
rg -n -i -C5 \
'recovery_required|recover|recovery|staging|temporary|tmp' \
packages/context-guard-receipt/README.md || true
printf '%s\n' '--- CLI parser and command names ---'
rg -n -C4 \
'add_parser|add_subparsers|set_defaults|issue|inspect|expand|assemble|store|recover|repair|cleanup' \
packages/context-guard-receipt/python/context_guard_receipt/cli.py
printf '%s\n' '--- CLI public entry points ---'
sed -n '1,260p' packages/context-guard-receipt/python/context_guard_receipt/cli.py
printf '%s\n' '--- Store recovery-related implementation and constants ---'
rg -n -C12 \
'RECOVERY_REQUIRED|_TEMP_NAME|_TEMP_DIRECTORY|temp_fd|temporary_id|os\.mkdir|os\.rename|shutil|unlink|rmdir|removedir|cleanup|recover' \
packages/context-guard-receipt/python/context_guard_receipt/store.py
printf '%s\n' '--- Store tests with explicit recovery assertions ---'
rg -n -C12 \
'RECOVERY_REQUIRED|recovery_required|recovery required|temp_fd|temporary_id|_TEMP_NAME|tmp' \
packages/context-guard-receipt/tests/contract/test_g004_store.py || trueRepository: ictechgy/context-guard
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- complete issue-batch write, publish, and cleanup path ---'
sed -n '1190,1365p' packages/context-guard-receipt/python/context_guard_receipt/store.py
printf '%s\n' '--- complete initialization exception path ---'
sed -n '974,1055p' packages/context-guard-receipt/python/context_guard_receipt/store.py
printf '%s\n' '--- inspection implementation ---'
sed -n '1560,1605p' packages/context-guard-receipt/python/context_guard_receipt/store.py
printf '%s\n' '--- all filesystem removal operations in the store module ---'
rg -n -C4 \
'os\.(unlink|rmdir|remove)|unlinkat|rmdir|removedir|shutil|rename\(' \
packages/context-guard-receipt/python/context_guard_receipt/store.pyRepository: ictechgy/context-guard
Length of output: 14014
Add a locked recovery path for non-empty tmp.
If issue_batch fails before os.rename, its staging directory remains in tmp; the finally block closes only batch_fd. Every later issuance then raises RECOVERY_REQUIRED. No CLI command or store API removes the directory. Add an operator-invocable, lock-protected cleanup operation and document its safe use.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/context-guard-receipt/python/context_guard_receipt/store.py` around
lines 1212 - 1215, Add an operator-invocable cleanup operation for the non-empty
staging directory under tmp, using the store’s existing locking mechanism and
recovery/error conventions; expose it through the appropriate CLI or store API
so operators can remove abandoned issue_batch state safely, and document when
and how to invoke it. Keep normal issuance behavior unchanged and preserve
RECOVERY_REQUIRED until the locked cleanup is explicitly performed.
| def _scan(self, *, return_payload_for: str | None = None) -> _Usage: | ||
| commit_names = sorted( | ||
| _bounded_names(self._commits_fd, self._limits.max_artifacts) | ||
| ) | ||
| artifacts = 0 | ||
| payload_bytes = 0 | ||
| lookup_ids: set[str] = set() | ||
| selected: StoredArtifact | None = None | ||
| for commit_name in commit_names: | ||
| if _HEX_256.fullmatch(commit_name) is None: | ||
| _raise(StoreErrorCode.STORE_CORRUPT) | ||
| commit_fd = _open_directory_at(self._commits_fd, commit_name) | ||
| try: | ||
| manifest_raw = _read_named_file( | ||
| commit_fd, _COMMIT_MANIFEST_NAME, _COMMIT_DOCUMENT_BYTES | ||
| ) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
_scan reads every stored payload on every lookup.
_scan walks all commits, and for each entry it reads payload.bin fully at Line 1509 and verifies its digest, even when return_payload_for names a single entry. _resolve_capability_record calls _scan for one handle, so a single retrieve reads up to max_total_artifact_bytes, which defaults to 64 MiB, and verifies up to 1024 record MACs. _issue_batch calls _scan twice, at Line 1216 and Line 1317.
The store is local and bounded, so this does not break correctness. It does make every retrieval cost proportional to total store size.
Read the payload only when the entry is the selected one, and derive payload_bytes from the MACed byte_length field, which Line 1531 already proves equal to the payload length for the entries that are read.
Also applies to: 1509-1513
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/context-guard-receipt/python/context_guard_receipt/store.py` around
lines 1429 - 1444, Update _scan so payload.bin is opened, read, and
digest-verified only for the entry matching return_payload_for; continue
validating metadata and MACs for all entries. For non-selected entries, derive
payload_bytes from the MAC-validated byte_length field, while preserving the
existing selected payload and length consistency checks.
| def run_mcp(*arguments: str, cwd: Path = PACKAGE_ROOT) -> subprocess.CompletedProcess[bytes]: | ||
| return subprocess.run( | ||
| [ | ||
| str(Path(sys.executable).resolve()), | ||
| "-I", | ||
| "-S", | ||
| "-B", | ||
| str(BOOTSTRAP), | ||
| "mcp", | ||
| *arguments, | ||
| ], | ||
| cwd=cwd, | ||
| env={"LANG": "C", "PATH": os.defpath, "PYTHONDONTWRITEBYTECODE": "1"}, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| check=False, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Redirect stdin and bound run_mcp, or the suite can hang.
run_mcp starts the stdio MCP server without stdin= and without timeout=. The child therefore inherits the test process stdin. serve_stdio reads sys.stdin.buffer until EOF, so if the inherited descriptor stays open the call at line 338 blocks forever and no timeout interrupts it. run_cli and run_node_cli always pass input=, which closes the child stdin; run_mcp does not.
🐛 Proposed fix
def run_mcp(*arguments: str, cwd: Path = PACKAGE_ROOT) -> subprocess.CompletedProcess[bytes]:
return subprocess.run(
[
str(Path(sys.executable).resolve()),
"-I",
"-S",
"-B",
str(BOOTSTRAP),
"mcp",
*arguments,
],
cwd=cwd,
env={"LANG": "C", "PATH": os.defpath, "PYTHONDONTWRITEBYTECODE": "1"},
+ stdin=subprocess.DEVNULL,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
+ timeout=30,
check=False,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def run_mcp(*arguments: str, cwd: Path = PACKAGE_ROOT) -> subprocess.CompletedProcess[bytes]: | |
| return subprocess.run( | |
| [ | |
| str(Path(sys.executable).resolve()), | |
| "-I", | |
| "-S", | |
| "-B", | |
| str(BOOTSTRAP), | |
| "mcp", | |
| *arguments, | |
| ], | |
| cwd=cwd, | |
| env={"LANG": "C", "PATH": os.defpath, "PYTHONDONTWRITEBYTECODE": "1"}, | |
| stdout=subprocess.PIPE, | |
| stderr=subprocess.PIPE, | |
| check=False, | |
| ) | |
| def run_mcp(*arguments: str, cwd: Path = PACKAGE_ROOT) -> subprocess.CompletedProcess[bytes]: | |
| return subprocess.run( | |
| [ | |
| str(Path(sys.executable).resolve()), | |
| "-I", | |
| "-S", | |
| "-B", | |
| str(BOOTSTRAP), | |
| "mcp", | |
| *arguments, | |
| ], | |
| cwd=cwd, | |
| env={"LANG": "C", "PATH": os.defpath, "PYTHONDONTWRITEBYTECODE": "1"}, | |
| stdin=subprocess.DEVNULL, | |
| stdout=subprocess.PIPE, | |
| stderr=subprocess.PIPE, | |
| timeout=30, | |
| check=False, | |
| ) |
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 148-163: Command coming from incoming request
Context: subprocess.run(
[
str(Path(sys.executable).resolve()),
"-I",
"-S",
"-B",
str(BOOTSTRAP),
"mcp",
*arguments,
],
cwd=cwd,
env={"LANG": "C", "PATH": os.defpath, "PYTHONDONTWRITEBYTECODE": "1"},
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.16.1)
[error] 149-149: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/context-guard-receipt/tests/contract/test_g010_cli.py` around lines
148 - 164, Update run_mcp to provide closed stdin input and a finite timeout
when invoking subprocess.run, matching the safeguards used by run_cli and
run_node_cli. Preserve its existing command, environment, output capture, and
return behavior while ensuring serve_stdio cannot block indefinitely on
inherited stdin.
| def run_command( | ||
| command: list[str], *, cwd: Path, environment: dict[str, str] | None = None | ||
| ) -> subprocess.CompletedProcess[str]: | ||
| return subprocess.run( | ||
| command, | ||
| cwd=cwd, | ||
| env=environment, | ||
| text=True, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| check=False, | ||
| ) | ||
|
|
||
|
|
||
| def run_binary_command( | ||
| command: list[str], *, cwd: Path, environment: dict[str, str], input_bytes: bytes = b"" | ||
| ) -> subprocess.CompletedProcess[bytes]: | ||
| return subprocess.run( | ||
| command, | ||
| cwd=cwd, | ||
| env=environment, | ||
| input=input_bytes, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| check=False, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unbounded child waits in both packaged-acceptance subprocess helpers. Both files wrap subprocess.run without a timeout, and both drive an MCP stdio child that must exit at EOF. If a child does not exit, the run blocks indefinitely with no diagnostic instead of failing.
packages/context-guard-receipt/tests/e2e/test_g001_offline_distribution.py#L34-L59: add a boundedtimeoutparameter torun_commandandrun_binary_command, and pass it tosubprocess.run.packages/context-guard-receipt/dev/packaged_acceptance.py#L40-L55: add the same boundedtimeouttorunandrun_binary.
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 36-44: Use of unsanitized data to create processes
Context: subprocess.run(
command,
cwd=cwd,
env=environment,
text=True,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
[error] 50-58: Use of unsanitized data to create processes
Context: subprocess.run(
command,
cwd=cwd,
env=environment,
input=input_bytes,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
[error] 36-44: Command coming from incoming request
Context: subprocess.run(
command,
cwd=cwd,
env=environment,
text=True,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[error] 50-58: Command coming from incoming request
Context: subprocess.run(
command,
cwd=cwd,
env=environment,
input=input_bytes,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.16.1)
[error] 37-37: subprocess call: check for execution of untrusted input
(S603)
[error] 51-51: subprocess call: check for execution of untrusted input
(S603)
📍 Affects 2 files
packages/context-guard-receipt/tests/e2e/test_g001_offline_distribution.py#L34-L59(this comment)packages/context-guard-receipt/dev/packaged_acceptance.py#L40-L55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/context-guard-receipt/tests/e2e/test_g001_offline_distribution.py`
around lines 34 - 59, The subprocess helpers currently allow child processes to
block indefinitely. In
packages/context-guard-receipt/tests/e2e/test_g001_offline_distribution.py,
update run_command and run_binary_command to accept the same bounded timeout and
pass it to subprocess.run; apply the equivalent change to run and run_binary in
packages/context-guard-receipt/dev/packaged_acceptance.py, preserving their
existing behavior otherwise.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py`:
- Around line 723-759: Update resolvePython() so explicit runtime paths undergo
the same ownership and group/other write-bit validation as automatically
discovered runtimes, rejecting writable explicit selections before probing or
spawning. Update the explicit writable-runtime contract assertion to expect
explicit: false, and add a positive test covering a foreign-owned 0o100755
runtime.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2cda8771-7b12-4bd9-91ab-8219f5d28b5c
📒 Files selected for processing (5)
packages/context-guard-receipt/README.mdpackages/context-guard-receipt/bin/launcher.cjspackages/context-guard-receipt/package-files.jsonpackages/context-guard-receipt/tests/contract/test_g001_distribution_contract.pytests/test_contextguard_stage2_feasibility.py
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/context-guard-receipt/package-files.json
- packages/context-guard-receipt/README.md
- packages/context-guard-receipt/bin/launcher.cjs
- tests/test_contextguard_stage2_feasibility.py
| writable_native = temporary_root / "caller-selected-writable-native" | ||
| shutil.copyfile("/usr/bin/true", writable_native) | ||
| writable_native.chmod(0o777) | ||
| explicit_writable = run_node( | ||
| "bin/context-guard-receipt.cjs", | ||
| "inspect", | ||
| "boundary", | ||
| environment=launcher_environment(**{PYTHON_ENV: str(writable_native)}), | ||
| ) | ||
| assert_json_error( | ||
| self, | ||
| explicit_writable, | ||
| code=78, | ||
| operation="launcher", | ||
| status="error", | ||
| reason="protocol_incompatible", | ||
| ) | ||
|
|
||
| writable_bin = temporary_root / "writable-bin" | ||
| writable_bin.mkdir() | ||
| (writable_bin / "python3").symlink_to(writable_native) | ||
| writable_path_environment = launcher_environment(PATH=str(writable_bin)) | ||
| writable_path_environment.pop(PYTHON_ENV) | ||
| automatic_writable = run_node( | ||
| "bin/context-guard-receipt.cjs", | ||
| "inspect", | ||
| "boundary", | ||
| environment=writable_path_environment, | ||
| ) | ||
| assert_json_error( | ||
| self, | ||
| automatic_writable, | ||
| code=69, | ||
| operation="launcher", | ||
| status="error", | ||
| reason="runtime_unavailable", | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline packages/context-guard-receipt/bin/launcher.cjs \
--items all --type function --match nativeExecutableRegularFile
rg -n -C 12 \
'nativeExecutableRegularFile|spawn|spawnSync|execFile|CGR_PYTHON|PYTHON_ENV' \
packages/context-guard-receipt/bin/launcher.cjs \
packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.pyRepository: ictechgy/context-guard
Length of output: 41930
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
launcher = Path("packages/context-guard-receipt/bin/launcher.cjs").read_text()
tests = Path("packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py").read_text()
start = launcher.index("function nativeExecutableRegularFile(")
end = launcher.index("\n}\n\nasync function waitForShutdown", start) + 2
predicate = launcher[start:end]
checks = {
"explicit_bypasses_write_bits": "allowCallerSelectedMetadata &&" not in predicate
and "(!allowCallerSelectedMetadata && (metadata.mode & 0o022) !== 0)" in predicate,
"explicit_bypasses_owner_check": "(!allowCallerSelectedMetadata" in predicate
and "metadata.uid !== process.getuid()" in predicate,
"explicit_resolves_then_probes": "resolveExecutable(explicit, true)" in launcher
and "compatibleProbe(python, bootstrap)" in launcher,
"test_currently_accepts_explicit_0777": "explicit: predicate('/managed/runtime', true)" in tests,
"test_currently_rejects_automatic_0777": "automatic: predicate('/managed/runtime', false)" in tests,
}
for name, value in checks.items():
print(f"{name}={value}")
print("predicate_lines:")
print(predicate)
PYRepository: ictechgy/context-guard
Length of output: 1482
Security Misconfiguration (CWE-276): Incorrect Default Permissions
Reachability: External · Exploitability: Moderate
Reject group- and world-writable explicit runtimes.
resolvePython() bypasses write-bit and ownership checks for explicit runtimes, then probes and spawns the selected path. A local principal who can replace a 0o100777 runtime can execute code as the launcher user. Apply the no-group-write and no-other-write checks to explicit selection, update the test to expect explicit: false, and add a positive foreign-owned 0o100755 case.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 724-724: chmod sets a world-writable permission bit; restrict the mode so other users cannot modify the file (e.g. 0o644 / 0o600).
Context: writable_native.chmod(0o777)
Note: [CWE-276] Incorrect Default Permissions.
(world-writable-chmod-python)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py`
around lines 723 - 759, Update resolvePython() so explicit runtime paths undergo
the same ownership and group/other write-bit validation as automatically
discovered runtimes, rejecting writable explicit selections before probing or
spawning. Update the explicit writable-runtime contract assertion to expect
explicit: false, and add a positive test covering a foreign-owned 0o100755
runtime.
|
Thanks for flagging this. I independently revalidated the trust boundary and am keeping the current behavior intentionally.
Automatic For those reasons this recommendation is not actionable for |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py (1)
1008-1035: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard the safe case against a foreign-owned temporary-directory ancestry.
trustedRuntimeAncestrywalks every ancestor of the resolved runtime, including the temporary root andTMPDIR. The safe case expectsprotocol_incompatible, which requires the whole ancestry to pass. IfTMPDIRresolves under a foreign-owned, non-sticky, group-writable directory, the safe case returnsruntime_unavailableand fails for an environment reason rather than the behavior under test.The sibling test at Line 1063 already skips on such layouts. Apply the same precondition check here, or assert the ancestry precondition before running the safe case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py` around lines 1008 - 1035, The safe-case setup in the test covering the `protocol_incompatible` assertion must verify that the temporary-directory ancestry satisfies the trusted-runtime precondition before invoking `run_node`. Reuse the sibling test’s existing foreign-owned, non-sticky, group-writable ancestry check (or equivalent assertion) so unsafe layouts skip or fail as an environmental precondition, while preserving the safe case’s expected `protocol_incompatible` result.packages/context-guard-receipt/tests/contract/test_g003_identity.py (1)
1144-1162: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBoth fake-git fixtures select the subcommand by argument position. Each fixture uses
case "$5"andcase "$6", which hard-codes the number of global options thatidentity.pypasses before the subcommand. A benign change to those global options sends every branch to the*)fallback and produces reason mismatches unrelated to the behavior under test. Replace the positional dispatch with an argument scan in both fixtures.
packages/context-guard-receipt/tests/contract/test_g003_identity.py#L1144-L1162: intest_filter_discovery_malformed_overflow_or_error_fails_closed, locate the subcommand by scanning"$@"for a known subcommand name, then select therev-parseoperand relative to that position instead of reading"$6".packages/context-guard-receipt/tests/contract/test_g003_identity.py#L1178-L1211: intest_filter_overrides_are_normalized_deduplicated_and_child_inherited, apply the same argument scan so theconfig,rev-parse,symbolic-ref, andstatusbranches no longer depend on"$5"and"$6".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g003_identity.py` around lines 1144 - 1162, Both fake-git fixtures hard-code global-option positions when dispatching subcommands, making tests sensitive to unrelated argument changes. In packages/context-guard-receipt/tests/contract/test_g003_identity.py lines 1144-1162 within test_filter_discovery_malformed_overflow_or_error_fails_closed, scan "$@" for a known subcommand and resolve the rev-parse operand relative to it; apply the same argument-scan dispatch in lines 1178-1211 within test_filter_overrides_are_normalized_deduplicated_and_child_inherited for config, rev-parse, symbolic-ref, and status, removing dependence on "$5" and "$6".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py`:
- Around line 961-998: Update
test_wrong_execute_class_is_runtime_unavailable_before_probe to skip when the
effective UID is 0, before creating the temporary runtime. Use the test
framework’s existing skip mechanism and retain the 0o401 permissions so the test
continues verifying that the owner execute class is unavailable for non-root
execution.
- Around line 1291-1338: Widen the timing margin in the boundary-timeout test by
increasing the native fixture’s nanosleep duration and updating the elapsed-time
assertion in the same test. Keep the launcher subprocess timeout and
protocol-incompatibility assertions unchanged, preserving the requirement that
the launcher aborts before the longer fixture completes.
---
Nitpick comments:
In
`@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py`:
- Around line 1008-1035: The safe-case setup in the test covering the
`protocol_incompatible` assertion must verify that the temporary-directory
ancestry satisfies the trusted-runtime precondition before invoking `run_node`.
Reuse the sibling test’s existing foreign-owned, non-sticky, group-writable
ancestry check (or equivalent assertion) so unsafe layouts skip or fail as an
environmental precondition, while preserving the safe case’s expected
`protocol_incompatible` result.
In `@packages/context-guard-receipt/tests/contract/test_g003_identity.py`:
- Around line 1144-1162: Both fake-git fixtures hard-code global-option
positions when dispatching subcommands, making tests sensitive to unrelated
argument changes. In
packages/context-guard-receipt/tests/contract/test_g003_identity.py lines
1144-1162 within test_filter_discovery_malformed_overflow_or_error_fails_closed,
scan "$@" for a known subcommand and resolve the rev-parse operand relative to
it; apply the same argument-scan dispatch in lines 1178-1211 within
test_filter_overrides_are_normalized_deduplicated_and_child_inherited for
config, rev-parse, symbolic-ref, and status, removing dependence on "$5" and
"$6".
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fcff4364-99a1-4180-b97b-42cbe2ad2994
📒 Files selected for processing (8)
packages/context-guard-receipt/README.mdpackages/context-guard-receipt/bin/launcher.cjspackages/context-guard-receipt/package-files.jsonpackages/context-guard-receipt/python/context_guard_receipt/identity.pypackages/context-guard-receipt/tests/contract/test_g001_distribution_contract.pypackages/context-guard-receipt/tests/contract/test_g003_identity.pypackages/context-guard-receipt/tests/contract/test_g008_runner.pytests/test_contextguard_stage2_feasibility.py
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/context-guard-receipt/README.md
- packages/context-guard-receipt/package-files.json
- tests/test_contextguard_stage2_feasibility.py
- packages/context-guard-receipt/python/context_guard_receipt/identity.py
- packages/context-guard-receipt/tests/contract/test_g008_runner.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py (1)
600-607: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to this
pscall for consistency.
process_existsat Line 253 andwait_for_process_stateat Line 291 both passtimeout=1.0to the same/bin/psinvocation. This call omits it. A hungpsblocks the test with no bound.♻️ Proposed change
observed = subprocess.run( ["/bin/ps", "-o", "stat=", "-p", str(process.pid)], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, text=True, check=False, + timeout=1.0, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py` around lines 600 - 607, Update the /bin/ps subprocess.run call inside the deadline loop to pass timeout=1.0, matching process_exists and wait_for_process_state. Preserve the existing stdout, stderr, text, and check behavior while ensuring each process-state probe is bounded.packages/context-guard-receipt/tests/contract/test_g003_identity.py (1)
1132-1137: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse a distinct error for filter-driver count overflow.
_MAX_GIT_FILTER_DRIVERSlimits discovery to 64 drivers, but the 65th driver raisesgit_output_limit, which also represents byte and NUL-field limits. Use a distinct reason so callers can separate filter-count overflow from output-size overflow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g003_identity.py` around lines 1132 - 1137, Update the overflow case in test_g003_identity.py and the corresponding discovery logic to emit a distinct error reason when the number of Git filter drivers exceeds _MAX_GIT_FILTER_DRIVERS. Preserve git_output_limit for byte and NUL-field output limits, and ensure callers can distinguish filter-count overflow from output-size overflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py`:
- Around line 278-302: Update wait_for_process_state and its three
_assert_launcher_reports_unconfirmed_cleanup callers to skip when /bin/ps is
unavailable, using the existing Path("/bin/ps").is_file() pattern and
unittest.skipUnless convention. Prefer raising unittest.SkipTest from
wait_for_process_state so all callers avoid timing out and report an
environmental skip rather than an assertion failure.
---
Nitpick comments:
In
`@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py`:
- Around line 600-607: Update the /bin/ps subprocess.run call inside the
deadline loop to pass timeout=1.0, matching process_exists and
wait_for_process_state. Preserve the existing stdout, stderr, text, and check
behavior while ensuring each process-state probe is bounded.
In `@packages/context-guard-receipt/tests/contract/test_g003_identity.py`:
- Around line 1132-1137: Update the overflow case in test_g003_identity.py and
the corresponding discovery logic to emit a distinct error reason when the
number of Git filter drivers exceeds _MAX_GIT_FILTER_DRIVERS. Preserve
git_output_limit for byte and NUL-field output limits, and ensure callers can
distinguish filter-count overflow from output-size overflow.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d0bd1f4-a963-4f30-853a-253394e165e1
📒 Files selected for processing (7)
packages/context-guard-receipt/README.mdpackages/context-guard-receipt/bin/launcher.cjspackages/context-guard-receipt/package-files.jsonpackages/context-guard-receipt/python/context_guard_receipt/bootstrap.pypackages/context-guard-receipt/tests/contract/test_g001_distribution_contract.pypackages/context-guard-receipt/tests/contract/test_g003_identity.pytests/test_contextguard_stage2_feasibility.py
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/context-guard-receipt/package-files.json
- packages/context-guard-receipt/README.md
- tests/test_contextguard_stage2_feasibility.py
- packages/context-guard-receipt/python/context_guard_receipt/bootstrap.py
- packages/context-guard-receipt/bin/launcher.cjs
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py (2)
2489-2495: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a compile timeout to match the shared probe helper.
compile_signal_probe_runtimebounds itsccinvocation withtimeout=15.0at Line 409. This call has no timeout. Ifcchangs, the test blocks with no bound, and the outertimeout=25.0at Line 2514 does not apply because it guards a later call.🔧 Proposed timeout
compilation = subprocess.run( [str(shutil.which("cc")), str(source), "-o", str(runtime)], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, check=False, + timeout=15.0, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py` around lines 2489 - 2495, Update the cc subprocess invocation in the surrounding distribution contract test to include the same 15.0-second timeout used by compile_signal_probe_runtime, ensuring a hung compilation is bounded while preserving the existing subprocess options.
956-968: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that each launcher-source rewrite applied.
String.prototype.replacewith a string pattern returns the source unchanged when the pattern is absent. If any of the three anchors inbin/launcher.cjsis renamed or reformatted, the rewrite silently no-ops. The test then runs with the real 5000 ms probe timeout and fails through the outertimeout=2.0bound, or with an undefinedcompatibleProbeexport. Both failures point away from the actual cause.Add an explicit check so drift produces an exact message.
♻️ Proposed rewrite guard
-const source = fs.readFileSync(launcherPath, 'utf8') - .replace( - 'const PROBE_TIMEOUT_MILLISECONDS = 5000;', - 'const PROBE_TIMEOUT_MILLISECONDS = 10;', - ) - .replace( - 'const INTERRUPT_KILL_WAIT_MILLISECONDS = 750;', - 'const INTERRUPT_KILL_WAIT_MILLISECONDS = 25;', - ) - .replace( - 'module.exports = { launch };', - 'module.exports = { compatibleProbe };', - ); +const rewrite = (text, from, to) => { + if (!text.includes(from)) { + throw new Error(`launcher anchor missing: ${from}`); + } + return text.replace(from, to); +}; +let source = fs.readFileSync(launcherPath, 'utf8'); +source = rewrite( + source, + 'const PROBE_TIMEOUT_MILLISECONDS = 5000;', + 'const PROBE_TIMEOUT_MILLISECONDS = 10;', +); +source = rewrite( + source, + 'const INTERRUPT_KILL_WAIT_MILLISECONDS = 750;', + 'const INTERRUPT_KILL_WAIT_MILLISECONDS = 25;', +); +source = rewrite( + source, + 'module.exports = { launch };', + 'module.exports = { compatibleProbe };', +);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py` around lines 956 - 968, Update the launcher-source rewrite setup in the test to verify that each of the three expected anchor strings is present before or during replacement, and fail immediately with an exact, actionable message identifying the missing anchor instead of silently continuing with unchanged source. Keep the existing timeout and export rewrites unchanged when their anchors match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py`:
- Around line 2489-2495: Update the cc subprocess invocation in the surrounding
distribution contract test to include the same 15.0-second timeout used by
compile_signal_probe_runtime, ensuring a hung compilation is bounded while
preserving the existing subprocess options.
- Around line 956-968: Update the launcher-source rewrite setup in the test to
verify that each of the three expected anchor strings is present before or
during replacement, and fail immediately with an exact, actionable message
identifying the missing anchor instead of silently continuing with unchanged
source. Keep the existing timeout and export rewrites unchanged when their
anchors match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aee8718a-07b0-4dab-b76e-d4d1a4f39c9e
📒 Files selected for processing (4)
packages/context-guard-receipt/bin/launcher.cjspackages/context-guard-receipt/package-files.jsonpackages/context-guard-receipt/tests/contract/test_g001_distribution_contract.pytests/test_contextguard_stage2_feasibility.py
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/context-guard-receipt/package-files.json
- packages/context-guard-receipt/bin/launcher.cjs
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/context-guard-receipt/tests/contract/test_g001_distribution_contract.py`:
- Around line 1034-1064: Increase the in-script watchdog delay in the
`setTimeout` around `compatibleProbe` and raise the enclosing `subprocess.run`
timeout together, preserving the existing timeout-reporting behavior and error
assertions while giving VM setup and `late-close` cleanup sufficient time to
complete.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 17df28d4-5231-4ee8-9c5f-7b993cd4c38e
📒 Files selected for processing (4)
packages/context-guard-receipt/bin/launcher.cjspackages/context-guard-receipt/package-files.jsonpackages/context-guard-receipt/tests/contract/test_g001_distribution_contract.pytests/test_contextguard_stage2_feasibility.py
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/context-guard-receipt/bin/launcher.cjs
- packages/context-guard-receipt/package-files.json
- tests/test_contextguard_stage2_feasibility.py
…-receipt-sanitized # Conflicts: # tests/test_contextguard_stage2_feasibility.py
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/context-guard-receipt/tests/contract/test_g003_identity.py`:
- Around line 229-247: Add the same explicit git-availability skip used by
test_snapshot_root_fd_remains_authoritative_after_ancestor_retarget to
test_non_git_snapshot_traverses_search_only_ancestor, before snapshot_repository
runs. When find_git() returns None, skip the test so its non_git_directory and
non_git assertions only run when git is available.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 046e998b-8977-424d-99d3-463e17f92daa
📒 Files selected for processing (10)
packages/context-guard-receipt/bin/launcher.cjspackages/context-guard-receipt/package-files.jsonpackages/context-guard-receipt/python/context_guard_receipt/identity.pypackages/context-guard-receipt/python/context_guard_receipt/runner.pypackages/context-guard-receipt/schemas/diagnostic-ledger-inspection.schema.jsonpackages/context-guard-receipt/tests/contract/test_g001_distribution_contract.pypackages/context-guard-receipt/tests/contract/test_g003_identity.pypackages/context-guard-receipt/tests/contract/test_g008_runner.pypackages/context-guard-receipt/tests/contract/test_g009_ledger.pytests/test_contextguard_stage2_feasibility.py
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/context-guard-receipt/schemas/diagnostic-ledger-inspection.schema.json
- packages/context-guard-receipt/package-files.json
- packages/context-guard-receipt/python/context_guard_receipt/runner.py
- packages/context-guard-receipt/tests/contract/test_g009_ledger.py
- tests/test_contextguard_stage2_feasibility.py
- packages/context-guard-receipt/tests/contract/test_g008_runner.py
- packages/context-guard-receipt/bin/launcher.cjs
| def test_non_git_snapshot_traverses_search_only_ancestor(self) -> None: | ||
| """Break caught: descriptor ancestry starts requiring directory reads.""" | ||
|
|
||
| identity = identity_module() | ||
| with tempfile.TemporaryDirectory() as directory: | ||
| ancestor = Path(directory) / "search-only" | ||
| root = ancestor / "root" | ||
| root.mkdir(parents=True) | ||
| ancestor.chmod(0o111) | ||
| try: | ||
| snapshot = identity.snapshot_repository( | ||
| root, git_executable=find_git() | ||
| ) | ||
| finally: | ||
| ancestor.chmod(0o700) | ||
|
|
||
| self.assertEqual(snapshot["disposition"], "pass_through") | ||
| self.assertEqual(snapshot["reason"], "non_git_directory") | ||
| self.assertEqual(snapshot["logical_state"]["kind"], "non_git") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the git-availability skip used by the sibling test.
This test asserts reason == "non_git_directory" and kind == "non_git". If find_git() returns None, _resolve_git_executable returns None and _snapshot_once produces an unresolved snapshot with reason git_unavailable. Both assertions then fail for an environment reason, not a contract break. test_snapshot_root_fd_remains_authoritative_after_ancestor_retarget at Line 734 already guards this case with an explicit skip. Apply the same guard here.
💚 Proposed fix to guard on git availability
identity = identity_module()
+ git = find_git()
+ if git is None:
+ self.skipTest("git is unavailable")
with tempfile.TemporaryDirectory() as directory:
ancestor = Path(directory) / "search-only"
root = ancestor / "root"
root.mkdir(parents=True)
ancestor.chmod(0o111)
try:
snapshot = identity.snapshot_repository(
- root, git_executable=find_git()
+ root, git_executable=git
)
finally:
ancestor.chmod(0o700)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_non_git_snapshot_traverses_search_only_ancestor(self) -> None: | |
| """Break caught: descriptor ancestry starts requiring directory reads.""" | |
| identity = identity_module() | |
| with tempfile.TemporaryDirectory() as directory: | |
| ancestor = Path(directory) / "search-only" | |
| root = ancestor / "root" | |
| root.mkdir(parents=True) | |
| ancestor.chmod(0o111) | |
| try: | |
| snapshot = identity.snapshot_repository( | |
| root, git_executable=find_git() | |
| ) | |
| finally: | |
| ancestor.chmod(0o700) | |
| self.assertEqual(snapshot["disposition"], "pass_through") | |
| self.assertEqual(snapshot["reason"], "non_git_directory") | |
| self.assertEqual(snapshot["logical_state"]["kind"], "non_git") | |
| def test_non_git_snapshot_traverses_search_only_ancestor(self) -> None: | |
| """Break caught: descriptor ancestry starts requiring directory reads.""" | |
| identity = identity_module() | |
| git = find_git() | |
| if git is None: | |
| self.skipTest("git is unavailable") | |
| with tempfile.TemporaryDirectory() as directory: | |
| ancestor = Path(directory) / "search-only" | |
| root = ancestor / "root" | |
| root.mkdir(parents=True) | |
| ancestor.chmod(0o111) | |
| try: | |
| snapshot = identity.snapshot_repository( | |
| root, git_executable=git | |
| ) | |
| finally: | |
| ancestor.chmod(0o700) | |
| self.assertEqual(snapshot["disposition"], "pass_through") | |
| self.assertEqual(snapshot["reason"], "non_git_directory") | |
| self.assertEqual(snapshot["logical_state"]["kind"], "non_git") |
🧰 Tools
🪛 GitHub Actions: CI / 0_test-and-prepublish (macos-latest, 3.12).txt
[error] 246-246: Contract test failed: test_non_git_snapshot_traverses_search_only_ancestor expected snapshot reason 'non_git_directory', but received 'git_command_failed'.
🪛 GitHub Actions: CI / test-and-prepublish (macos-latest, 3.12)
[error] 246-246: Contract test test_non_git_snapshot_traverses_search_only_ancestor failed: expected snapshot reason 'non_git_directory' but received 'git_command_failed'. The test suite failed with exit code 1.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/context-guard-receipt/tests/contract/test_g003_identity.py` around
lines 229 - 247, Add the same explicit git-availability skip used by
test_snapshot_root_fd_remains_authoritative_after_ancestor_retarget to
test_non_git_snapshot_traverses_search_only_ancestor, before snapshot_repository
runs. When find_git() returns None, skip the test so its non_git_directory and
non_git assertions only run when git is available.
|
@coderabbitai review |
✅ Action performedReview finished.
|
…-receipt-sanitized # Conflicts: # tests/test_contextguard_stage2_feasibility.py
1 participant
Summary
Evidence boundary
This PR remains companion-local only: selected branch S2-UNSUPPORTED, selected transport NONE, no runtime observer, no provider join, no provider claim authority, and no Stage 1 or Stage 2 evidence. It does not automatically modify host settings or claim measured provider token savings.
Final verification
Exact head:
b15fc1332f1673d67a43aff44d016f52f01e22b4Exact base:
d7d53b9a63b367fd6a868e3ed018bb8bc1b79e67Prerequisite CI budget PR #285 was separately reviewed, verified, and merged before the final base integration.