Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 0b93e26eeb
ℹ️ 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".
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?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: 2b233fadf5
ℹ️ 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: 0c7f56b118
ℹ️ 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: fe1e197292
ℹ️ 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: 0d9bb9c59e
ℹ️ 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: e176f29bf6
ℹ️ 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: e84a9246a7
ℹ️ 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".
A hosted kind may declare a closed-object resultSchema beside its displaySchema, and each outcome may declare resultRequired. A requester may narrow declared fields per item at create time through resultConstraints (enum, oneOf with titles, minimum/maximum, minLength/maxLength), where every choice and bound must already satisfy the pinned kind schema: the compiled schema is the authority and the subset rules never re-implement JSON Schema semantics. A decision may then carry a result that validates against the schema and the accepted narrowing. All new fields are serde-optional and skip serialization when absent, so existing policies, snapshots, and request payloads keep byte-identical canonical forms and digests: the starter kind digest is pinned by test at sha256:6f9fbf8b801dda6527aece6bbf34cdc7d0b8ff7899d9e012367302aff1aa3139. Five validation reasons join the catalogue (result_not_declared, result_required, field_not_declared, constraint_invalid, constraint_violated) with header-safe instance paths that fall back to the payload root prefix when a field name is not header-safe. Deviation from spec section 3.7 noted for the reviewers of the whole change: hosted validation failures are HTTP 400 request.invalid with the registry-casework-validation-path/-reason headers in this tree, exactly like the existing nine reasons; the section's 422 wording is drift. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Migration 0015 adds result_constraints to casework_hosted_items, result to casework_hosted_terminal_events, and result_digest to casework_hosted_accountability. The items payload-erasure CHECK now also forbids surviving constraints once the requester payload is erased; constraints are optional at creation, so they may be NULL while the payload is live but never present after erasure. Create stores accepted constraints after re-checking them against the pinned snapshot, and they ride the create idempotency hash via the canonical request bytes. Decide validates the result against the pinned snapshot and the item's stored constraints, persists the result on the terminal event, its sha256-over-canonical-JSON digest on the accountability record, and the result verbatim in the completed history detail. The requester projection, staff work-item context, terminal page, and accountability record each expose their new field, and the erasure clock nulls constraints with the rest of the payload. The Rust, Python, and Node clients register the five new validation reasons, keeping the cross-catalogue gate green. Security note (data minimization and erasure): the result is Casework's own decision data and rides the same terminal clock as the display payload; the accountability row keeps only a digest of the canonical result until its own clock expires, and both are deleted by the existing retention sweep. Spec section 3.7 drift noted in the core commit: these failures answer 400 request.invalid with validation headers, not 422. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The hosted PostgreSQL suite gains three tests: constraints flow from create through the requester projection, staff work-item context, a narrowed decision, the terminal page, and an accountability digest equal to sha256 over the canonical result JSON; a create replay under the same idempotency key with different constraints conflicts while a refused payload leaves the key reusable; and the result payload erases on the terminal clock with the rest of the requester payload while the digest survives until the accountability clock deletes its row. The standalone HTTP journey gains a wire test asserting each of the five new validation reasons arrives as HTTP 400 request.invalid with the snake_case reason and instance path headers, using an in-code result-capable kind so the shipped example stays result-free. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
The generator documents resultRequired on outcomes, resultSchema on the kind policy and staff context, resultConstraints on the create request, requester projection, and staff context, result on decisions and completed terminal results, and resultDigest on accountability records, each optional and bounded by the same 16 KiB canonical and depth-16 payload limits as display. The hard terminal-fields check and the bound markers now pin the result payload constants too. The configure page evidence anchor follows the renamed check_closed_object_schema helper so the docs anchor gate stays green. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Clippy prefers the strict bound form without the plus-one offset, and the migration ledger tests now expect head 15 after the hosted result migration. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…elds Regenerate the @registrystack/client facade typings via release/scripts/sync-registry-client-node.py so the optional hosted result surface (resultRequired on outcomes, resultSchema and resultConstraints on kind policy, work-item context, create requests, and requester items, result on decisions and completed terminal results, resultDigest on accountability records) reaches the unified package exactly as the maintained registry-casework-client-node binding declares it. Security note (data minimization review): the facade keeps result payloads on the requester terminal-results surface only; the accountability surface exposes the sha256 result digest, not the result itself, matching the Rust and maintained Node typings. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
… clients Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…ment The hosted validation-reason header enum on the four hosted validation routes still named only the original nine reasons after the structured result work, so a generated client would reject the server's own 400 refusals for result_not_declared, result_required, field_not_declared, constraint_invalid, and constraint_violated. Append the five codes in HostedValidationReason declaration order and regenerate the committed document; generate_openapi.py --check passes and both result routes now carry the fourteen-value enum. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
…acks
Two review findings on the result validation core.
Bound containment (check_constraint_bounds) compared constraint bounds
only against keywords written directly on the property object, so a
property like {"type":"integer","$ref":"#/$defs/bounded"} or one
hiding its bound behind allOf accepted a widening constraint at create
and left the kind schema to arbitrate at decide. Bound keywords are now
refused unless the property subschema is plain inline: no $ref sibling
and no allOf/anyOf/oneOf, whose effective bounds are not statically
visible. Choices (enum/oneOf values) on such properties are unchanged
because they are validated by instance against the compiled schema.
The sanitized-path fallback in HostedValidationError::new was the
literal $.display, so a result error under a field name that cannot
travel in a response header (a space, a non-ASCII byte) was mislabeled
$.display while display was valid. The fallback is now scoped:
display validation keeps $.display, result validation falls back to
$.result, and constraint errors fall back to $.resultConstraints.
Tests: bound_constraints_require_plain_inline_properties and
result_errors_fall_back_to_the_result_root_not_display; the existing
inline-bound and display-fallback tests stay green.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Spec 3.5 requires the If-Match, idempotency, deciding-profile, and claim checks to run before any result validation, but the decide transaction validated the result first: an officer submitting an invalid result without a claim, with a stale revision, or under a reused idempotency key learned a 400 validation refusal instead of the semantic error, and an unclaimed item leaked schema feedback to a caller who could not decide it at all. The store now runs result validation after the deciding-profile re-check, the idempotency replay/conflict, the revision/state If-Match, and the holder check, inside the same FOR UPDATE transaction; the check returns the typed StoreError::HostedValidation so the wire keeps the bounded reason and path headers. The service drops its eager request check, keeps the hosted_policy_for_actor gate so an officer without deciding authority still learns only that the item is not found, and converts the store's typed refusal to ServiceError::HostedValidation, preserving the existing error shape. Security note (data minimization review): the reorder means validation feedback now requires deciding authority plus a live claim on the item at the expected revision, so result-schema probing cannot use validation refusals as an oracle on items the caller cannot decide. Test: decide_semantic_checks_precede_result_validation pins the conflict, stale-revision, and idempotency-conflict precedence over validation; the existing wire-reason and results-flow tests stay green. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
e84a924 to
c203181
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2031810c1
ℹ️ 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".
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ff65a6945
ℹ️ 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".
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa8b509ae5
ℹ️ 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".
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd9f00340c
ℹ️ 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".
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36d3c24523
ℹ️ 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".
| Err(_) => { | ||
| budget_exhausted = true; | ||
| break; |
There was a problem hiding this comment.
When the first candidate's source preflight reaches the global page deadline, examined has already been incremented, but this branch exits before assigning continuation. The later examined == 0 outage check is therefore skipped and the endpoint returns an empty successful page with nextCursor: null, even though unexamined tasks remain; clients can interpret that as the end of the inbox and miss work. Return SourceUnavailable or a resumable cursor for the timed-out candidate.
AGENTS.md reference: AGENTS.md:L124-L126
Useful? React with 👍 / 👎.
| WHERE request_entity_id=$1 AND request_id=$2 AND proposal_version=$3 | ||
| AND state='cancelling'", |
There was a problem hiding this comment.
In an HA deployment, the cancellation HTTP timeout may equal the 30-second lease, so another replica can reclaim the row while the first request is still finishing. A stale failure then matches only the submission identity and state='cancelling', clears the replacement worker's lease, and immediately makes the row reclaimable for another duplicate cancellation; carry the claimed lease value or an ownership token through both cancellation completion paths.
Useful? React with 👍 / 👎.
| let token = std::str::from_utf8(secret.expose_secret()) | ||
| .map_err(|_| RuntimeError::CompletionConfiguration)? | ||
| .to_owned(); |
There was a problem hiding this comment.
If a referenced token contains a trailing newline, whitespace, non-ASCII text, or another HTTP-header-invalid byte, the secret resolver and this UTF-8 conversion accept it and the runtime starts normally. bearer_auth then records an invalid-header error, every delivery attempt fails locally, and the outbox eventually becomes exhausted without ever contacting the receiver; validate the resolved value with the shared BearerToken type during startup.
Useful? React with 👍 / 👎.
| AND ($3::uuid IS NULL OR (occurred_at,event_id)>( | ||
| SELECT occurred_at,event_id FROM casework_review_history | ||
| WHERE request_id=$1 AND event_id=$3 | ||
| )) |
There was a problem hiding this comment.
When a caller supplies a well-formed cursor that is absent or belongs to another request, this scalar subquery returns no row, making the tuple comparison unknown for every history entry. The endpoint consequently returns a successful empty page with no continuation even when later entries exist, silently truncating traversal; resolve and validate the cursor before running the page query and return the cursor-expired/invalid response when it cannot be found.
Useful? React with 👍 / 👎.
| FROM casework_review_clock_occurrences o | ||
| LEFT JOIN casework_review_tasks t ON t.task_id=o.task_id | ||
| WHERE o.request_id=$1 ORDER BY o.clock_id,o.scope,o.clock_occurrence_id", |
There was a problem hiding this comment.
When accountabilityDays exceeds terminalDays, terminal cleanup leaves clock occurrences and the minimized request row until the later accountability deadline, and this query has no result_available_until check. A requester can therefore continue reading retained subject-clock correlations—including the exact source, subject type and subject ID plus timing—after the declared request-payload deadline; fresh evidence beyond the earlier source-visibility fix is that these retained rows remain directly queryable here after tasks, history, context, and results are erased.
Useful? React with 👍 / 👎.
| if complete.value.subject != request.subject | ||
| || &complete.value.submission_digest != expected_submission_digest | ||
| { |
There was a problem hiding this comment.
If Casework or an intermediary returns the correct subject and submission digest but a policy binding for another review kind, this maintained client accepts it because it never checks policy.id against request.kind or validates the policy binding. The returned value then becomes the expected policy for subsequent result validation, so Rust, Node, and Python callers can accept a terminal result governed by the wrong policy instead of reporting a protocol failure; apply the same accepted-response checks as registry-review-client.
Useful? React with 👍 / 👎.
| AND (state='source_facts_missing' | ||
| OR (state='running' AND next_action_at<=now())) |
There was a problem hiding this comment.
When an activity clock is waiting for a holiday document, its occurrence remains source_facts_missing, and this selector makes it due unconditionally on every two-second maintenance pass. Updating updated_at—including the explicit defer path—never makes that state temporarily ineligible, so each task repeatedly performs an authoritative source read and database evaluation until the holiday data appears; with many missing clocks or a slow source this can saturate the source and delay the same maintenance loop's retention work. Gate this state on a retry timestamp or an updated_at backoff.
Useful? React with 👍 / 👎.
2 participants
Summary
Checks
products/casework/scripts/check-review-examples.shcargo clippy --locked --profile ci --all-targets -- -D warningscargo fmt --all -- --checkandgit diff --checkNotes
DCO
Signed-off-bytrailer.