make fj_write.py report issue creation and stop routing label writes through fj #29

Merged
jerome merged 6 commits from fix/fj-write-create-and-label into main 2026-08-09 19:19:35 +03:00
Collaborator

Goal: Make fj_write.py honour its "never a silent success" guarantee on issue-create, and make pr-label / issue-label work regardless of the installed fj version.

Closes: #27

Value: high
Effort: 2h


Why

Two defects reported in #27, both hit while creating and labelling a single
issue.

Silent create. cmd_create()'s issue branch runs fj issue create and
prints its stdout only if out:. fj issue create emits nothing, so the
command exits 0 having printed zero bytes -- the caller cannot tell success
from failure and never learns the new issue's number. That directly contradicts
the module docstring ("never silently swallows a failure") and SKILL.md
("prints the resulting state on success, so you act on stdout directly"). The
practical cost in #27 was a whole fj-worker dispatch just to answer "did that
land, and what number is it?", plus the standing duplicate-filing risk when a
caller assumes failure and retries. cmd_create is also the only handler that
never calls show_state() -- it has no number to re-read with.

Labels depend on the fj build. cmd_label() builds
fj <group> edit <N> labels -a <L>. On the fj inside the dispatch container
that argv fails outright:

error: unexpected argument 'labels' found
Usage: fj issue edit [OPTIONS] <ISSUE> <COMMAND>

so every issue-label call exits 5 and escalates to fj-worker, which cannot
fix it either -- the capability is missing from fj, not mis-invoked. Note that
this does not reproduce on every fj: the build on the mac host does
expose fj issue edit <N> labels [-a ADD] [-r RM] and fj pr edit [PR] labels,
so the same script silently works in one environment and hard-fails in another.
That version-sensitivity is the real defect, and it is why the fix is to stop
going through fj for labels at all rather than to special-case a version
probe. Forgejo's REST labels endpoints are stable and already the established
pattern here -- cmd_review() bypasses fj for exactly this reason.


Steps

  • Add a api_post-sibling REST helper for the calls the labels endpoints need -- a JSON GET for the repo label list and a DELETE that tolerates an empty 204 body -- reusing fj_read's token/base plumbing and api_post's error scheme (5 on non-2xx, 4 on network).
  • Rewrite cmd_label() to resolve label names to IDs against the repo's label list, then POST /repos/{repo}/issues/{number}/labels with the add IDs and DELETE /repos/{repo}/issues/{number}/labels/{id} per removal. The endpoint is shared by issues and PRs, so the pr-/issue- split collapses to one code path.
  • Fail with exit 2 and the offending name when a requested label does not exist on the repo, instead of the current silent no-op -- an unknown name is a caller typo and must not look like success.
  • Keep --dry-run meaningful for cmd_label(): print the resolved endpoint(s) and label IDs without issuing a request.
  • Rewrite the issue branch of cmd_create() to POST /repos/{repo}/issues (returns the created issue JSON including its number), then call show_state() on that number so create prints the same block every other handler prints.
  • Make the PR branch of cmd_create() print resulting state too: parse the PR number out of fj pr create's stdout and call show_state(); if the number cannot be parsed, say so on stderr rather than exiting silent.
  • Extend tests/plugins/forgejo-cli/skills/forgejo-cli/scripts/test_fj_write.py with unit coverage for the new pure logic -- name-to-ID resolution (hit, miss, case handling) and the PR-number parse -- keeping the stdlib-unittest-under-pytest style already in the file. No network in tests.
  • Update plugins/forgejo-cli/skills/forgejo-cli/SKILL.md where it describes the write surface: note that label writes and issue creation go through REST (like pr-review), and that unknown label names now fail loudly.

Notes

Endpoints. Add is additive and takes IDs, not names:

GET    /repos/{owner}/{repo}/labels                      # name -> id map
POST   /repos/{owner}/{repo}/issues/{index}/labels       # {"labels": [id, ...]}
DELETE /repos/{owner}/{repo}/issues/{index}/labels/{id}  # 204, empty body

Forgejo treats a PR as an issue for label purposes, so /issues/{index}/labels
is correct for both -- which is what lets the two handlers merge.

Name-to-ID lookup. fj_read.py --list-labels already pages the label list;
reuse that gather rather than adding a second pager. Org-level labels can appear
alongside repo labels -- include both in the lookup, and if a name is ambiguous
across the two scopes, prefer the repo-level one and say so.

Why not just require a newer fj. Pinning an fj version would fix the
container and leave the script silently version-coupled for the next capability
gap. The script's stated design is already hybrid: shell out to fj where it is
battle-tested, REST where fj has a gap. Labels have now demonstrably been on
both sides of that line, so they belong in the REST half.

Rejected: keep fj and probe for the subcommand. A --help probe per call
costs a subprocess and still leaves two divergent code paths to keep correct.

Out of scope. The issue's third section (the fj-worker subagent having no
network access, and its printing the live token to stdout when blocked) is a
separate concern in plugins/forgejo-cli/agents/fj-worker.md and its sandbox
config, not in fj_write.py. It should get its own PR -- the token-echo half is
worth doing promptly.

