Split comenq into put, list, bump, bust, and del subcommands - #149
Draft
leynos wants to merge 7 commits into
Draft
Split comenq into put, list, bump, bust, and del subcommands#149leynos wants to merge 7 commits into
leynos wants to merge 7 commits into
Conversation
added 5 commits
July 19, 2026 17:24
Add a `protocol` module to `comenq-lib` with tagged `Request` and `Response` enums covering the forthcoming queue operations: `put`, `list`, `bump`, `bust`, and `del`. `PendingEntry` carries the deterministic identifier, approximate ETA in seconds, target pull request, and full comment body; display truncation is left to consumers. Each connection will carry one JSON request and one JSON reply. Derive `Clone` on `CommentRequest` so queue entries can embed it.
Introduce `comenqd::store::QueueStore`, a filesystem-backed queue holding one JSON file per pending comment under `<queue_path>/entries`. Each entry carries: - a deterministic eight-character identifier (64-bit FNV-1a over the request fields and enqueue time, first eight hex digits), stable across restarts; - an explicit ordering key so entries can be moved to the head (`bump`), moved to the tail (`bust`), or removed (`del`); and - a flutter sampled at enqueue time, fixing the entry's estimated posting time from the moment it is reported. The Unix time of the most recent successful post persists in `<queue_path>/last_post`, letting `schedule` project an estimated posting time for every pending entry: the head is due one full cooldown plus its own flutter after the last post (immediately when nothing has been posted), and each successor follows its predecessor's projected time by a further cooldown plus flutter. Files are written to a temporary sibling and renamed into place so entries are never observed half-written. The append-only `yaque` queue cannot express reordering or deletion, so subsequent commits move the daemon onto this store.
Replace the yaque-based pipeline (listener → mpsc channel → queue writer → worker) with a `SharedQueue` that bundles the persistent store, the daemon configuration, and a change notifier: - The listener now parses protocol `Request`s, executes them against the shared queue, and writes the JSON `Response` back over the connection. Malformed requests receive an error reply instead of a silently dropped connection. - The worker recomputes the head entry's due time on every iteration and sleeps until it falls due; queue mutations and shutdown interrupt the wait, so bump, bust, and del take effect immediately. Successful posts record `last_post`; API failures keep the entry and retry after a full cooldown. - The supervisor sheds the queue-writer task, its respawn logic, and the client channel; it now supervises just the listener and worker over the shared queue. Remove the `yaque` dependency and its metadata-file helper. Rewrite the daemon integration tests and the listener and worker cucumber steps against the protocol, strengthening the success scenario to assert the posted entry leaves the queue — which also caught the success mock returning an unparseable empty body.
Replace the single-purpose argument list with five subcommands: - `put owner/repo <pr> <body>` enqueues a comment and prints its deterministic identifier with the daemon-reported approximate ETA (`Queued 1a2b3c4d for owner/repo#7 — posts in ~1h 01m`). - `list` prints one line per pending comment: identifier, ETA, target pull request, and the body collapsed to a single line and truncated to sixty characters with an ellipsis. - `bump <id>`, `bust <id>`, and `del <id>` move an entry to the head or tail of the queue, or remove it. The `--socket` flag becomes global (still discoverable via `$XDG_RUNTIME_DIR` and overridable with `COMENQ_SOCKET`). The client now completes a full transaction: it writes one protocol request, half-closes, reads the JSON reply, and surfaces daemon-reported errors distinctly from transport failures. Rendering lives in a new `output` module with unit-tested ETA and summary formatting.
Add `tests/features/queue.feature` with scenarios exercising the daemon's protocol surface end to end: `put` replies carry an eight-character identifier and an ETA, identifiers reappear in listings, listings report strictly increasing ETAs in posting order, long bodies collapse to a one-line sixty-character summary, `bump` moves an entry to the head, `bust` to the tail, `del` removes it, and unknown identifiers are rejected with a named error. Refresh the documentation for the new architecture: the design document now describes the bespoke `QueueStore` (entry format, identifier scheme, ordering keys, atomic writes, `last_post`, ETA projection), the request/response protocol, the two-task supervisor topology, and the client subcommands; the `comenq` man page and README document `put`, `list`, `bump`, `bust`, and `del`. Remove the vestigial `client_channel_capacity` configuration field — nothing has buffered client requests since the queue-writer task was removed.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
added 2 commits
July 19, 2026 18:20
A default `put` previously posted immediately when the queue was idle. Each entry now records a `not_before` floor — its enqueue time plus one full cooldown plus its own sampled flutter — and the schedule takes the later of that floor and the chain-projected due time, so even an idle queue paces a fresh comment and `bump` cannot circumvent the floor. `comenq put --now` (protocol field `immediate`) lifts the floor, restoring the previous post-as-soon-as-possible behaviour. The field defaults off on the wire and in the store, so entries persisted before this change behave like immediate puts. Group the store's put inputs in `PutOptions` (cooldown, flutter ceiling, immediacy), cover the floor with store unit tests and new queue scenarios, and document the behaviour in the design document, man page, and README.
Bring in the stale-socket discovery fix: candidates are probed by connecting in order rather than checking file existence. Adapt the subcommand client's `transact` to connect through `connect_first` over `Args::socket_candidates`, so every operation (put, list, bump, bust, del) falls past a stale user socket to a healthy system daemon.
This was referenced Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch splits the
comenqclient into queue-management subcommands —put,list,bump,bust, anddel— and rebuilds the daemon's queue so those operations are possible.putnow prints the comment's deterministic eight-character identifier and an approximate ETA;listprints the pending schedule with identifiers, ETAs, and one-line sixty-character summaries;bump,bust, anddelreorder or remove entries by identifier. The append-onlyyaquequeue cannot reorder or delete entries, so the daemon now uses a bespoke persistent store, and the client-daemon socket exchange becomes a JSON request/response protocol. Random flutter is pre-calculated when a comment is enqueued, so the ETA reported atputtime is the schedule the worker executes; cooldowns always run in full and flutter only ever lengthens the wait. A defaultputadditionally waits one full cooldown (plus its flutter) from enqueue even when the queue is idle — each entry records anot_beforefloor thatbumpcannot circumvent — andput --nowlifts the floor to post as soon as the queue allows.Stacked on #148 (
user-hosted-execution); the base branch of this pull request isuser-hosted-executionand it should merge after #148. Note the daemon's on-disk queue format and wire protocol change together, so client and daemon must be upgraded in lockstep (the previous queue format's files are ignored; the store starts empty).Review walkthrough
Requestper connection (op:put/list/bump/bust/del), answered by oneResponse(okwith optionalentry/entries, orerror).<queue_path>/entries(atomic write-then-rename), an explicit ordering key (bump= head minus one,bust= tail plus one), flutter sampled at enqueue, deterministic identifiers (first eight hex digits of a 64-bit FNV-1a hash over the request fields and enqueue second; an identical request within the same second is idempotent), a persistedlast_posttimestamp, and the ETA projection inschedule.--socket/COMENQ_SOCKEToverride; crates/comenq/src/client.rs performs the full transaction and distinguishes daemon-reported errors from transport failures; crates/comenq/src/output.rs renders ETAs and the sixty-character one-line summaries.client_channel_capacityconfiguration field is removed.Validation
make lint(clippy with warnings denied plus the Whitaker Dylint suite): clean on every commit.make test: 165 unit and integration tests passed; all nine cucumber feature files passed, including the new queue-management feature.make markdownlintandmake nixie: clean (all Mermaid diagrams in the refreshed design document validate).systemd --user;comenq listanswers over the discovered socket.Notes
putis idempotent for an identical request within the same second by construction of the identifier; this is documented onQueueStore::put.immediateprotocol field and the storednot_beforefloor both default off/zero, so entries persisted before the floor existed behave like immediate puts.