Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions skills/unleash/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ An `execute` envelope carries everything you need, so you never fetch the task a
- **`bounced`** — `true` when the task is coming *back* to you rather than arriving
fresh. The program derives it; you never have to notice it yourself. What to do about
it is the next section.
- **`feedback`** — on a bounce, the comments that arrived since the last transition,
already cut for you: **that is the ask**. An empty list means the thread moved but
left nothing past the anchor. The key is *absent* only when the read failed — the one
case where you go read the thread yourself.
- **`pr`** — present only when an earlier turn on this task already delivered. It is
where that work lives: **continue on that branch and update that PR** rather than
opening a second one ([`DELIVERY.md`](DELIVERY.md)).
Expand Down Expand Up @@ -120,13 +124,13 @@ that leaves the claim ref standing and the task reads as held until the lease ex

## What is yours: the judgment

**On `bounced: true`, read the thread before the Goal** (`gh -R OWNER/tasks issue view
<n> --comments`). The task is coming back: any comment I leave puts a held task in the
queue again — from `awaiting-merge` exactly as from `needs-decision` (PROTOCOL.md §6) —
so **the newest feedback is the ask** and the Goal is only background. If it asks for
nothing actionable ("nice, thanks", "merging tomorrow"), do **not** invent rework — run
`then.escalate` saying the delivery still stands and you cannot tell what I want changed.
A question on the thread is the honest answer to a comment nobody can act on.
**On `bounced: true`, read `feedback` before the Goal.** The task is coming back: any
comment I leave puts a held task in the queue again — from `awaiting-merge` exactly as
from `needs-decision` (PROTOCOL.md §6) — so **the newest feedback is the ask** and the
Goal is only background. If it asks for nothing actionable ("nice, thanks", "merging
tomorrow"), or the list came back empty, do **not** invent rework — run `then.escalate`
saying the delivery still stands and you cannot tell what I want changed. A question on
the thread is the honest answer to a comment nobody can act on.

Then, inside an `execute`:

Expand Down
5 changes: 3 additions & 2 deletions skills/unleash/kraken/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,9 @@
)
from .next_action import (
NEXT_ACTIONS, NEXT_ACTION_EXIT, NextAction, NextActionEnvelope,
cmd_next_action, issue_is_finished, lease_block, next_action,
next_action_envelope, resume_verdict, task_brief, then_commands
cmd_next_action, feedback_since, issue_is_finished, lease_block,
next_action, next_action_envelope, resume_verdict, task_brief,
then_commands
)
from .watch import (
WATCH_MAX_FAILURES, WATCH_WARN_EVERY, cmd_watch, snapshot_state,
Expand Down
34 changes: 22 additions & 12 deletions skills/unleash/kraken/claim.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,17 +450,26 @@ def acquire_next(api: Api, project: str, worker: Worker,
one-task-at-a-time guard, §6 reconcile, then guard + CAS down the candidate
list — as DATA rather
than as printed output: returns `(exit_code, won)` where `won` is
`{"issue", "title", "body", "bounced", "pr"}` on EXIT_OK, `{"issue"}` naming
the already-held claim on EXIT_NOT_CLEAR, and None on every other outcome.

`bounced` and `pr` come off the state record this read already carried, and
they are here because the record is READ HERE and nowhere else afterwards:
a first claim writes none (§3.1), so a caller that wanted either would have
to fetch what this function had in hand and threw away. `bounced` is §6's
requeue derivation — two integers, exact — so no reader downstream has to
re-derive "is this task coming back" from the thread, and `pr` is where the
last delivery went, so rework continues on that branch instead of opening a
second PR beside it.
`{"issue", "title", "body", "bounced", "pr", "anchor"}` on EXIT_OK,
`{"issue"}` naming the already-held claim on EXIT_NOT_CLEAR, and None on
every other outcome.

`bounced`, `pr` and `anchor` come off the state record this read already
carried, and they are here because the record is READ HERE and nowhere else
afterwards: a first claim writes none (§3.1), so a caller that wanted any of
them would have to fetch what this function had in hand and threw away.
`bounced` is §6's requeue derivation — two integers, exact — so no reader
downstream has to re-derive "is this task coming back" from the thread, and
`pr` is where the last delivery went, so rework continues on that branch
instead of opening a second PR beside it.

`anchor` is the OTHER half of that derivation: the comment total frozen into
the record, which is the index the new comments start at. `bounced` says the
thread moved on; `anchor` says WHERE, and a reader holding both can cut the
feedback out of the thread instead of judging by eye which comments are new.
Carried as the integer rather than as the comments themselves because this
function is the deterministic claim loop and reading a thread is not part of
it — `next-action` pays for that read, and only when `bounced` is true.

`claim-next` and `next-action` are both thin renderings of this, which is
what keeps the deterministic claim loop from drifting between them: there is
Expand Down Expand Up @@ -540,7 +549,8 @@ def acquire_next(api: Api, project: str, worker: Worker,
return (EXIT_OK, {"issue": cand.number, "title": cand.title,
"body": cand.body,
"bounced": record.requeued(cand.task.comment_total),
"pr": record.pr})
"pr": record.pr,
"anchor": record.comments})
if rc == EXIT_TRANSPORT:
# State is now ambiguous — do NOT move on to another candidate while
# a write of ours may have half-landed. Re-check before any retry.
Expand Down
91 changes: 79 additions & 12 deletions skills/unleash/kraken/next_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
import os

from .contract import (
ClaimRecord, ENTRYPOINT, EXIT_LOST, EXIT_NONE, EXIT_NOT_CLEAR, EXIT_OK,
EXIT_TRANSPORT, EXIT_UNKNOWN_PROJECT, Envelope, Epoch, Gen, Issue, Json,
Repo, Worker, diag, diagnostics_on_stderr
ClaimRecord, CommentRecord, ENTRYPOINT, EXIT_LOST, EXIT_NONE,
EXIT_NOT_CLEAR, EXIT_OK, EXIT_TRANSPORT, EXIT_UNKNOWN_PROJECT, Envelope,
Epoch, Gen, Issue, Json, Repo, Worker, diag, diagnostics_on_stderr
)
from .comments import parse_marker
from .transport import Api, comment_total_of
from .lease import (
Lease, UNREADABLE_LEASE, clear_claim_state, format_iso,
Expand Down Expand Up @@ -107,6 +108,38 @@ def lease_block(
}


def feedback_since(api: Api, issue: Issue, anchor: int,
) -> list[CommentRecord] | None:
"""The comments that arrived AFTER the state record was written — §6's
requeue derivation as the words that caused it, not as a boolean the agent
then goes back to the thread to interpret. Returns the records in server
order, or None on transport failure.

Two steps, and they answer different questions. WHERE the new comments start
is positional: the anchor is a comment TOTAL and `comment_records` returns
the whole thread in that same server order, so the tail past that index is
the range §6 derived its verdict from — every comment counts there (§6), so
no filter may run before the cut without sliding it.

WHAT of that range is the ask is a different question, and the machine's own
comments are not it. A marker is exactly the structural signal that says
"worker-authored" (§4), and by the time this runs the claim's own comment is
already on the thread — so an unfiltered tail would hand every bounced task
back its own "Claimed this task" line as though the operator had written it.
Filtering AFTER the cut costs the range nothing: the index is already fixed.

A thread that SHRANK below its anchor yields an empty list rather than a
slice from the end — which is honest, and is the c13 case: the anchor is
stale, §6's re-anchor repair is what fixes it, and a reader must not invent
feedback to fill the gap. Empty is still an answer, and the caller reports
it as one; None is the absence that means "the read did not land"."""
records = api.comment_records(issue)
if records is None:
return None
return [c for c in records[max(0, int(anchor)):]
if parse_marker(c.get("body") or "") is None]


def task_brief(title: str, body: str) -> Json:
"""The task as the agent needs it: the issue-form sections split out, plus
the raw body for a hand-written issue that carries no headings. An empty or
Expand Down Expand Up @@ -152,21 +185,32 @@ def build(self, action: str, *,
resumed: bool | None = None,
bounced: bool | None = None,
pr: str | None = None,
feedback: list[CommentRecord] | None = None,
brief: Json | None = None,
lease: Json | None = None,
reason: str | None = None,
detail: str | None = None,
holding: Json | None = None) -> Envelope:
"""The one JSON shape next-action emits.

`bounced` and `pr` are what the state record knows about this task's
PAST, carried because the program already computed both and the agent
would otherwise have to rediscover them from the thread — `bounced` by
reading comments and judging whose they are, `pr` by hunting the earlier
delivery in the body. `bounced` rides every execute, false included: "no,
this is a fresh task" is an answer, and an absent key would read as an
older program that could not tell. `pr` is omitted when there is none,
the same rule the markers follow — no delivery is not an empty delivery.
`bounced`, `pr` and `feedback` are what the state record knows about this
task's PAST, carried because the program already computed them and the
agent would otherwise have to rediscover them from the thread —
`bounced` by reading comments and judging whose they are, `pr` by hunting
the earlier delivery in the body. `bounced` rides every execute, false
included: "no, this is a fresh task" is an answer, and an absent key
would read as an older program that could not tell. `pr` is omitted when
there is none, the same rule the markers follow — no delivery is not an
empty delivery.

`feedback` is what `bounced` is ABOUT: the comments past the record's
anchor, so the ask arrives with the verdict instead of costing a fetch
and an eyeball judgment of which comments are new. It rides a bounced
execute only — a fresh task has no thread to cut — and its two absences
are distinct, which is why it is omitted rather than emitted empty on
failure: `[]` says the read landed and found nothing past the anchor (a
stale anchor, §6's repair case), while an ABSENT key says the read did
not land and the agent owes the thread a look of its own.

`holding` is `{"repo", "issue"}` naming a claim this worker must resolve,
and it exists because a `blocked` claim may live in a **different repo**
Expand All @@ -188,6 +232,8 @@ def build(self, action: str, *,
env["bounced"] = bool(bounced)
if pr:
env["pr"] = pr
if feedback is not None:
env["feedback"] = list(feedback)
if reason:
env["reason"] = reason
if detail:
Expand Down Expand Up @@ -445,13 +491,32 @@ def _resumed(self, issue: Issue, detail: Json, issue_obj: Json,
# and still carries the anchor the live total ran past. Every resume of
# this task therefore reports the same `bounced` its acquisition did —
# which is what the agent needs, since it is rework for its whole turn.
# And because the anchor is the same, so is the cut: a resumed bounce
# carries the same feedback the acquisition did, rather than a shorter
# tail that would silently drop what the agent is meant to answer.
bounced = state.requeued(comment_total_of(issue_obj))
return self.envelope.answer(
"execute", issue=issue, resumed=True,
bounced=state.requeued(comment_total_of(issue_obj)), pr=state.pr,
bounced=bounced, pr=state.pr,
feedback=self._feedback(issue, bounced, state.comments),
brief=task_brief(issue_obj.get("title") or "",
issue_obj.get("body") or ""),
lease=lease)

def _feedback(self, issue: Issue, bounced: bool,
anchor: int) -> list[CommentRecord] | None:
"""The comments this bounce is about, or None when there is nothing to
cut or the cut could not be read.

Gated on `bounced` so the common case — a fresh task, no thread — pays
nothing: this is the one read `next-action` adds beyond what the claim
loop already did, and it is owed only when the program has just told the
agent the thread moved on. A failed read degrades to the older behaviour
(the agent goes to the thread itself), never to a wrong cut."""
if not bounced:
return None
return feedback_since(self.api, issue, anchor)

# --- acquiring the next task ---------------------------------------------

def acquire(self) -> tuple[int, Envelope]:
Expand All @@ -462,6 +527,8 @@ def acquire(self) -> tuple[int, Envelope]:
return self.envelope.answer(
"execute", issue=won["issue"], resumed=False,
bounced=won["bounced"], pr=won["pr"],
feedback=self._feedback(won["issue"], won["bounced"],
won["anchor"]),
brief=task_brief(won["title"], won["body"]),
lease=lease_block(self.now, self.now, self.ttl,
source="estimated"))
Expand Down
10 changes: 10 additions & 0 deletions skills/unleash/kraken/render.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ def render_next_action(env: Envelope) -> None:
print(f" {env['detail']}")
if env.get("pr"):
print(f" pr: {env['pr']} (continue on this branch — do not open a second)")
# Only on a bounce, and as a COUNT: the bodies are the JSON's business, and
# a console line that dumped a thread would bury the verdict it sits under.
# An absent key on a bounced envelope is worth saying out loud — it is the
# one case where the agent still owes the thread a read of its own.
if env.get("bounced"):
feedback = env.get("feedback")
if feedback is None:
print(" feedback: unread — check the thread yourself")
else:
print(f" feedback: {len(feedback)} comment(s) past the anchor")
brief = env.get("brief")
if brief:
print(f" title: {brief['title']}")
Expand Down
53 changes: 52 additions & 1 deletion skills/unleash/kraken/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from .refs import Refs
from .queue import is_empty_section, section_body
from .render import render_init
from .state import States

# --- subcommand: init --------------------------------------------------------
# The bootstrap `init`, single-sourced here so the skill and the program never
Expand Down Expand Up @@ -204,7 +205,9 @@ def cmd_validate(args: argparse.Namespace) -> int:
a compliant task gets none (no noise on the happy path, and the same exit
once the operator fixes what was flagged). Debounced: a re-run whose missing
set is unchanged posts no duplicate. Informs only — never holds, closes, or
relabels. Exit 0 on a clean run, 20 on gh/transport failure."""
relabels; the one ref it writes is the §3.1 anchor refresh its own comment
owes (`_refresh_anchor`), which changes no state. Exit 0 on a clean run, 20
on gh/transport failure."""
api, issue = args.api, args.issue

labels = api.issue_label_names(issue)
Expand Down Expand Up @@ -248,10 +251,58 @@ def cmd_validate(args: argparse.Namespace) -> int:
if not api.post_comment(issue, body_to_post):
print(f"validate: gh-failure stage=comment issue={issue}", file=sys.stderr)
return EXIT_TRANSPORT

rc = _refresh_anchor(api, issue)
if rc != EXIT_OK:
return rc
print(f"validate: #{issue} flagged (missing: project/Goal/Acceptance as listed)")
return EXIT_OK


def _refresh_anchor(api, issue) -> int:
"""Move the state record's comment anchor past the comment this pass just
posted — the PROTOCOL.md §3.1 SHOULD that any program-authored comment on a
task's thread owes.

Without it the validator's own comment is indistinguishable from an
operator's reply: §6 derives a requeue from `total > record.comments` and
counts every comment, so a held task the validator touches reads as bounced.
The next worker then claims rework nobody asked for, finds an automated
nag on the thread, and — correctly, per the skill — escalates it back as a
question. The noise costs the operator a `needs-decision` entry; the fix is
to stop generating it, since the count is arithmetic the program has.

`re_anchored` is exactly the right write and no more: the anchor moves, the
hold does not lift, the state, worker, expiry count and PR all stand. Only a
task that HAS a record is touched — an absent record reads as queued (§3.1)
and holds nothing, so there is no anchor to move and writing one would
invent state this pass has no business creating.

A failed read or write is reported as transport rather than swallowed: the
comment has landed by now, so a silent failure leaves precisely the stale
anchor this exists to prevent, and the run should say so.

The record is read BEFORE the count, which is the cheap order for this
caller: the validator fires on new and edited queue entries, and a task that
has never been put down has no record at all — the overwhelmingly common
case exits here having paid one ref read, never the issue fetch as well."""
states = States(api)
record = states.of(issue)
if record.unknown:
print(f"validate: gh-failure stage=record issue={issue}", file=sys.stderr)
return EXIT_TRANSPORT
if not record.recorded:
return EXIT_OK
total = api.comment_count(issue)
if total is None:
print(f"validate: gh-failure stage=anchor issue={issue}", file=sys.stderr)
return EXIT_TRANSPORT
if not states.write(issue, record.re_anchored(total)):
print(f"validate: gh-failure stage=record issue={issue}", file=sys.stderr)
return EXIT_TRANSPORT
return EXIT_OK


def is_identity_label(name: str) -> bool:
"""A label cleanup MUST preserve on a closed task: the task-type label
(kraken-task) and its project routing label (project:<name>). Everything else
Expand Down
Loading
Loading