Gitea client: conditional issue listing #4

Closed
opened 2026-08-07 12:39:01 +00:00 by grenade · 1 comment
Owner

Part of #1.

Goal

Implement tireless_core::port::ForgeClient for GiteaClient, read paths only.
The struct exists with #[allow(dead_code)] fields and no implementation.

The read path is list_opted_in_issues, and it must be a polite API citizen:
send If-None-Match when the repo has a stored ETag, treat 304 Not Modified
as an empty result rather than an error, and back off with jitter on 429 and
5xx. Being unattended is not a licence to hammer a forge (design.md §5).

The token comes from the environment variable named by
config.forge.gitea.token_env — never from the config file, and never the
operator's own token (design.md §6.4).

Files

  • crates/tireless-data/src/forge.rs — the ForgeClient impl for GiteaClient
  • crates/tireless-data/src/lib.rs — exports if needed

Steps

  1. Give GiteaClient a constructor taking base URL and token, reading the token
    from the named environment variable at construction and failing loudly if it
    is absent.
  2. Implement list_opted_in_issues: query the repo's open issues filtered by
    the opt-in label, sending If-None-Match when repo.last_etag is set.
  3. Return an empty vec on 304, and surface the new ETag so the caller can
    store it.
  4. Map the Gitea issue payload to DiscoveredIssue, preserving all labels — the
    router needs them, and so does reconciliation.
  5. Implement retry with exponential backoff and jitter on 429/5xx, bounded;
    a forge that is down should not be retried forever inside one poll.
  6. Leave the write methods (add_label, remove_label, comment,
    create_issue, open_pull_request) returning a clear "not implemented in
    stage 1" error rather than silently succeeding.

Acceptance

  • cargo test --workspace
  • cargo clippy --all-targets --all-features -- -D warnings
  • A test against a mock HTTP server shows If-None-Match is sent when an ETag
    is stored, and that a 304 response yields Ok(vec![]).
  • A test shows a 429 is retried with backoff and eventually gives up rather
    than looping.

Out of scope

  • Every forge write. Stage 2 turns those on, deliberately, after the claim
    protocol has been exercised with a dry-run executor.
  • GitHubClient. It is for legacy repos and is disabled in the shipped config;
    leave it as it is.
  • The poll loop and scheduling — a separate child.
  • Do not store the token in Config. It is named there, not carried there.
Part of #1. ## Goal Implement `tireless_core::port::ForgeClient` for `GiteaClient`, read paths only. The struct exists with `#[allow(dead_code)]` fields and no implementation. The read path is `list_opted_in_issues`, and it must be a polite API citizen: send `If-None-Match` when the repo has a stored `ETag`, treat `304 Not Modified` as an empty result rather than an error, and back off with jitter on `429` and `5xx`. Being unattended is not a licence to hammer a forge (design.md §5). The token comes from the environment variable named by `config.forge.gitea.token_env` — never from the config file, and never the operator's own token (design.md §6.4). ## Files - `crates/tireless-data/src/forge.rs` — the `ForgeClient` impl for `GiteaClient` - `crates/tireless-data/src/lib.rs` — exports if needed ## Steps 1. Give `GiteaClient` a constructor taking base URL and token, reading the token from the named environment variable at construction and failing loudly if it is absent. 2. Implement `list_opted_in_issues`: query the repo's open issues filtered by the opt-in label, sending `If-None-Match` when `repo.last_etag` is set. 3. Return an empty vec on `304`, and surface the new `ETag` so the caller can store it. 4. Map the Gitea issue payload to `DiscoveredIssue`, preserving all labels — the router needs them, and so does reconciliation. 5. Implement retry with exponential backoff and jitter on `429`/`5xx`, bounded; a forge that is down should not be retried forever inside one poll. 6. Leave the write methods (`add_label`, `remove_label`, `comment`, `create_issue`, `open_pull_request`) returning a clear "not implemented in stage 1" error rather than silently succeeding. ## Acceptance - `cargo test --workspace` - `cargo clippy --all-targets --all-features -- -D warnings` - A test against a mock HTTP server shows `If-None-Match` is sent when an ETag is stored, and that a `304` response yields `Ok(vec![])`. - A test shows a `429` is retried with backoff and eventually gives up rather than looping. ## Out of scope - Every forge *write*. Stage 2 turns those on, deliberately, after the claim protocol has been exercised with a dry-run executor. - `GitHubClient`. It is for legacy repos and is disabled in the shipped config; leave it as it is. - The poll loop and scheduling — a separate child. - Do not store the token in `Config`. It is named there, not carried there.
grenade added the tireless/implement label 2026-08-07 12:40:48 +00:00
Author
Owner

