Skip to content

fix(broker): accept only canonical cc-<n> ticket ids - #322

Merged
ScriptedAlchemy merged 3 commits into
masterfrom
fix/canonical-ticket-ids
Sep 26, 2026
Merged

ScriptedAlchemy merged 3 commits into
masterfrom
fix/canonical-ticket-ids

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Why

A zero-padded ticket id never woke its waiters. parseTicket in src/internal/contracts/protocol.ts accepted cc-01 as id 1, so ledger.getRequestByTicket("cc-01") found row 1. The broker keys waiters by the raw string, in dependencies.ts for --after and in awaitTicket in broker.ts. notifyWaiters("cc-1") therefore never woke a --after cc-01 dependent. The dependency wait has no timeout, so that dependent stayed queued forever. hauler await cc-01 slept its full wait.

The new broker test reproduced it before the fix. The daemon acknowledged the dependent with waitingFor: ['cc-01'].

Scope

ticketPattern in src/internal/contracts/protocol.ts now matches only ^cc-([1-9]\d*)$, the spelling formatTicket produces. Dependencies, await, result, and fetch resolve tickets through parseTicket, either directly or through ledger.getRequestByTicket, so they now agree with the waiter keys on one spelling. kill already matched the in-memory directory by exact string and refused cc-01 before this change.

Blast Radius

A padded or zero id is now unknown everywhere. --after cc-01 is rejected as a bad intent naming cc-01. hauler await cc-01 and hauler result cc-01 report the ticket as not found. Ledger ids start at 1, so cc-0 never named a real ticket. Canonical ids behave as before.

Verification

tests/unit/util/utils.test.ts asserts parseTicket('cc-042') is null. It failed before the fix with expected 42 to be null.

tests/integration/daemon-after.test.ts drives the in-process broker over the fake cargo. A --after cc-01 submit is rejected with unknown prerequisite ticket cc-01. A --after cc-1 dependent waits on cc-1 and finishes done. awaitTicket('cc-01') returns no record. Before the fix the padded submit was accepted.

The real CLI confirms the same. hauler request --after cc-01 is rejected. hauler await cc-01 returns "not found" in under 2 seconds while cc-1 runs. A --after cc-1 dependent runs to done.

pnpm run check passes.

Principles

Fix Root Causes. The fix lives in the one parser every consumer shares, not in the waiter maps.

Test Behavior, Not Implementation. Both tests call the parser and the broker the way users do and assert literal ids and messages.

Laziness Protocol. The diff is one regex.

@changeset-bot

changeset-bot Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e35c790

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
cargo-hauler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-26T00:31:18.116543Z 2225624 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ScriptedAlchemy
ScriptedAlchemy merged commit 1702562 into master Sep 26, 2026
@ScriptedAlchemy
ScriptedAlchemy deleted the fix/canonical-ticket-ids branch September 26, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant