Skip to content

[scanner] fix(contribute): persist task leases so a hub restart doesn't revoke every in-flight task - #5693

Closed
kubestellar-hive[bot] wants to merge 1 commit into
v4from
scanner/fix-5681-lease-persistence
Closed

[scanner] fix(contribute): persist task leases so a hub restart doesn't revoke every in-flight task#5693
kubestellar-hive[bot] wants to merge 1 commit into
v4from
scanner/fix-5681-lease-persistence

Conversation

@kubestellar-hive

Copy link
Copy Markdown
Contributor

Fix

Claims: src/pkg/dashboard/contribute_ws.go (lease registry: recordLease/renewLease/revokeLease/lookupLease, hub constructor), new src/pkg/dashboard/contribute_lease_persist.go + contribute_lease_persist_test.go, changelog.d/fixed-5681-lease-restart-persistence.md. No overlap with open hold-gated PRs (#5673 docs, #5665 labelPlanSink tests, #5642/#5635/#5559 planning docs, #4032 proxy).

The C4 server-issued task-lease registry lived only in hub memory. Any hub restart (routine under self-upgrade rolls) emptied it, so every relay mid-task that reconnected was told no active lease for this task, had its agent interrupted mid-turn, and was re-assigned the identical issue seconds later — exactly the 14:24:40 revoke → 14:24:44 re-assign sequence in the issue.

This persists the registry to task-leases.json in the PVC-backed contributors dir, following the existing cooldown/streak/verdict ledger pattern:

  • saveLeases() — atomic tmp+rename write on every recordLease / effective renewLease / effective revokeLease; expired entries skipped so the ledger self-cleans.
  • loadLeases() — at hub construction; drops expired/partial/corrupt records.
  • Honours HIVE_CONTRIBUTORS_DIR and the persistTaskLedgers gate like the other ledgers.

Security posture unchanged: a restored lease is byte-for-byte the {identity, task, repo, number, tier, generation, expiry} tuple the hub issued — the C4 exact-match contract and the #2568 generation fence apply to it unchanged, a lease revoked before the restart is absent and stays absent, and expiry still runs on the #4260 renewal clock.

Tests: TestLeasePersist_ResumeSurvivesHubRestart, _RevokedLeaseStaysRevokedAcrossRestart, _ExpiredLeaseNotRestored, _RenewalExtendsWindowAcrossRestart, _MalformedAndPartialRecordsSkipped; full -run 'Lease|C4|Contribute' suite passes.

Fixes #5681


Filed by scanner agent (ACMM L5 — hold-gated mode). Hold-gated: human review required.

— hive: agent=scanner backend=copilot model=claude-fable-5 copilot=1.0.78

…'t revoke every in-flight task

Leases (C4 server-issued task ownership records) lived only in hub
memory. A hub restart emptied the registry, so every reconnecting
relay's resume failed lookupLease, got 'no active lease for this
task', had its agent interrupted mid-turn, and was handed the same
issue back seconds later as a fresh task.

Persist the registry to task-leases.json in the PVC-backed
contributors dir (same pattern as the cooldown/streak/verdict
ledgers): saved on every recordLease/renewLease/revokeLease, loaded
at hub construction with expired and malformed entries dropped.

A restored lease is byte-for-byte the tuple the hub issued, so the
C4 exact-match contract and the #2568 generation fence are
unchanged; revoked leases are absent from the ledger and stay
revoked; expiry still runs on the #4260 renewal clock.

Fixes #5681

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: sec-check <sec-check@hive.kubestellar.io>
@kubestellar-hive kubestellar-hive Bot added the hold label Sep 2, 2026
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Sep 2, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign danathar for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubestellar-prow kubestellar-prow Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 2, 2026
@kubestellar-hive kubestellar-hive Bot added agent/scanner Created or modified by the scanner agent hive/hosted-available-oke-11-placeholder-r05x Approved by a Hive merger/owner for auto-merge on green CI labels Sep 2, 2026
@clubanderson

Copy link
Copy Markdown
Member

Review — closing in favor of #5688, which fixes the same #5681 lease-volatility bug (opened first, by the issue's reporter) and goes further on exactly the edges this registry needs:

Unique value worth carrying forward: the round-trip unit tests here are solid, and this PR targets v4 while #5688 targets v5 — v4 still needs a faithful port of #5688 (including the taskGen re-derivation and a changelog.d fragment in this style). Noting that on the survivor. Thanks — the diagnosis and ledger-pattern instincts were right; the survivor simply covers more of the coupled invariants.

clubanderson pushed a commit that referenced this pull request Sep 3, 2026
A contributor relay works one issue at a time out of a single persistent
checkout, and nothing resets that checkout between tasks. The task prompt
told the agent to fork, clone, commit, push and open a PR, but mentioned a
branch exactly once — "push your branch to your fork remote" — and never
said which branch to start from or target. The base was therefore whatever
the previous task happened to leave checked out.

With two active branches that is enough for one branch-specific issue to
redirect every later PR of a session. On 2026-09-02 issue #5617, titled
"[v5] reviewer lane follow-ups …", correctly put the checkout on v5; the
four PRs after it (#5688, #5700, #5705, #5711) inherited v5, and three of
them were fixes for defects live on the deployed v4. Branch ancestry
confirms inheritance rather than choice — each is 1–2 commits ahead of v5
and 64–67 ahead of v4, with no base_ref_changed event on any of them. The
cost was real: #5688 and the scanner's independent #5693 fixed the same
defect on two branches, a maintainer had to adjudicate between them, and
the v4 fixes were backported by hand hours later.

The failure is invisible from every seat. The agent has nothing to check
against, the contributor sees PRs opening and merging normally, and a
maintainer sees correctly-formed PRs on a plausible branch.

buildTaskPromptBody now names the base and tells the agent to start its
work branch from it ('git checkout -b <branch> upstream/<base>'), open the
PR with 'gh pr create --base <base>', and confirm the PR's base before
reporting done. The base comes from taskBaseBranch: the branch this hive
was built from — the same upstreamBranch() the onboarding page's clone
command already names (#3990), so the two answers agree — unless the issue
title carries a release-line tag such as "[v5]", which wins. The tag shape
is narrow on purpose ('v' plus digits, the shape image_pulls.go matches and
.github/release-lines.yml lists), so the lane prefixes the classifier
routes on ("[quality]", "[architect]") cannot be read as branches.

The prompt is the load-bearing half. Fixing only the workspace was measured
and found insufficient: a working branch reset from v5 onto v4 mid-task,
holding zero commits and a clean tree, was restored to v5 by the agent,
because the plan it had already formed said v5. An agent follows what it
was told over what it finds, so the instruction has to carry the answer.

When no base resolves at all, the prompt still refuses inheritance and
names the substitute — the upstream repository's own default branch —
rather than falling silent.

Not included, and deliberately: resetting the workspace to the base before
each task. bin/contributor-relay.sh performs no git operations today (it
sets a cwd and types a prompt), so that defence in depth means a new
protocol field plus a 'git reset --hard' against a contributor's persistent
checkout — a destructive new surface that deserves its own change and its
own observation, not a rider on this one.

Regression coverage in contribute_task_base_branch_test.go asserts the
prompt names a base, that a task following a branch-specific one is told
this hive's branch rather than the previous task's, that the base is
derived rather than re-hardcoded, that an uninjected build branch falls
back instead of emitting "unknown", that an unresolvable base still forbids
inheritance, and that lane prefixes are not mistaken for release lines.

Fixes #5729

Signed-off-by: Danathar <doug.baggett@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent/scanner Created or modified by the scanner agent copilot dco-signoff: yes Indicates the PR's author has signed the DCO. hive/hosted-available-oke-11-placeholder-r05x Approved by a Hive merger/owner for auto-merge on green CI hold size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 bug: a hub restart makes every in-flight contributor task unresumable — the agent is interrupted, then handed the same issue back seconds later

1 participant