Done in 98f193d, together with #3.

The spec gap

Step 3 says "surface the new ETag so the caller can store it" — but
list_opted_in_issues returned a bare Vec<DiscoveredIssue>, so there was
nowhere to put it. The only copy would have stayed inside the client, and a
poller that looked correct would have re-fetched every repo in full on every
tick, forever, with nothing to show it was happening.

It returns an IssuePage now, which also carries not_modified. That
distinction turned out to matter more than the ETag: a 304 is not an empty
repo
. A caller that conflated them would read every quiet poll as "every issue
disappeared" and abandon the jobs behind them. Carrying it in the type means a
caller has to decide rather than assume, and there is a test pinning it.

Beyond the spec

Pull requests are filtered out. Gitea's type=issues parameter should
exclude them, but an older server ignores it and returns both — and a pull
request enqueued as an issue would be planned or implemented as though it were
one. Cheap to guard, expensive to debug.

Retry-After is honoured but capped at 60s. A forge asking us to wait an
hour would otherwise stall the whole poll tick for every other repo.

Backoff is jittered. N repos throttled at the same moment must not all retry
at the same moment and throttle each other again.

Writes return an error naming the stage they land in, rather than a silent
Ok. A no-op that succeeded would let stage 2 look finished while the forge saw
nothing — and there is a test asserting they do not even reach the network.

Verified against a mock forge

Twelve tests, no database needed, so these run in the ordinary cargo test:

  • a stored ETag is sent as If-None-Match, and 304 is reported as
    not-modified rather than as empty or as an error;
  • a first poll sends no conditional header — a stale or empty If-None-Match
    could earn a 304 for a client that has never seen the issues;
  • the returned ETag is surfaced for the next poll;
  • 429 is retried and then given up on, with the attempt budget asserted — an
    unbounded retry would hold the tick open indefinitely;
  • 503 is retried and a later success is returned;
  • 404 is not retried, because it will not fix itself;
  • pull requests are filtered out;
  • labels and body survive the mapping, since the router reads one and the
    planner reads the other;
  • every unimplemented write fails without touching the forge.

GitHubClient is deliberately still a stub. It is disabled in the shipped
config, and an unused implementation is one more thing to keep working for
nobody.

Done in `98f193d`, together with #3. ## The spec gap Step 3 says "surface the new `ETag` so the caller can store it" — but `list_opted_in_issues` returned a bare `Vec<DiscoveredIssue>`, so there was nowhere to put it. The only copy would have stayed inside the client, and a poller that looked correct would have re-fetched every repo in full on every tick, forever, with nothing to show it was happening. It returns an `IssuePage` now, which also carries `not_modified`. That distinction turned out to matter more than the ETag: **a 304 is not an empty repo**. A caller that conflated them would read every quiet poll as "every issue disappeared" and abandon the jobs behind them. Carrying it in the type means a caller has to decide rather than assume, and there is a test pinning it. ## Beyond the spec **Pull requests are filtered out.** Gitea's `type=issues` parameter should exclude them, but an older server ignores it and returns both — and a pull request enqueued as an issue would be planned or implemented as though it were one. Cheap to guard, expensive to debug. **`Retry-After` is honoured but capped** at 60s. A forge asking us to wait an hour would otherwise stall the whole poll tick for every other repo. **Backoff is jittered.** N repos throttled at the same moment must not all retry at the same moment and throttle each other again. **Writes return an error naming the stage they land in**, rather than a silent `Ok`. A no-op that succeeded would let stage 2 look finished while the forge saw nothing — and there is a test asserting they do not even reach the network. ## Verified against a mock forge Twelve tests, no database needed, so these run in the ordinary `cargo test`: - a stored ETag is sent as `If-None-Match`, and 304 is reported as not-modified rather than as empty or as an error; - a first poll sends no conditional header — a stale or empty `If-None-Match` could earn a 304 for a client that has never seen the issues; - the returned ETag is surfaced for the next poll; - 429 is retried and then given up on, with the attempt budget asserted — an unbounded retry would hold the tick open indefinitely; - 503 is retried and a later success is returned; - 404 is *not* retried, because it will not fix itself; - pull requests are filtered out; - labels and body survive the mapping, since the router reads one and the planner reads the other; - every unimplemented write fails without touching the forge. `GitHubClient` is deliberately still a stub. It is disabled in the shipped config, and an unused implementation is one more thing to keep working for nobody.
Sign in to join this conversation.