feat(applications): add read-only maintainer context CLI - #229
Conversation
📝 WalkthroughWalkthroughThis PR adds a read-only maintainer Context CLI, a shared loopback dogfood HTTP client, centralized public package validation and digest logic, installation wiring, comprehensive tests, and updated capability and operational documentation. ChangesMaintainer Context CLI
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12b84b7a37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if cast(dict[str, object], grant).get("value") != REDACTED_EGRESS_GRANT: | ||
| return raw | ||
| envelope = dict(raw) | ||
| envelope["egressGrant"] = None |
There was a problem hiding this comment.
stometa, when an untrusted capture contains egressGrant.value == "REDACTED-EGRESS-GRANT", this replaces the entire grant with None before Pydantic validation, so an object with an invalid kind or forbidden extra fields is accepted and its package is rendered. This violates the documented closed-envelope inspection contract; substitute a pattern-valid placeholder for only the value while preserving the original grant structure for validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72015af7d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "", | ||
| f"block {ordinal}:", | ||
| f" blockId: {block.blockId}", | ||
| f" text: {block.text}", |
There was a problem hiding this comment.
stometa, when a queried document or untrusted capture contains ANSI/OSC control characters in an otherwise valid Block, the human renderer writes block.text directly to the maintainer's terminal. A recomputed package digest does not prevent this, so authorized or captured content can spoof CLI fields, alter terminal state, or attempt clipboard manipulation; escape or reject terminal control characters while preserving the original bytes in JSON output.
Useful? React with 👍 / 👎.
| return _PackageCapture( | ||
| package=outcome.package, | ||
| document=package_document, | ||
| expires_at=_validated_expiry(outcome.package), |
There was a problem hiding this comment.
stometa, when an untrusted capture has a correctly recomputed digest but uses a naive sourceAclEvidence.aclAsOf—or the corresponding live/weak checkedAt, snapshotAsOf, or expiresAt—Pydantic accepts the datetime and this validation checks only the Package's outer asOf/expiresAt. The capture is therefore reported as valid and malformed ACL timing is rendered as citation lineage; validate every Evidence and SourceAclEvidence instant as timezone-aware and UTC-normalizable before accepting the capture.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
engine/public_contract.py (2)
72-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
Noneto the end of both unions to clear RUF036.Ruff reports RUF036 at lines 73-79 and 85. Reordering the union members changes no behavior.
♻️ Proposed reordering
type CanonicalJsonValue = ( - None - | bool + bool | int | float | str | list["CanonicalJsonValue"] | dict[str, "CanonicalJsonValue"] + | None ) def _json_value(value: object, ancestors: set[int]) -> CanonicalJsonValue: if type(value) in (type(None), bool, float): - return cast(None | bool | float, value) + return cast(bool | float | None, value)🤖 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 `@engine/public_contract.py` around lines 72 - 85, Reorder the union members in CanonicalJsonValue and the return annotation of _json_value so None appears last in each union, preserving all existing members and behavior.Source: Linters/SAST tools
50-63: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueDocument that
continuationis omitted frompackageDigest.
ContextPackageWire.continuationaccepts aContinuationOfferWire, but the public digest document always forcescontinuation: Nonebefore hashing. The currentcontext-package-canonical-json-v3profile is consistent across Runtime and HTTP, but it needs an explicit note that continuation offers are transmitted but not covered bypackageDigest.🤖 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 `@engine/public_contract.py` around lines 50 - 63, Update the documentation for the context-package-canonical-json-v3 profile near complete_context_package_nullable_fields to explicitly state that ContextPackageWire.continuation may be transmitted but is always normalized to null and therefore omitted from packageDigest coverage; keep the existing canonicalization behavior unchanged.applications/maintainer_context.py (1)
379-380: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign
ensure_asciiwith the other JSON renderings.Line 380 omits
ensure_ascii=False. Lines 393 and 461 set it. A non-ASCII projected field name renders as an escape sequence here and as a literal elsewhere. Use the same setting for consistent human output.♻️ Proposed consistency fix
" projectedFields: " - + json.dumps(list(evidence.projectedFields), separators=(",", ":")), + + json.dumps( + list(evidence.projectedFields), + ensure_ascii=False, + separators=(",", ":"), + ),🤖 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 `@applications/maintainer_context.py` around lines 379 - 380, Update the projectedFields JSON rendering in the evidence formatting logic to pass ensure_ascii=False to json.dumps, matching the settings used by the other JSON renderings such as those near lines 393 and 461. Preserve the existing compact separators and field serialization.tests/integration/test_maintainer_context_cli.py (2)
156-168: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThis assertion cannot fail.
captured_envelopeandenvelopeboth come from parsing the samemachine.stdoutstring. Line 168 compares two decodings of one source, so it proves nothing about the file round trip. Compare the file text againstmachine.stdoutinstead, or remove the assertion.🤖 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/integration/test_maintainer_context_cli.py` around lines 156 - 168, Update the assertion in the test around captured_envelope and envelope so it validates the file round trip rather than comparing two objects parsed from the same machine.stdout source. Compare the serialized capture file content with machine.stdout, or remove the redundant assertion if that round-trip check is covered elsewhere.
60-80: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winShutdown handlers never run, so the composed engines are not disposed.
Config(..., lifespan="off")disables the ASGI lifespan protocol.create_dogfood_appregistersruntime_engine.dispose,control_engine.dispose, androots.closeasshutdownevent handlers (adapters/http/dogfood.py lines 361-366). With lifespan off, none of those handlers run, so the database engines and file roots stay open for the rest of the session.Enable the lifespan protocol, or dispose the engines explicitly in the
finallyblock.Also close the listener only after the thread stops. Line 78 closes the socket while the server thread may still own it.
♻️ Proposed cleanup ordering
finally: server.should_exit = True thread.join(timeout=5) - listener.close() if thread.is_alive(): raise RuntimeError("local CLI session server did not stop") + listener.close()🤖 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/integration/test_maintainer_context_cli.py` around lines 60 - 80, Update the local CLI session server setup around Config and the try/finally cleanup: enable ASGI lifespan so create_dogfood_app shutdown handlers dispose runtime_engine, control_engine, and roots, then signal shutdown and join the server thread before closing listener. Preserve the existing startup failure cleanup and raise if the thread remains alive.tests/unit/test_maintainer_context_cli.py (1)
522-536: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound every CLI subprocess invocation added by this PR. Both sites launch the maintainer CLI without a
timeout, while the sibling helpers_commandand_run_cliset one. A hang at either site blocks the suite with no failure signal.
tests/unit/test_maintainer_context_cli.py#L522-L536: addtimeout=30to thesubprocess.runcall that lists imported modules.tests/process/test_processes.py#L77-L83: addtimeout=30to thesubprocess.runcall that runscontext-engine-context --help.🤖 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/unit/test_maintainer_context_cli.py` around lines 522 - 536, Bound both added CLI subprocess invocations with a 30-second timeout: update the subprocess.run call in tests/unit/test_maintainer_context_cli.py at lines 522-536 and the subprocess.run call in tests/process/test_processes.py at lines 77-83. No other changes are needed.tests/process/test_processes.py (1)
77-83: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAdd a timeout to this subprocess call.
The other CLI subprocess tests in this PR bound their invocations. This call has none. If the console script fails to resolve or hangs, the test blocks with no failure signal.
♻️ Proposed bound
cwd=ROOT, check=False, capture_output=True, text=True, + timeout=30, )🤖 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/process/test_processes.py` around lines 77 - 83, Add a finite timeout to the subprocess.run invocation in the context-engine-context help test, matching the bounded CLI subprocess tests nearby. Keep the existing command and result handling unchanged while ensuring hangs or unresolved console scripts terminate with a failure.
🤖 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
`@docs/decisions/0088-bind-local-consumers-to-fresh-evidence-bearing-packages.md`:
- Around line 123-129: Correct the temporal wording in the ADR paragraph
beginning “On 2026-08-03”: use the verified date when issue `#216` actually fired,
or rewrite the statement as planned/future wording if it has not occurred yet.
Preserve the surrounding claims about applications/dogfood_client.py, the
obligations, category boundary, and NOT_ACTIVE carriers.
In `@README.md`:
- Around line 534-536: Remove the numeric test-to-engine ratio from README.md
lines 534-536 and README.zh-CN.md lines 246-247, replacing each with a
qualitative non-numeric statement while preserving the surrounding meaning.
- Line 513: Clarify the applications/ boundary in all four documented locations:
in README.md lines 513 and 529-533, state that applications may handle CLI
validation, rendering, and loopback HTTP transport while authorization,
retrieval decisions, and delivery remain in engine/; make the equivalent updates
in README.zh-CN.md lines 226 and 242-245, preserving the existing
repository-layout and architecture descriptions.
In `@tests/unit/test_maintainer_context_cli.py`:
- Around line 304-313: Update the expired-package fixture in
test_expired_package_is_content_free_and_has_stable_exit_class to use an
unambiguously historical expiresAt value far before the current date, while
keeping the related timestamps and packageDigest recalculation consistent.
Ensure the fixture deterministically remains expired regardless of the test
execution time.
---
Nitpick comments:
In `@applications/maintainer_context.py`:
- Around line 379-380: Update the projectedFields JSON rendering in the evidence
formatting logic to pass ensure_ascii=False to json.dumps, matching the settings
used by the other JSON renderings such as those near lines 393 and 461. Preserve
the existing compact separators and field serialization.
In `@engine/public_contract.py`:
- Around line 72-85: Reorder the union members in CanonicalJsonValue and the
return annotation of _json_value so None appears last in each union, preserving
all existing members and behavior.
- Around line 50-63: Update the documentation for the
context-package-canonical-json-v3 profile near
complete_context_package_nullable_fields to explicitly state that
ContextPackageWire.continuation may be transmitted but is always normalized to
null and therefore omitted from packageDigest coverage; keep the existing
canonicalization behavior unchanged.
In `@tests/integration/test_maintainer_context_cli.py`:
- Around line 156-168: Update the assertion in the test around captured_envelope
and envelope so it validates the file round trip rather than comparing two
objects parsed from the same machine.stdout source. Compare the serialized
capture file content with machine.stdout, or remove the redundant assertion if
that round-trip check is covered elsewhere.
- Around line 60-80: Update the local CLI session server setup around Config and
the try/finally cleanup: enable ASGI lifespan so create_dogfood_app shutdown
handlers dispose runtime_engine, control_engine, and roots, then signal shutdown
and join the server thread before closing listener. Preserve the existing
startup failure cleanup and raise if the thread remains alive.
In `@tests/process/test_processes.py`:
- Around line 77-83: Add a finite timeout to the subprocess.run invocation in
the context-engine-context help test, matching the bounded CLI subprocess tests
nearby. Keep the existing command and result handling unchanged while ensuring
hangs or unresolved console scripts terminate with a failure.
In `@tests/unit/test_maintainer_context_cli.py`:
- Around line 522-536: Bound both added CLI subprocess invocations with a
30-second timeout: update the subprocess.run call in
tests/unit/test_maintainer_context_cli.py at lines 522-536 and the
subprocess.run call in tests/process/test_processes.py at lines 77-83. No other
changes are needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e128167e-a214-45da-9be6-f836c2ccd3a8
📒 Files selected for processing (18)
README.mdREADME.zh-CN.mdSTATUS.mdadapters/http/contracts.pyapplications/dogfood_client.pyapplications/dogfood_evaluation.pyapplications/local_context.pyapplications/maintainer_context.pydocs/decisions/0088-bind-local-consumers-to-fresh-evidence-bearing-packages.mddocs/operations/maintainer-context-cli.mdengine/public_contract.pyengine/runtime/contracts.pyengine/runtime/evidence.pyengine/runtime/package_digest.pypyproject.tomltests/integration/test_maintainer_context_cli.pytests/process/test_processes.pytests/unit/test_maintainer_context_cli.py
|
|
||
| On 2026-08-03, issue #216 fired the first clause: the read-only maintainer | ||
| Context CLI is the second kind of local consumer, and | ||
| `applications/dogfood_client.py` is now the shared data and transport seam that | ||
| both it and the Claude Code skill call. The four obligations, the local | ||
| read-only category boundary, and every carrier this decision leaves | ||
| `NOT_ACTIVE` are unchanged, so no superseding decision is required. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the actual issue date.
The ADR says “On 2026-08-03” in past tense, but the current date is August 2, 2026. Replace it with the date when issue #216 actually fired, or use planned wording until that date occurs.
🧰 Tools
🪛 LanguageTool
[style] ~125-~125: Phrases like “kind of” can make your writing sound less confident. You could consider removing it to sound more confident.
Context: ...ly maintainer Context CLI is the second kind of local consumer, and `applications/dogfo...
(KIND_OF_SORT_OF_2)
🤖 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
`@docs/decisions/0088-bind-local-consumers-to-fresh-evidence-bearing-packages.md`
around lines 123 - 129, Correct the temporal wording in the ADR paragraph
beginning “On 2026-08-03”: use the verified date when issue `#216` actually fired,
or rewrite the statement as planned/future wording if it has not occurred yet.
Preserve the surrounding claims about applications/dogfood_client.py, the
obligations, category boundary, and NOT_ACTIVE carriers.
| - **Tests outweigh implementation ~3:1.** The `tests/` tree is several times the | ||
| size of `engine/`. For a project whose central claim is a security invariant, | ||
| the executable evidence *is* the product. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the numeric repository-size claim from both READMEs.
Both changed sections retain an approximate 3:1 test-to-engine ratio, although the PR removes volatile README counts. Use a non-numeric qualitative statement in both languages.
README.md#L534-L536: remove~3:1.README.zh-CN.md#L246-L247: remove “约为实现量的 3 倍”.
📍 Affects 2 files
README.md#L534-L536(this comment)README.zh-CN.md#L246-L247
🤖 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 `@README.md` around lines 534 - 536, Remove the numeric test-to-engine ratio
from README.md lines 534-536 and README.zh-CN.md lines 246-247, replacing each
with a qualitative non-numeric statement while preserving the surrounding
meaning.
| def test_expired_package_is_content_free_and_has_stable_exit_class() -> None: | ||
| package = _package_document() | ||
| package["asOf"] = "2026-08-02T12:00:00Z" | ||
| package["expiresAt"] = "2026-08-02T12:05:00Z" | ||
| evidence = cast(list[dict[str, object]], package["evidence"])[0] | ||
| evidence["authorizationAsOf"] = "2026-08-02T12:00:00Z" | ||
| source_acl = cast(dict[str, object], evidence["sourceAclEvidence"]) | ||
| source_acl["aclAsOf"] = "2026-08-02T12:00:00Z" | ||
| package.pop("packageDigest") | ||
| package["packageDigest"] = context_package_digest(package) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use an unambiguously historical instant for the expired-package fixture.
The fixture sets expiresAt to 2026-08-02T12:05:00Z. That instant is not clearly in the past. The CLI compares it against datetime.now(tz=UTC), so the assertion returncode == 13 depends on the wall clock at the moment the suite runs. Use a far-past instant to make the expiry deterministic, in the same way the current-package fixture uses 2099.
🐛 Proposed deterministic fixture
- package["asOf"] = "2026-08-02T12:00:00Z"
- package["expiresAt"] = "2026-08-02T12:05:00Z"
+ package["asOf"] = "2000-01-01T12:00:00Z"
+ package["expiresAt"] = "2000-01-01T12:05:00Z"
evidence = cast(list[dict[str, object]], package["evidence"])[0]
- evidence["authorizationAsOf"] = "2026-08-02T12:00:00Z"
+ evidence["authorizationAsOf"] = "2000-01-01T12:00:00Z"
source_acl = cast(dict[str, object], evidence["sourceAclEvidence"])
- source_acl["aclAsOf"] = "2026-08-02T12:00:00Z"
+ source_acl["aclAsOf"] = "2000-01-01T12:00:00Z"📝 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_expired_package_is_content_free_and_has_stable_exit_class() -> None: | |
| package = _package_document() | |
| package["asOf"] = "2026-08-02T12:00:00Z" | |
| package["expiresAt"] = "2026-08-02T12:05:00Z" | |
| evidence = cast(list[dict[str, object]], package["evidence"])[0] | |
| evidence["authorizationAsOf"] = "2026-08-02T12:00:00Z" | |
| source_acl = cast(dict[str, object], evidence["sourceAclEvidence"]) | |
| source_acl["aclAsOf"] = "2026-08-02T12:00:00Z" | |
| package.pop("packageDigest") | |
| package["packageDigest"] = context_package_digest(package) | |
| def test_expired_package_is_content_free_and_has_stable_exit_class() -> None: | |
| package = _package_document() | |
| package["asOf"] = "2000-01-01T12:00:00Z" | |
| package["expiresAt"] = "2000-01-01T12:05:00Z" | |
| evidence = cast(list[dict[str, object]], package["evidence"])[0] | |
| evidence["authorizationAsOf"] = "2000-01-01T12:00:00Z" | |
| source_acl = cast(dict[str, object], evidence["sourceAclEvidence"]) | |
| source_acl["aclAsOf"] = "2000-01-01T12:00:00Z" | |
| package.pop("packageDigest") | |
| package["packageDigest"] = context_package_digest(package) |
🤖 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/unit/test_maintainer_context_cli.py` around lines 304 - 313, Update the
expired-package fixture in
test_expired_package_is_content_free_and_has_stable_exit_class to use an
unambiguously historical expiresAt value far before the current date, while
keeping the related timestamps and packageDigest recalculation consistent.
Ensure the fixture deterministically remains expired regardless of the test
execution time.
72015af to
a0c5580
Compare
stone16
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0c55802dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| query.add_argument("--max-tokens", type=int) | ||
| query.add_argument("--max-provider-calls", type=int) | ||
| query.add_argument("--max-cost-microunits", type=int) | ||
| query.add_argument("--max-elapsed-ms", type=int) |
There was a problem hiding this comment.
stometa, when a budget value is syntactically invalid, such as --max-tokens not-an-int, argparse exits here with status 2 before _query can classify it. This breaks the CLI's documented stable exit contract, which assigns invalid budget and request inputs to exit 14, so automation cannot reliably distinguish these inputs from an undocumented failure; route parser errors through EXIT_INVALID_CONFIGURATION instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/integration/test_maintainer_context_cli.py (2)
87-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDo not raise from the
finallyblock on the teardown path.If the
withbody fails, thefinallyblock still runs. If the uvicorn thread is then still alive, line 94 raisesRuntimeError. That new exception replaces the original assertion failure, so the real cause of the test failure is lost. Report the stuck thread as a warning instead, or check liveness only when no exception is in flight.♻️ Proposed teardown that preserves the original failure
try: yield finally: server.should_exit = True thread.join(timeout=5) listener.close() - if thread.is_alive(): - raise RuntimeError("local CLI session server did not stop") + stuck = thread.is_alive() + if stuck: + raise RuntimeError("local CLI session server did not stop")🤖 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/integration/test_maintainer_context_cli.py` around lines 87 - 94, Update the teardown around the server thread in the test’s context manager so a still-alive thread is reported as a warning rather than raised from the finally block. Preserve the original exception from the with body while retaining the existing shutdown, join, and listener cleanup behavior.
51-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGive a clear failure when the console script is not installed.
_run_clistartscontext-engine-contextby bare name. The test then depends on the entry point being present onPATH. If the package is not installed in the active environment,subprocess.runraisesFileNotFoundErrorand the report shows no reason. Resolve the executable first and skip with an explicit message.♻️ Proposed guard
+import shutil + def _run_cli(*arguments: str) -> subprocess.CompletedProcess[str]: + executable = shutil.which("context-engine-context") + if executable is None: + pytest.skip("context-engine-context console script is not installed") return subprocess.run( - ["context-engine-context", *arguments], + [executable, *arguments], check=False,🤖 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/integration/test_maintainer_context_cli.py` around lines 51 - 59, Update _run_cli to resolve the context-engine-context executable before invoking subprocess.run, and catch FileNotFoundError when it is unavailable. Skip the test with an explicit message explaining that the console script is not installed, while preserving the existing subprocess arguments and timeout.applications/dogfood_evaluation.py (1)
13-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPromote the shared helpers to public names instead of importing private ones.
This module imports
_as_object,_package_from_outcome,_require_exact_text, and_require_opaque_reffromapplications.dogfood_client. The leading underscore marks them as module-private, so this import crosses a declared module boundary.dogfood_clientalready exposesvalidate_dogfood_queryandvalidate_dogfood_request_idfor two of these uses. Give the remaining helpers public names indogfood_client, then import only public symbols here.Run this script to list the call sites that must change:
#!/bin/bash rg -nP --type=py '\b(_as_object|_package_from_outcome|_require_exact_text|_require_opaque_ref)\b' -C2As per coding guidelines: "engine, adapters, bot_delivery, action_plane, and supporting modules must respect their declared process and module boundaries."
🤖 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 `@applications/dogfood_evaluation.py` around lines 13 - 26, Promote the shared helpers currently named _as_object, _package_from_outcome, _require_exact_text, and _require_opaque_ref in dogfood_client to public names, update all call sites to use those names, and change this import to include only the public symbols. Preserve the existing helper behavior and use the repository-wide call-site search to ensure no private references remain.Source: Coding guidelines
🤖 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 `@applications/dogfood_evaluation.py`:
- Around line 13-26: Promote the shared helpers currently named _as_object,
_package_from_outcome, _require_exact_text, and _require_opaque_ref in
dogfood_client to public names, update all call sites to use those names, and
change this import to include only the public symbols. Preserve the existing
helper behavior and use the repository-wide call-site search to ensure no
private references remain.
In `@tests/integration/test_maintainer_context_cli.py`:
- Around line 87-94: Update the teardown around the server thread in the test’s
context manager so a still-alive thread is reported as a warning rather than
raised from the finally block. Preserve the original exception from the with
body while retaining the existing shutdown, join, and listener cleanup behavior.
- Around line 51-59: Update _run_cli to resolve the context-engine-context
executable before invoking subprocess.run, and catch FileNotFoundError when it
is unavailable. Skip the test with an explicit message explaining that the
console script is not installed, while preserving the existing subprocess
arguments and timeout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 668d0346-9a62-4941-871f-6d9bfd842c03
📒 Files selected for processing (18)
README.mdREADME.zh-CN.mdSTATUS.mdadapters/http/contracts.pyapplications/dogfood_client.pyapplications/dogfood_evaluation.pyapplications/local_context.pyapplications/maintainer_context.pydocs/decisions/0088-bind-local-consumers-to-fresh-evidence-bearing-packages.mddocs/operations/maintainer-context-cli.mdengine/public_contract.pyengine/runtime/contracts.pyengine/runtime/evidence.pyengine/runtime/package_digest.pypyproject.tomltests/integration/test_maintainer_context_cli.pytests/process/test_processes.pytests/unit/test_maintainer_context_cli.py
🚧 Files skipped from review as they are similar to previous changes (8)
- applications/local_context.py
- pyproject.toml
- STATUS.md
- engine/runtime/evidence.py
- engine/runtime/contracts.py
- adapters/http/contracts.py
- engine/runtime/package_digest.py
- README.zh-CN.md
…apture validation
…ainer CLI test CI builds the pull request's merge commit, not this branch tip. On main, `create_dogfood_app` now constructs a real `LocalQwenEmbeddingProvider`, and `test_dogfood_runtime_activation` keeps that network-free by patching the provider in a module-local autouse fixture while its `_environment` helper still hands out the placeholder model directory. The maintainer CLI test reuses those helpers, but pytest applies an autouse fixture only to the module that defines it, so the merged composition loaded the placeholder directory for real and refused with `DogfoodConfigurationUnavailable`. Import the twin fixture beside the helpers it belongs to, which states the dependency the module actually has instead of inheriting it by file position.
… dedupe dogfood validators, tighten capture grant guard
…rs before membership tests
…lag documentation
a0c5580 to
5b511e8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b511e8b9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raw = capture.read(MAX_CAPTURE_BYTES + 1) | ||
| if len(raw) > MAX_CAPTURE_BYTES: | ||
| raise ValueError("capture is too large") | ||
| return json.loads(raw) |
There was a problem hiding this comment.
stometa, when an untrusted capture repeats a security-relevant member such as kind, package, or packageDigest, json.loads silently keeps only the last occurrence, so the CLI can report an ambiguous, non-closed document as valid even though another JSON consumer may interpret a different value. Use an object_pairs_hook (or equivalent strict decoder) that rejects duplicate object names before schema and digest validation.
Useful? React with 👍 / 👎.
| category=capture.category, | ||
| ) | ||
| return EXIT_EXPLICIT_REFUSAL | ||
| if datetime.now(tz=UTC) >= capture.expires_at: |
There was a problem hiding this comment.
stometa, when an inspected capture is only slightly short of expiresAt—especially a large capture rendered in human form—this check can pass before serialization/rendering and the Package content can then be printed after expiry. Buffer the selected output first and perform the final expiry check immediately before emitting it so near-expiry captures fail closed rather than presenting expired context.
Useful? React with 👍 / 👎.
1 participant
Closes #216.
Summary
context-engine-context query|inspectread-only maintainer surface with human and strict JSON output.NOT_ACTIVE.Maintainer decisions: v1 remains
context-engine-context; inspection accepts explicitly untrusted file/stdin captures; no citation-open, evaluation, Control, ActionPlane, model, effect, or promotion commands are exposed; a valid saved unavailable-request capture returns exit 10; served content-free status classes map narrowly to stable exits, including authentication failure to invalid configuration while transport/5xx stays service unavailable.Verification
make lint: passed.make typecheck: passed on 511 Python sources and all TypeScript surfaces.make test: 2,612 passed.checkspassed, including catalog, process, real PostgreSQL integration/security, M0 security gate, and shipped-artifact governance; CodeRabbit passed.NOT_ACTIVE, and content-free tampered-capture refusal. The sanitized local transcript contains no bearer, redeemable egress grant, package identifiers, digests, or opaque lineage references.The README edits are coordinator-required drive-by corrections removing known-false volatile line/ADR counts while retaining their semantic architecture claims.
No merge is requested by this automation; the PR remains for maintainer review.