Repository navigation
Conversation
One application service over the CB-02 planning read model, used by the coordination:read CLI command and by an optional read-only coordination listener on a second Unix socket (ADR-0036 Decisions 5 and 7). - Exposure registry and explicit P/T mappers with a best-effort credential scrubber, 256-code-point truncation flagged per item, a 256 KiB cap. - Operations status, milestones, tasks, requirements, dependencies, blockers and runnable; optional allowed-class policy. - Listener disabled by default, enabled per project and data class in <runtime home>/coordination-listener.json; GET-only coordination routes and a minimal health route, no commands or dashboard routes, no audit row on a read; an invalid file keeps it closed and the Runtime up. - The SQLite planning read refuses to join an open writer transaction on the shared connection (RUNTIME_BUSY) instead of reading uncommitted state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
…ener Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
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. |
- Never take over the main socket or a socket that still answers. - Enforce the per-project exposure policy in CoordinationReadService. - Cut relationship lists at 100 entries so one item always fits. - Extend the credential scrubber (Stripe, GitLab, npm, Google OAuth, Basic auth, URL userinfo, secret assignments, zero-width splits), bound scanned text and never leave a partial redaction marker. - Retry a planning read that meets a short writer transaction. - Bound concurrent listener reads, log internal failures by type only, create the socket owner-only, stop it without awaiting slow clients. - Read the configuration with O_NOFOLLOW and require a regular file owned by the Runtime user and not writable by others. - runtime:purge removes a stale coordination socket. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
|
@codex review |
![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: 5b4ed484a9
ℹ️ 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".
- The coordination snapshotId is the digest of the exposed projection, so a scrubbed credential or a cut title tail cannot change it. - runtime:purge refuses while coordination.sock accepts connections. - Only a refused connection marks the coordination socket stale. - Open the listener configuration with O_NONBLOCK so a FIFO cannot block Runtime start. - Close the listener before stopping: new reads answer RUNTIME_BUSY. - Scrubber: keep text without a credential unchanged, quoted keys and values, more Slack prefixes and invisible characters, bounded code-point conversion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
|
@codex review |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…e the projection - Requirements in the exposed projection are ordered by exposed key, so a scrubbed key cannot reveal where its stored value sorts. - The scrubber redacts quoted values with escapes and unterminated ones. - The exposed projection is cached per CB-02 identity, so unchanged state is mapped and hashed once across pages and status reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
An unterminated quoted value after a secret keyword is now redacted to the end of its line, escapes included, so a multi-word passphrase cannot leave its tail behind. Also reattach the exposedProjection docblock and name the cache test after what it checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cda876fd04
ℹ️ 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".
…ned items - An unterminated quoted value ending in a lone backslash is redacted. - Collection pages return detached copies, so changing a returned item cannot change the cached exposed projection served to later reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
Milestones kept the read model's creation-time order, a value the coordination exposure never returns, so swapping only creation times changed page order, snapshotId and pageDigest. Exposed milestones are now ordered by id and the protocol lists creation times among the values the exposed identity does not depend on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
…alues Pin single quotes with CRLF, odd backslash runs, an escaped closing quote at the end of a line, the accepted redaction of an empty unterminated value, and a bounded scan of a long backslash run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
A keyword after an underscore, as in DB_PASSWORD or GITHUB_TOKEN, now starts an assignment, and an escaped line break no longer ends a closed quoted value early, so the rest of the value is redacted with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
…ating Both stored keys sorted after "R-2", so the test passed without the exposed-key sort. One key now sorts before it and one after. The protocol no longer claims that list entries past 100 leave the identity unchanged: counts still reflect the uncut relationships. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
…d accessToken Claude-Session: https://claude.ai/code/session_01Q21NUKiVwrCvpZ5YgzjM5W
….4 and harden socket tests Bun 1.3.6 reports ENOENT, not ECONNREFUSED, when connecting to a stale socket file, so the listener never replaced it. ENOENT now counts as stale only while the path is still a socket. Socket-holder tests wait until the holder accepts connections, keep paths within the macOS sun_path limit and pass the path through the environment instead of building script text from it (CodeQL js/bad-code-sanitization). Claude-Session: https://claude.ai/code/session_01Q21NUKiVwrCvpZ5YgzjM5W
…dService.execute The policy parameter defaulted to the owner policy, so an adapter that omitted it would silently gain every project and the P and T classes. The contract now requires the caller to state its authority: the CLI passes owner, the listener passes its exposure policy, and tests pass an explicit policy. A regression test pins that a caller omitting the policy gets no data. Claude-Session: https://claude.ai/code/session_01FKM37BV4LS8Rc8ny5GCwXy
…nd error Review hardening: an omitted or malformed policy was refused only because reading its kind threw a TypeError. The service now refuses it explicitly with ProjectNotFoundError and the regression test asserts that type. Claude-Session: https://claude.ai/code/session_01FKM37BV4LS8Rc8ny5GCwXy
|
Update for head 0a5301c: ChatGPT P2 confirmed and fixed. |
avalla
left a comment
avalla
left a comment
There was a problem hiding this comment.
Independent ChatGPT review — HEAD 0a5301c (static source review, not Codex CLI)
Technical verdict: no newly confirmed P1/P2 findings in the reviewed code. Reviewed the coordination application service, policy enforcement, CLI and listener call sites, route parser, exposure mapper, SQLite planning-read adapter, listener startup, and purge guard. CoordinationReadService.execute now requires an explicit policy; the CLI supplies owner, while the listener supplies project-scoped exposure. An absent/unknown runtime policy fails closed. The listener has no command or dashboard fallthrough, and the purge code refuses on a live or inconclusive coordination socket. Redacted projection data is used to derive the coordination snapshot digest.
P3 follow-up (non-blocking): consider a direct listener unit assertion of the passed exposure policy, in addition to the existing behavioural e2e coverage. The text-scrubber is documented as best-effort, not a general secret-scanning guarantee.
Evidence: GitHub workflows on the published HEAD are green. I did not run Bun locally, so this review relies on source inspection and CI records. The PR description's consolidation note still names an earlier HEAD and should be refreshed.
Gate: This review is not codex review --base origin/main; leave the configured external-review gate distinct.
…ener policy - The assignment scrubber required a non-alphanumeric character before the keyword, so prefixed camelCase names such as dbPassword, githubToken or rsaPrivateKey were exposed. A case-sensitive companion pattern now matches a capitalised keyword that follows a lower-case letter or digit; a single lower-case word such as mytoken stays untouched. - Add a direct assertion that the listener passes its per-project exposure policy to the read service, and that a client cannot select the owner policy or widen the enabled set through query parameters. Claude-Session: https://claude.ai/code/session_01FKM37BV4LS8Rc8ny5GCwXy
An upper-case acronym running into the keyword (DBPassword, JWTSecret, GCPApiKey) was not redacted because the camelCase pattern required a lower-case letter or digit before it. Accept any letter or digit; keywords stay capitalised, so a single lower-case word such as mytoken is still untouched. Claude-Session: https://claude.ai/code/session_01FKM37BV4LS8Rc8ny5GCwXy
|
Hardening cycle closed on |
Summary
Adds the M23 client-neutral coordination read API (CB-10): one application service over the CB-02 planning read model, exposed by the
coordination:readCLI command and by an optional, read-only coordination listener on a second Unix socket (ADR-0036 Decisions 5 and 7). Later adapters (GitHub bridge, MCP server) use the same service through that listener; none recomputes the read model or has an authority path of its own.Task
3a112f30-fc68-4ccd-9d1b-874148287f6c— CB-10 Client-neutral coordination API (requirement CB-R10)status,milestones,tasks,requirements,dependencies,blockers,runnable)/api/coordination/v1/...and a minimal health query, no/commands, no dashboard routes, disabled by defaultcoordinationReadOperations)Scope
coordination-exposure.ts(registry, mapper, credential scrubber, 256-code-point truncation withtruncatedFields),coordination-read-service.ts(operations, optional allowed-class policy, 256 KiB response cap),coordination-listener-configuration.ts(closed-schema parser),GetPlanningSnapshot.readSnapshot(extracted, behaviour unchanged).coordination-query-protocol.ts(prefix, route and parameter grammar, limits, error codes; pure, for the CB-11B query client).coordination-query-api.ts(thin HTTP transport that never falls through), secondBun.serve({ unix })in the host, configuration file loader.PlanningReadBusyError→RUNTIME_BUSY), so no adapter can observe uncommitted state.coordination:read, help line.Dependencies
808d771, verified on main)mainat808d771Design notes
<runtime home>/coordination-listener.json,{"version":1,"enabled":true,"projects":[{"projectId":"…","classes":["P","T"]}]}, read once at start. Missing means disabled. Invalid, oversized, symlinked or unreadable means misconfigured: the listener stays closed and the Runtime runs unchanged.<runtime home>/coordination.sock, mode 0600. Only a stale socket is replaced, never another file at that path. A bind failure leaves the listener closed and never the Runtime down.404 Project not found, exactly like a missing one. Requested classes must be a subset of the enabled ones.coordination:readon the main socket records only the host's generic command envelope, as every command does.pageDigestover the whole exposed response except that field (same definition as CB-02, recomputed after mapping).snapshotIdis the CB-02 identity.INVALID_REQUEST400,NOT_FOUND404,METHOD_NOT_ALLOWED405,PAYLOAD_TOO_LARGE413/414,RUNTIME_BUSY503 withretry-after,INTERNAL_ERROR500 without detail.Pipeline exception
The project's enforced
deliverypipeline could not run as a binding Runtime pipeline:openai:gpt-6.1-soland the reviewer toopenai:gpt-5.5;gpt-5.5(codex-cli 0.163.0is installed);Changing routing, the allowlist or credentials only to get past this was excluded by the owner. The work therefore follows in-session task-delivery with independent subagents, as for CB-01 and CB-02.
Verification
See the hardening update below (final head, checks and reviews).
Merge
Do not merge without explicit authorization. READY FOR MERGE is not DONE: post-merge verification is still required.
🤖 Generated with Claude Code
https://claude.ai/code/session_01RMasEtuNxnMyf4ZRrFndu9
Hardening update (2026-10-19) - HEAD
21fb235dbPassword,githubToken,rsaPrivateKey,awsSecretAccessKey,DBPassword,JWTSecret,GCPApiKey) are now redacted by a case-sensitive companion pattern that shares the value half of the existing one; the existing pattern is byte-identical (verified), and a single lower-case word such asmytoken=stays untouched. Negative tests cover prose, calls and short values.{kind: "exposure", projects}(the listener's own map) to the read service and thatpolicy,kindorprojectsquery parameters are rejected with 400 before any read. A mutation to the owner policy fails the test.Evidence:
bun run checkexit 0 on21fb235(250 files, 3623 passed, 242 skipped); GitHub CI 13/13 green. Independent read-only review: no P1/P2; one P3 (acronym prefixes), fixed in21fb235; no ReDoS found up to 50,000 characters.Not in this PR, tracked in task
92cf4e55-aaf4-47b0-9d9a-42f79d6d1ac6: upper-case keyword glued to a prefix (dbPASSWORD), over-redaction of names likenext_page_token, and a shared scanner design. The scrubber remains documented as best-effort.External review (Codex): not satisfied on the final head. The Codex CLI and the GitHub connector are unavailable (usage limit); no other review stands in for it. The independent review below is by a subagent of the same model family, not Codex.