Repo note. public/chiche-plugins has no task:active / task:blocked
labels, so the implement/review passes cannot set them here. One-time setup via
the web UI if that signalling is wanted; not a blocker.

**Goal:** Make `fj_write.py` honour its "never a silent success" guarantee on `issue-create`, and make `pr-label` / `issue-label` work regardless of the installed `fj` version. **Closes:** #27 **Value:** high **Effort:** 2h --- ## Why Two defects reported in #27, both hit while creating and labelling a single issue. **Silent create.** `cmd_create()`'s issue branch runs `fj issue create` and prints its stdout only `if out:`. `fj issue create` emits nothing, so the command exits `0` having printed zero bytes -- the caller cannot tell success from failure and never learns the new issue's number. That directly contradicts the module docstring ("never silently swallows a failure") and `SKILL.md` ("prints the resulting state on success, so you act on stdout directly"). The practical cost in #27 was a whole `fj-worker` dispatch just to answer "did that land, and what number is it?", plus the standing duplicate-filing risk when a caller assumes failure and retries. `cmd_create` is also the only handler that never calls `show_state()` -- it has no number to re-read with. **Labels depend on the `fj` build.** `cmd_label()` builds `fj <group> edit <N> labels -a <L>`. On the `fj` inside the dispatch container that argv fails outright: ``` error: unexpected argument 'labels' found Usage: fj issue edit [OPTIONS] <ISSUE> <COMMAND> ``` so every `issue-label` call exits `5` and escalates to `fj-worker`, which cannot fix it either -- the capability is missing from `fj`, not mis-invoked. Note that this does **not** reproduce on every `fj`: the build on the mac host *does* expose `fj issue edit <N> labels [-a ADD] [-r RM]` and `fj pr edit [PR] labels`, so the same script silently works in one environment and hard-fails in another. That version-sensitivity is the real defect, and it is why the fix is to stop going through `fj` for labels at all rather than to special-case a version probe. Forgejo's REST labels endpoints are stable and already the established pattern here -- `cmd_review()` bypasses `fj` for exactly this reason. --- ## Steps - [x] Add a `api_post`-sibling REST helper for the calls the labels endpoints need -- a JSON `GET` for the repo label list and a `DELETE` that tolerates an empty 204 body -- reusing `fj_read`'s token/base plumbing and `api_post`'s error scheme (`5` on non-2xx, `4` on network). - [x] Rewrite `cmd_label()` to resolve label names to IDs against the repo's label list, then `POST /repos/{repo}/issues/{number}/labels` with the add IDs and `DELETE /repos/{repo}/issues/{number}/labels/{id}` per removal. The endpoint is shared by issues and PRs, so the `pr-`/`issue-` split collapses to one code path. - [x] Fail with exit `2` and the offending name when a requested label does not exist on the repo, instead of the current silent no-op -- an unknown name is a caller typo and must not look like success. - [x] Keep `--dry-run` meaningful for `cmd_label()`: print the resolved endpoint(s) and label IDs without issuing a request. - [x] Rewrite the issue branch of `cmd_create()` to `POST /repos/{repo}/issues` (returns the created issue JSON including its `number`), then call `show_state()` on that number so create prints the same block every other handler prints. - [x] Make the PR branch of `cmd_create()` print resulting state too: parse the PR number out of `fj pr create`'s stdout and call `show_state()`; if the number cannot be parsed, say so on stderr rather than exiting silent. - [x] Extend `tests/plugins/forgejo-cli/skills/forgejo-cli/scripts/test_fj_write.py` with unit coverage for the new pure logic -- name-to-ID resolution (hit, miss, case handling) and the PR-number parse -- keeping the stdlib-`unittest`-under-pytest style already in the file. No network in tests. - [x] Update `plugins/forgejo-cli/skills/forgejo-cli/SKILL.md` where it describes the write surface: note that label writes and issue creation go through REST (like `pr-review`), and that unknown label names now fail loudly. --- ## Notes **Endpoints.** Add is additive and takes IDs, not names: ``` GET /repos/{owner}/{repo}/labels # name -> id map POST /repos/{owner}/{repo}/issues/{index}/labels # {"labels": [id, ...]} DELETE /repos/{owner}/{repo}/issues/{index}/labels/{id} # 204, empty body ``` Forgejo treats a PR as an issue for label purposes, so `/issues/{index}/labels` is correct for both -- which is what lets the two handlers merge. **Name-to-ID lookup.** `fj_read.py --list-labels` already pages the label list; reuse that gather rather than adding a second pager. Org-level labels can appear alongside repo labels -- include both in the lookup, and if a name is ambiguous across the two scopes, prefer the repo-level one and say so. **Why not just require a newer `fj`.** Pinning an `fj` version would fix the container and leave the script silently version-coupled for the next capability gap. The script's stated design is already hybrid: shell out to `fj` where it is battle-tested, REST where `fj` has a gap. Labels have now demonstrably been on both sides of that line, so they belong in the REST half. **Rejected: keep `fj` and probe for the subcommand.** A `--help` probe per call costs a subprocess and still leaves two divergent code paths to keep correct. **Out of scope.** The issue's third section (the `fj-worker` subagent having no network access, and its printing the live token to stdout when blocked) is a separate concern in `plugins/forgejo-cli/agents/fj-worker.md` and its sandbox config, not in `fj_write.py`. It should get its own PR -- the token-echo half is worth doing promptly. **Repo note.** `public/chiche-plugins` has no `task:active` / `task:blocked` labels, so the implement/review passes cannot set them here. One-time setup via the web UI if that signalling is wanted; not a blocker.
fix: scaffold fj_write create/label fixes
All checks were successful
gitleaks / gitleaks (pull_request) Successful in 3s
70ea3baf17
Author
Collaborator

