feat(cloud): ingest typed routine outcomes idempotently - #2125
Conversation
|
Warning Review limit reached
Next review available in: 37 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis change adds a versioned cloud-routine receipt contract, validates executable predicates, plans idempotent task upserts, provides guarded ingestion and verification CLIs, and adds receipt, lineage, IRF, lever, and governance artifacts. ChangesCloud-routine receipt ingestion
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
Multi-agent review roll call (CodeRabbit and Claude review automatically. Reviewers: post substantive findings only. Authors/agents: address every thread, push fixes to this branch, reply and resolve, then re-request review.) |
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a typed contract (CloudRoutineReceiptV1) for recurring cloud-routine observations and an ingest consumer that classifies receipts, deduplicates against existing/pending TABVLARIVS work, and (when armed) emits idempotent upsert tickets for only new_work dispositions.
Changes:
- Added
CloudRoutineReceiptV1JSON Schema and a matching Pydantic model + task-planning utilities. - Added
scripts/cloud-routine-ingest.pyto validate receipt deliveries, dedupe against board + pending inbox tickets, and optionally submit upsert tickets. - Published the receipt/consumer contract in
cloud-routines.jsonand added focused unit tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
spec/contracts/cloud-routine-receipt-v1.schema.json |
Defines the versioned JSON Schema for CloudRoutineReceiptV1 receipts and conditional owner requirements. |
cli/src/limen/cloud_routine.py |
Implements the typed receipt model plus deterministic task id generation and idempotent upsert planning. |
scripts/cloud-routine-ingest.py |
Adds the receipt ingestion CLI that validates, classifies, dedupes, and optionally submits TABVLARIVS upsert tickets. |
cli/tests/test_cloud_routine.py |
Adds unit coverage for receipt validation rules, idempotent planning, and manifest contract publication. |
cloud-routines.json |
Binds the cloud-routine manifest to the published receipt schema and names the ingest consumer + delivery contract. |
institutio/governance/parameters.yaml |
Registers the LIMEN_CLOUD_ROUTINE_INGEST_APPLY arming parameter for the ingest consumer. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![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: fca5adc5ee
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdc51197f6
ℹ️ 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".
|
Terminal receipt for #2125 exact head |
|
New review correction packet is on #2125 at exact head |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (10)
cli/src/limen/intake.py (1)
162-162: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWrite the
$'literal directly.
"$" + "'"builds the two-character string$'at runtime. A raw string states the intent and removes the concatenation.♻️ Proposed change
- or (("$" + "'") in command and ("\\n" in command or "\\r" in command)) + or (r"$'" in command and ("\\n" in command or "\\r" in command))🤖 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 `@cli/src/limen/intake.py` at line 162, Update the condition in the command validation logic to use a raw string literal for the `$'` sequence instead of constructing it with `"$" + "'"`, while preserving the existing newline and carriage-return checks.spec/contracts/cloud-routine-receipt-v1.schema.json (1)
65-71: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a parity test for the executable allowlist.
Line 69 embeds the executable allowlist directly in the schema pattern.
EXECUTABLESincli/src/limen/intake.pylines 32-61 holds the same list. Two copies now exist, and nothing enforces agreement. The$commentat line 70 states parity but does not verify it.If a maintainer adds an executable to
EXECUTABLES, the published schema silently rejects predicates that the model accepts. Add a test that extracts the alternation from the schema pattern and compares it toEXECUTABLES.🤖 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 `@spec/contracts/cloud-routine-receipt-v1.schema.json` around lines 65 - 71, Add a parity test that extracts the executable alternation from the schema’s predicate pattern and compares the resulting set with EXECUTABLES from cli/src/limen/intake.py. Ensure the test fails when either list gains or loses an executable, while preserving the existing schema and validation behavior.cli/tests/test_cloud_routine.py (3)
274-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the baseline document passes, and match the governance validator configuration.
Line 277 builds
validbut the test never asserts thatvaliditself produces no schema errors. Every assertion from line 301 onward is negative. If the base fixture later violates the schema, all of those assertions still hold and the test stays green.Line 276 also omits
format_checker.scripts/check-cloud-routine-ingest.pyconstructs its validator withFormatChecker(), soformat: date-timeonobserved_atis asserted in production but not here.💚 Proposed change
- validator = Draft202012Validator(schema) + validator = Draft202012Validator(schema, format_checker=FormatChecker()) valid = _receipt().model_dump(mode="json")assert not list(validator.iter_errors(valid_substitution)) + assert not list(validator.iter_errors(valid))Add
FormatCheckerto the import at line 8.🤖 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 `@cli/tests/test_cloud_routine.py` around lines 274 - 313, Update test_published_schema_carries_executable_and_human_gate_constraints to construct Draft202012Validator with FormatChecker(), matching scripts/check-cloud-routine-ingest.py, and add a positive assertion that validator.iter_errors(valid) is empty before the negative cases. Import FormatChecker alongside the existing validator imports.
442-452: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winParse
gates.yamlinstead of matching substrings.Line 452 asserts that each path appears anywhere in the raw file text. The assertion passes if the path sits in a comment, in a different gate's entry, or in a
deploy_triggersblock. It does not prove that the scoped cloud gate implicates those paths.Load the YAML and assert that the paths appear in the implicated-paths list of the cloud-routine gate.
As per coding guidelines: "Declare each gate, its implicated paths, cost tier, serialization behavior, and deploy triggers in
institutio/governance/gates.yaml".🤖 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 `@cli/tests/test_cloud_routine.py` around lines 442 - 452, Update test_scoped_gate_covers_every_external_cloud_contract_artifact to parse gates.yaml as YAML, locate the scoped cloud-routine gate, and assert every listed artifact appears specifically in that gate’s implicated-paths list rather than anywhere in the raw text. Preserve the existing artifact set and validate the gate’s structured configuration.Source: Coding guidelines
348-356: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the script loader into a fixture.
The same six-line
spec_from_file_locationandexec_moduleblock appears in eleven tests in this file, at lines 348-356, 385-390, 403-408, 488-493, 505-510, 525-530, 560-565, 573-578, 593-598, 611-616, and 637-642. Each test re-executes the target script.Add one session-scoped fixture per script and inject it. The change removes the repetition and cuts test setup cost.
♻️ Proposed fixture
def _load_script(name: str): script = ROOT / "scripts" / name spec = importlib.util.spec_from_file_location(script.stem.replace("-", "_"), script) assert spec is not None and spec.loader is not None module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) return module `@pytest.fixture`(scope="session") def ingest_module(): return _load_script("cloud-routine-ingest.py") `@pytest.fixture`(scope="session") def checker_module(): return _load_script("check-cloud-routine-ingest.py")🤖 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 `@cli/tests/test_cloud_routine.py` around lines 348 - 356, Extract the repeated script-loading logic from the affected tests into a shared _load_script helper, then add session-scoped ingest_module and checker_module fixtures for the two scripts. Update all tests currently importing either script to accept and use the corresponding fixture, ensuring each script is executed once per test session.mcp/src/limen_mcp/intake.py (1)
146-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftThree copies of
is_executable_predicatenow exist.This block is identical to
cli/src/limen/intake.pylines 146-166 andweb/api/limen_intake.pylines 146-166. TheEXECUTABLESset at lines 32-61 is also identical in all three files. The published pattern inspec/contracts/cloud-routine-receipt-v1.schema.jsonline 69 holds a fourth copy of the allowlist.Each new rule must be applied four times by hand. Add a shared parity test that asserts the three Python copies agree, or extract the validator into one module that all three import.
Run the following script to confirm the three copies are identical today:
#!/bin/bash set -euo pipefail for f in cli/src/limen/intake.py mcp/src/limen_mcp/intake.py web/api/limen_intake.py; do echo "== $f" sed -n '120,170p' "$f" | sha256sum done rg -n --type=py 'def is_executable_predicate' -g '!**/tests/**'🤖 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 `@mcp/src/limen_mcp/intake.py` around lines 146 - 166, Eliminate the duplicated executable-validation logic shared by is_executable_predicate and the EXECUTABLES allowlist across the three Python intake modules. Extract these into one shared module and update cli, mcp, and web callers to import it, preserving current behavior and keeping the schema allowlist synchronized as required.cli/src/limen/cloud_routine.py (1)
301-301: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNormalize
createdto UTC.Line 301 takes the date from the raw
observed_atoffset. Line 396 converts the same value withastimezone(timezone.utc)before building the occurrence suffix. Two receipts that describe the same instant in different offsets then produce the same task identity but differentcreateddates.Use the UTC date so both derived values agree.
♻️ Proposed change
- created=receipt.observed_at.date(), + created=receipt.observed_at.astimezone(timezone.utc).date(),🤖 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 `@cli/src/limen/cloud_routine.py` at line 301, Update the receipt task construction around created and the occurrence suffix logic to derive created from receipt.observed_at converted to timezone.utc first, using its UTC calendar date. Keep the task identity and occurrence suffix based on the same normalized instant so receipts with different offsets produce consistent values.scripts/cloud-routine-ingest.py (1)
303-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the
observed_at=parser into one helper.The same context parsing exists twice: here at Lines 307-314 and in
_historical_cloud_task_stateat Lines 228 and 247-253. Both sites must agree on the context format. A change to the emitted context can update one site and miss the other.Extract one helper and call it from both sites.
♻️ Proposed refactor
+_OBSERVED_AT_RE = re.compile(r"observed_at=([^;]+)") + + +def _parse_observed_at(context: str) -> datetime | None: + match = _OBSERVED_AT_RE.search(context) + if not match: + return None + try: + return datetime.fromisoformat(match.group(1).replace("Z", "+00:00")) + except ValueError: + return Nonefor task in board.tasks: - context = str(task.context or "") - match = re.search(r"observed_at=([^;]+)", context) - if match: - try: - historical_observed_at[task.id] = datetime.fromisoformat( - match.group(1).replace("Z", "+00:00") - ) - except ValueError: - pass + stamp = _parse_observed_at(str(task.context or "")) + if stamp is not None: + historical_observed_at[task.id] = stamp🤖 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 `@scripts/cloud-routine-ingest.py` around lines 303 - 321, Extract the observed_at context parsing into a shared helper, preserving the existing ISO timestamp conversion and invalid-value behavior. Update both the board-task loop in the historical state setup and `_historical_cloud_task_state` to call this helper, removing their duplicated regex/parsing logic so both paths use the same context format.institutio/governance/gates.yaml (1)
739-739: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReconsider routing receipt-data edits through the heavy serialized suite.
Three points on this path list:
cli/tests/test_cloud_routine.pyis redundant.cli/**already selects it. The gate note explains that the non-clipaths exist becausecli/testsasserts on root registries it does not live beside. Acli/testspath is not in that class.The three
docs/receipts/*-20260808.jsonentries pin a date into the gate. The next census lands under a new filename, and this gate silently stops selecting it.scripts/check-cloud-routine-ingest.pyhardcodes the same dated filenames, so both surfaces drift together and go quiet rather than red.
pytest-cliistier: heavy,serialize: true,timeout_seconds: 1500. A data-only receipt edit now runs the full serialized suite. This gate's own note records the opposite decision fortasks.yaml: it is deliberately excluded because "routing every board publish through a 10-minute suite would trade this defect for a worse one."Consider a dedicated cheap gate for the cloud-routine surface running
python3 scripts/check-cloud-routine-ingest.pyplus the focused test file, and keep only the scripts and the schema onpytest-cli.♻️ Proposed direction
- paths: ["cli/**", "cli/tests/test_cloud_routine.py", "ianva/**", "his-hand-levers.json", "cloud-routines.json", "spec/contracts/cloud-routine-receipt-v1.schema.json", "scripts/cloud-routine-ingest.py", "scripts/check-cloud-routine-ingest.py", "docs/receipts/cloud-routine-findings-20260808.json", "docs/receipts/cloud-routine-lineage.json", "docs/receipts/irf-p0-owner-classification-20260808.json"] + paths: ["cli/**", "ianva/**", "his-hand-levers.json", "cloud-routines.json"]Then add one entry for the cloud-routine surface:
cloud-routine-ingest: command: "python3 scripts/check-cloud-routine-ingest.py && bash scripts/run-pytest-hermetic.sh cli/tests/test_cloud_routine.py -q" paths: ["scripts/cloud-routine-ingest.py", "scripts/check-cloud-routine-ingest.py", "spec/contracts/cloud-routine-receipt-v1.schema.json", "cli/src/limen/cloud_routine.py", "cli/tests/test_cloud_routine.py", "docs/receipts/cloud-routine-*.json", "docs/receipts/irf-p0-owner-classification-*.json", "cloud-routines.json", "his-hand-levers.json"] ci_job: "pr-gate.yml:pr-gate" owner: continuity note: "..."As per coding guidelines: "Verify changes with
scripts/verify-scoped.sh, which runs exactly the gates implicated by the diff ... do not run the whole matrix habitually."🤖 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 `@institutio/governance/gates.yaml` at line 739, Restructure the governance gate around the cloud-routine surface: remove the redundant cli/tests path and dated receipt filenames from pytest-cli, while retaining only the scripts and schema there. Add a cloud-routine-ingest gate using the proposed ingest checker plus focused test command, with glob-based receipt paths and the relevant source, registry, script, and schema paths; preserve the existing ownership and CI conventions, then verify with scripts/verify-scoped.sh.Source: Coding guidelines
scripts/check-cloud-routine-ingest.py (1)
49-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe IRF P0 invariant is implemented twice, and both copies hardcode the census size
41. One copy lives in the checker, the other inside the receipt's ownpredicatestring. The two can diverge, and the receipt copy is already the weaker one: it omits thederived_human != human_idsequality check and the lever-liveness check. Both break on the next legitimate census.
scripts/check-cloud-routine-ingest.py#L49-L56: derive the expected count from the receipt's owndenominatorfield and assert internal consistency, instead of comparing against the literal41. Apply the same treatment to the literal11at Line 110.docs/receipts/irf-p0-owner-classification-20260808.json#L299-L299: replace the inlinepython3 -cbody withpython3 scripts/check-cloud-routine-ingest.py, so one predicate owns the invariant.🤖 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 `@scripts/check-cloud-routine-ingest.py` around lines 49 - 56, The IRF invariant is duplicated with hardcoded census sizes, allowing the checker and receipt predicate to diverge. In scripts/check-cloud-routine-ingest.py lines 49-56, derive the expected row count from the receipt’s denominator and validate internal consistency instead of comparing with 41; apply the same approach to the literal 11 at line 110. In docs/receipts/irf-p0-owner-classification-20260808.json line 299, replace the inline predicate command with python3 scripts/check-cloud-routine-ingest.py so the checker exclusively owns the invariant.
🤖 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 `@cli/src/limen/cloud_routine.py`:
- Around line 41-49: Replace the inner [^'";|&]+ with the single-character
[^'";|&] form in the predicate pattern at cli/src/limen/cloud_routine.py lines
41-49 and spec/contracts/cloud-routine-receipt-v1.schema.json lines 65-71,
preserving the surrounding quantifier structure. Update the schema’s $comment
parity claim if needed so it remains accurate across both predicate patterns.
- Around line 341-355: Update the receipt-collapse loop to track each receipt by
lineage and observed_at timestamp, rather than comparing only with
latest_by_lineage. When a timestamp is seen again, compare the receipt with the
previously stored entry and raise ValueError for differences; continue selecting
the newest timestamp for latest_by_lineage while preserving collapsed-count
behavior and deterministic results.
- Around line 52-98: Bound recursion in _substitution_end and
_has_unsafe_command_substitution with an explicit depth parameter and a shared
maximum appropriate for validate_predicate’s input limit. Check the bound before
descending, treating an exceeded depth as unsafe: return None from
_substitution_end and True from _has_unsafe_command_substitution. Pass depth + 1
at every nested $(: recursive call so deeply nested predicates are rejected
without RecursionError.
In `@cli/src/limen/intake.py`:
- Around line 146-166: Update the shell-option scan in cli/src/limen/intake.py
lines 146-166, mcp/src/limen_mcp/intake.py lines 146-166, and
web/api/limen_intake.py lines 146-166 to consume values for value-taking options
such as -o, +o, --rcfile, and --init-file before applying the non-option break.
Preserve scanning through subsequent -c-like options so their operands receive
the existing composition guard, keeping all three intake copies and the Worker
contract consistent.
In `@docs/receipts/cloud-routine-findings-20260808.json`:
- Around line 102-111: The open-p0.denominator-41 predicate currently points to
the ingest-validation script, which does not verify that the finding is
discharged. Update the predicate in the receipt entry to invoke a check of the
human_gate_irf_ids set in irf-p0-owner-classification-20260808.json, requiring
that set to be empty while preserving the existing finding metadata.
In `@docs/receipts/cloud-routine-lineage.json`:
- Around line 1-4: Update the checker’s validation flow in
check-cloud-routine-ingest.py to load cloud-routine-lineage.json and assert that
every receipt in the validated findings collection has a matching task_id_for
value among the lineage entries. Validate that the lineage entries field is a
list, report malformed or unreadable lineage data through failures, and add a
failure identifying any finding absent from the tracked lineage while preserving
the existing findings and IRF checks.
In `@docs/receipts/irf-p0-owner-classification-20260808.json`:
- Around line 14-16: Clarify the receipt contract for the self-referential rows
represented by owner_kind "irf" and owner_ref "irf:<same id>": explicitly
document that this shape denotes machine-ownable rows without an external owner,
or replace those references with a durable external owner if that is not
intended. Keep the 23 non-human rows, the L-IRF-P0-HUMAN-ACTIONS-20260808 lever,
and the check-cloud-routine-ingest.py validation consistent with the chosen
contract.
In `@his-hand-levers.json`:
- Line 1188: Change the lever’s status value from the sentence to the enum token
open, and move the codex MCP auth_needed condition into the existing gate or
note prose field. Preserve the condition text while ensuring _lever_states can
compare status against TERMINAL_LEVER_STATUSES.
In `@scripts/check-cloud-routine-ingest.py`:
- Around line 132-135: Update the validate_human_gate_owners call in the checker
to pass latest_receipts_by_lineage(receipts), matching the collapse behavior
used by load_receipts in the ingest CLI while preserving the existing lever_path
argument.
In `@scripts/cloud-routine-ingest.py`:
- Around line 332-344: Update the submission loop in main around
submit_task_upsert to catch broker failures, store the error as submit_error,
and stop further submissions while preserving already appended task IDs. Ensure
the final result payload reports submit_error and main returns a non-zero status
(1) when it is set, while retaining the normal success behavior otherwise.
- Around line 192-200: Update _tracked_cloud_task_state and its caller to derive
cloud-routine-lineage.json from the repository ROOT rather than tasks_path,
while keeping the archive scan rooted at the board location. Ensure overrides
such as LIMEN_TASKS or LIMEN_ROOT cannot redirect the tracked receipt lookup and
cause duplicate lineage emission.
---
Nitpick comments:
In `@cli/src/limen/cloud_routine.py`:
- Line 301: Update the receipt task construction around created and the
occurrence suffix logic to derive created from receipt.observed_at converted to
timezone.utc first, using its UTC calendar date. Keep the task identity and
occurrence suffix based on the same normalized instant so receipts with
different offsets produce consistent values.
In `@cli/src/limen/intake.py`:
- Line 162: Update the condition in the command validation logic to use a raw
string literal for the `$'` sequence instead of constructing it with `"$" +
"'"`, while preserving the existing newline and carriage-return checks.
In `@cli/tests/test_cloud_routine.py`:
- Around line 274-313: Update
test_published_schema_carries_executable_and_human_gate_constraints to construct
Draft202012Validator with FormatChecker(), matching
scripts/check-cloud-routine-ingest.py, and add a positive assertion that
validator.iter_errors(valid) is empty before the negative cases. Import
FormatChecker alongside the existing validator imports.
- Around line 442-452: Update
test_scoped_gate_covers_every_external_cloud_contract_artifact to parse
gates.yaml as YAML, locate the scoped cloud-routine gate, and assert every
listed artifact appears specifically in that gate’s implicated-paths list rather
than anywhere in the raw text. Preserve the existing artifact set and validate
the gate’s structured configuration.
- Around line 348-356: Extract the repeated script-loading logic from the
affected tests into a shared _load_script helper, then add session-scoped
ingest_module and checker_module fixtures for the two scripts. Update all tests
currently importing either script to accept and use the corresponding fixture,
ensuring each script is executed once per test session.
In `@institutio/governance/gates.yaml`:
- Line 739: Restructure the governance gate around the cloud-routine surface:
remove the redundant cli/tests path and dated receipt filenames from pytest-cli,
while retaining only the scripts and schema there. Add a cloud-routine-ingest
gate using the proposed ingest checker plus focused test command, with
glob-based receipt paths and the relevant source, registry, script, and schema
paths; preserve the existing ownership and CI conventions, then verify with
scripts/verify-scoped.sh.
In `@mcp/src/limen_mcp/intake.py`:
- Around line 146-166: Eliminate the duplicated executable-validation logic
shared by is_executable_predicate and the EXECUTABLES allowlist across the three
Python intake modules. Extract these into one shared module and update cli, mcp,
and web callers to import it, preserving current behavior and keeping the schema
allowlist synchronized as required.
In `@scripts/check-cloud-routine-ingest.py`:
- Around line 49-56: The IRF invariant is duplicated with hardcoded census
sizes, allowing the checker and receipt predicate to diverge. In
scripts/check-cloud-routine-ingest.py lines 49-56, derive the expected row count
from the receipt’s denominator and validate internal consistency instead of
comparing with 41; apply the same approach to the literal 11 at line 110. In
docs/receipts/irf-p0-owner-classification-20260808.json line 299, replace the
inline predicate command with python3 scripts/check-cloud-routine-ingest.py so
the checker exclusively owns the invariant.
In `@scripts/cloud-routine-ingest.py`:
- Around line 303-321: Extract the observed_at context parsing into a shared
helper, preserving the existing ISO timestamp conversion and invalid-value
behavior. Update both the board-task loop in the historical state setup and
`_historical_cloud_task_state` to call this helper, removing their duplicated
regex/parsing logic so both paths use the same context format.
In `@spec/contracts/cloud-routine-receipt-v1.schema.json`:
- Around line 65-71: Add a parity test that extracts the executable alternation
from the schema’s predicate pattern and compares the resulting set with
EXECUTABLES from cli/src/limen/intake.py. Ensure the test fails when either list
gains or loses an executable, while preserving the existing schema and
validation behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5554a57f-2ba9-4dc3-8940-12e3cf2b452b
📒 Files selected for processing (16)
cli/pyproject.tomlcli/src/limen/cloud_routine.pycli/src/limen/intake.pycli/tests/test_cloud_routine.pycloud-routines.jsondocs/receipts/cloud-routine-findings-20260808.jsondocs/receipts/cloud-routine-lineage.jsondocs/receipts/irf-p0-owner-classification-20260808.jsonhis-hand-levers.jsoninstitutio/governance/gates.yamlinstitutio/governance/parameters.yamlmcp/src/limen_mcp/intake.pyscripts/check-cloud-routine-ingest.pyscripts/cloud-routine-ingest.pyspec/contracts/cloud-routine-receipt-v1.schema.jsonweb/api/limen_intake.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a756b915bb
ℹ️ 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".
| option = argv[index] | ||
| if option == "--" or not option.startswith("-"): | ||
| # After a script operand, later -c-like values are positional arguments. | ||
| break |
There was a problem hiding this comment.
-c
When a predicate is bash -o posix -c 'false; true', this scanner advances past -o, mistakes its posix operand for a script filename, and stops before inspecting -c; the predicate is therefore accepted and exits 0 despite the failed check. Fresh evidence beyond the prior combined-option reports is this option-operand bypass; the inspected Bash 5.2.21 bash --help lists both -c command and -o option, so options that consume arguments must be parsed before deciding that a script operand was reached.
AGENTS.md reference: AGENTS.md:L194-L198
Useful? React with 👍 / 👎.
| ) | ||
| _append_cloud_lineage_receipt(tasks_path, receipt) | ||
| submitted.append(task.id) |
There was a problem hiding this comment.
After --apply successfully submits a task, this call only edits docs/receipts/cloud-routine-lineage.json in the local checkout; no ingestion path commits or pushes that edit. Fresh evidence after the earlier persistence fix is that the new helper writes a tracked file but never publishes it, so a fresh checkout after the task is terminal and pruned still lacks the receipt and can recreate the same base task on replay. Persist the observation through the keeper or another remotely published owner rather than relying on an uncommitted working-tree mutation.
AGENTS.md reference: AGENTS.md:L160-L164
Useful? React with 👍 / 👎.
| "not": { | ||
| "properties": { | ||
| "predicate": { | ||
| "pattern": "(?:^|\\s)(?:bash|sh|zsh)\\s+[^\\r\\n]*-[^\\s]*c[^\\s]*\\s+[^\\r\\n]*[;|&]" |
There was a problem hiding this comment.
When a valid predicate is bash check.sh -c 'value;literal', the model accepts it because arguments after check.sh are positional, but this schema pattern scans through the script operand, treats the later -c as Bash's command option, and rejects the receipt. Fresh evidence beyond the prior intake-parser report is that the Python validator was corrected while this published mirror was not; bash --help documents the bash ... script-file ... invocation form, so schema-only producers are currently denied a model-valid one-command predicate.
AGENTS.md reference: AGENTS.md:L194-L198
Useful? React with 👍 / 👎.
| ingest.validate_human_gate_owners( | ||
| receipts, | ||
| lever_path=lever_path, | ||
| ) |
There was a problem hiding this comment.
When one of the 11 committed receipts is reclassified as material owned, superseded, or no_change with a missing or terminal lever: owner, this checker skips it because it invokes only validate_human_gate_owners, then can print the green durable-classification result even though load_receipts rejects the same denominator through validate_lever_owners. Fresh evidence beyond the consumer-side owner report is that ingestion now has the general validation while its advertised completion predicate still omits it; call the same general validator here before returning success.
AGENTS.md reference: AGENTS.md:L205-L208
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4489f4e907
ℹ️ 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".
| first = argv[command_index] | ||
| if first in {"bash", "sh", "zsh"}: | ||
| index = command_index + 1 | ||
| value_options = {"-o", "+o", "--rcfile", "--init-file"} |
There was a problem hiding this comment.
-O before treating its operand as a script
When a predicate is bash -O extglob -c 'false; true', -O consumes extglob, but this set omits that option, so the loop advances once, mistakes extglob for the script operand, and never inspects -c. The inspected Bash 5.2.21 bash --help lists both -O shopt_option and -c command, and this concrete invocation exits 0 after false; fresh evidence beyond the prior -o report is this distinct documented operand-taking option, which should be parsed before applying the script boundary.
AGENTS.md reference: AGENTS.md:L194-L198
Useful? React with 👍 / 👎.
| or "\\n" in program | ||
| or "\\r" in program | ||
| or (("$" + "'") in command and ("\\n" in command or "\\r" in command)) |
There was a problem hiding this comment.
When a predicate is bash -c $'false\x0atrue', these checks miss it because they recognize only the literal \n and \r spellings. Bash ANSI-C quoting decodes \x0a into a newline, so the nested program runs false followed by true and returns 0; fresh evidence beyond the prior \n report is this hexadecimal escape surviving the replacement check, so decode or reject every line-break escape form before accepting the program.
AGENTS.md reference: AGENTS.md:L194-L198
Useful? React with 👍 / 👎.
| normalized = value.strip() | ||
| if len(normalized) > 8192: | ||
| raise ValueError("predicate must be at most 8192 characters") | ||
| if _has_unsafe_command_substitution(normalized) or not _PREDICATE_SCHEMA_RE.fullmatch(normalized): |
There was a problem hiding this comment.
When a receipt uses test -r <(false), the bounded grammar and unsafe-substitution scan accept it because they recognize $( but not Bash process substitution. Bash runs false asynchronously and substitutes a readable descriptor, allowing test—and therefore the predicate executed by _run_predicate—to return 0 while the substituted command failed; reject <(/>( constructs or otherwise ensure their child status cannot be discarded.
AGENTS.md reference: AGENTS.md:L194-L198
Useful? React with 👍 / 👎.
| _LEVER_REF_RE = re.compile(r"^lever:[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") | ||
| _DURABLE_OWNER_RE = re.compile( | ||
| r"^(?:lever:[A-Za-z0-9][A-Za-z0-9._-]{0,127}|" | ||
| r"irf:[A-Za-z0-9][A-Za-z0-9._:-]{0,127}|" |
There was a problem hiding this comment.
irf: owners against the IRF registry
When a material owned, superseded, or no_change receipt supplies owner_ref="irf:DOES-NOT-EXIST", this alternative accepts it, while load_receipts resolves only lever: references. The planner consequently counts the finding as classified and emits no task even though the named IRF row has no durable owner; validate the identifier against the tracked IRF registry or remove this owner form from the accepted grammar.
AGENTS.md reference: AGENTS.md:L160-L165
Useful? React with 👍 / 👎.
| @field_validator("observed_at") | ||
| @classmethod | ||
| def validate_observed_at(cls, value: datetime) -> datetime: |
There was a problem hiding this comment.
When a JSON delivery supplies "observed_at": 0, Pydantic coerces it to the timezone-aware Unix epoch before this validator runs, so ingestion accepts a payload that violates the published schema's string date-time contract. For a previously observed lineage, that coerced 1970 timestamp compares older than history and is silently counted as a duplicate; for a new lineage it creates a task dated 1970. Validate the raw value in mode="before" or make the datetime field strict.
Useful? React with 👍 / 👎.
4444J99
added
the
lifecycle:delivery
Summary
CloudRoutineReceiptV1with routine identity, timezone-aware observation, stable finding key, disposition, owner, and executable predicatenew_workdispositions into provider-neutral tasksSafety
The consumer is dry-run by default. Emission requires both
--applyandLIMEN_CLOUD_ROUTINE_INGEST_APPLY=1; it callssubmit_task_upsertand never editstasks.yaml.Verification
Focused model, intake-contract, idempotency, manifest, parameter, and contract-schema checks run in exact-head CI.
Umbrella owner: #2120
Summary by CodeRabbit