Resolver started. Worktree .worktrees/fj-write-create-and-label.

Note: this repo has no task:active / task:blocked labels, so the pass cannot signal itself with one (one-time setup via the web UI if wanted).

session: 57102610-75e1-40c1-97c4-154145a98467

Resolver started. Worktree `.worktrees/fj-write-create-and-label`. Note: this repo has no `task:active` / `task:blocked` labels, so the pass cannot signal itself with one (one-time setup via the web UI if wanted). session: 57102610-75e1-40c1-97c4-154145a98467
feat: add a REST DELETE helper and carry label ids through the read gather
All checks were successful
gitleaks / gitleaks (pull_request) Successful in 3s
64f46d2eea
fix: route label writes through the REST API instead of fj
All checks were successful
gitleaks / gitleaks (pull_request) Successful in 3s
96a4c09fa6
fix: make create print the resulting state instead of exiting silent
All checks were successful
gitleaks / gitleaks (pull_request) Successful in 3s
c4086dfe7b
test: cover label name-to-id resolution and the created-PR number parse
All checks were successful
gitleaks / gitleaks (pull_request) Successful in 3s
b985ec8d1d
docs: describe the REST label and issue-create paths in the skill
All checks were successful
gitleaks / gitleaks (pull_request) Successful in 3s
12057be2f2
Author
Collaborator

2026-08-09 -- resolver pass on fix/fj-write-create-and-label at 12057be.

  • Steps done: all 8 (REST DELETE helper + label ids on the read gather; cmd_label() rewritten onto the labels endpoint; unknown names now exit 2; --dry-run prints the resolved endpoint + ids; issue-create via REST POST + show_state(); pr-create parses its number and re-reads; unit tests for resolve_label_ids() and parse_created_number(); SKILL.md + module docstring updated).
  • Deferred to /task-orchestrate: 2 -- uv run --with pytest pytest tests -q and the --dry-run smoke command. No test or install was run in this pass.
  • Surprise worth knowing: bug 2 does not reproduce on every fj. The build on the mac host does expose fj issue edit <N> labels and fj pr edit [PR] labels; the container's build does not. That version-sensitivity is folded into the PR body's Why, and it is the argument for going to REST rather than pinning an fj version.
  • resolve_label_ids() also covers org-level labels, with repo scope shadowing an org label of the same name.
  • This repo has no task:active label, so the pass could not signal itself with one.

session: 57102610-75e1-40c1-97c4-154145a98467

2026-08-09 -- resolver pass on `fix/fj-write-create-and-label` at `12057be`. - Steps done: all 8 (REST DELETE helper + label ids on the read gather; `cmd_label()` rewritten onto the labels endpoint; unknown names now exit 2; `--dry-run` prints the resolved endpoint + ids; `issue-create` via REST POST + `show_state()`; `pr-create` parses its number and re-reads; unit tests for `resolve_label_ids()` and `parse_created_number()`; SKILL.md + module docstring updated). - Deferred to /task-orchestrate: 2 -- `uv run --with pytest pytest tests -q` and the `--dry-run` smoke command. No test or install was run in this pass. - Surprise worth knowing: bug 2 does **not** reproduce on every `fj`. The build on the mac host does expose `fj issue edit <N> labels` and `fj pr edit [PR] labels`; the container's build does not. That version-sensitivity is folded into the PR body's Why, and it is the argument for going to REST rather than pinning an `fj` version. - `resolve_label_ids()` also covers org-level labels, with repo scope shadowing an org label of the same name. - This repo has no `task:active` label, so the pass could not signal itself with one. session: 57102610-75e1-40c1-97c4-154145a98467
jerome approved these changes 2026-08-09 19:19:29 +03:00
jerome changed title from WIP: make fj_write.py report issue creation and stop routing label writes through fj to make fj_write.py report issue creation and stop routing label writes through fj 2026-08-09 19:19:31 +03:00
jerome merged commit 8be048a7e5 into main 2026-08-09 19:19:35 +03:00
jerome deleted branch fix/fj-write-create-and-label 2026-08-09 19:19:35 +03:00
Sign in to join this conversation.
No reviewers
No milestone
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
public/chiche-plugins!29
No description provided.