HOMELAB-1734: refactor(epic-session): generalize extension for all projects #44
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "plane/HOMELAB-1734-epic-session-generic"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
Rewrites
/epic-sessionso it works in any project instead of silently applying sendrast defaults everywhere. Observed live: running it in~/repos/homelabclaimed a sendrast slot via a hardcoded fallback path (/home/aaron/repos/sendrast/api-sendrast/scripts/dev-slot.sh) and used/tmp/sendrast-slots,sendrast-s<N>, and hardcoded sendrast locks.Slot backends (auto-selected)
scripts/dev-slot.sh(sendrast). The hardcoded absolute fallback path is removed; script candidates are--script/EPIC_SESSION_SLOT_SCRIPT→ configslotScript→<cwd>/scripts→ project root → each repo → git-object extraction with configurableslotScriptGitRefs.slotsDir(same schema as dev-slot.sh), env block generated from config port-block math.Config
~/.cache/epic-session/slots/<project>registry,<project>-scompose prefix (was/tmp/sendrast-slots/sendrast-s).EPIC_SESSION_SLOTS_DIRenv;SENDRAST_SLOTS_DIRkept as legacy alias.portsmap ({NAME: offset}) for the env block; generic default emitsSLOT+COMPOSE_PROJECT_NAMEonly.postCreatehooks (legacyparaglideCommand/webRepokeys still accepted).sharedLocks) and doc references are config-driven, not hardcoded.Verification
start/attach/status/endcycle on a synthetic project with the builtin registry: port math (5500+10·N+offset), post-create hooks, duplicate-claim/slot-0/refusal gates, worktree create/remove onepic/<slug>.statusregression in~/repos/sendrast: script backend resolved fromapi-sendrast/scripts, all six doc derivations intact (worktree layout, port block, slots dir, compose prefix, paraglide hook, teardown checklist).~/repos/homelabresolution with the new machine config: builtin backend,epic/<epic>fromorigin/live.Machine config
~/.config/epic-session/projects.jsonnow carries homelab (repos: infra-core, prod,baseRef: origin/live) and apexDirector entries; lab-director deferred (repos not cloned on this machine).Notes
/epic-session start TESTin homelab with the merged extension) is the ticket's post-merge verify step.LD0) from the session that surfaced this bug will be released separately — tracked on the ticket.Ticket: HOMELAB-1734
Rewrite the /epic-session extension so it works in any project (homelab, lab-director, apexdirector, sendrast) instead of silently applying sendrast defaults everywhere — observed live when a homelab epic claimed a sendrast slot via a hardcoded fallback path. Slot backends: - script: unchanged delegation to a project's scripts/dev-slot.sh (sendrast), with script resolution order explicit/config-driven; the hardcoded /home/aaron/repos/sendrast fallback path is gone and git-object extraction refs are config (slotScriptGitRefs, default origin/main) - builtin: new native claim/release/list/ports registry used whenever no script resolves — atomic claim JSON in the project's slotsDir (same schema as dev-slot.sh), env block generated from config port-block math Config (layered, doc < machine JSON < project JSON < env): - name-derived defaults: slotsDir ~/.cache/epic-session/slots/<project>, compose prefix <project>-s (was /tmp/sendrast-slots / sendrast-s) - EPIC_SESSION_SLOTS_DIR env (SENDRAST_SLOTS_DIR kept as legacy alias) - config-driven ports map ({NAME: offset}) for the env block - paraglide -> generic postCreate hooks (legacy paraglideCommand/webRepo keys still accepted) - sharedLocks config feeds the agent kickoff (was hardcoded sendrast paths) - kickoff references docs/parallel-sessions.md only when it exists Verified: full start/attach/status/end cycle on a synthetic project with the builtin registry (ports math, hooks, refusal gates, worktrees), and a read-only sendrast status regression confirming the script backend and full doc derivation are unchanged.Landing review on
ff46f3a— NOT merged: 1 blocking regression in the builtin registry refresh pathThree fresh-context review rounds ran on this PR (primary sources only: the diff, the ticket acceptance criteria,
dev-slot.shas the reference registry, the repo docs). The first three blockers found in the new builtin slot registry are genuinely fixed onff46f3a, but that fix introduced a new claim-loss regression, reproduced independently by the reviewer and by the landing worker (3/3 trials). Merging is on hold until it is resolved.Blocker — own-slot refresh deletes the claim before the replacement exists (
epic-session/slot.ts:494-502)Between the unlink and the successful
linkSyncthe epic holds no claim and the slot looks free. Consequences:index.tsalways passes an explicit slot for a live epic (start:slot: requestedSlot ?? existing?.slot), so when a concurrent auto claim takes the slot in that gap the explicit branch (slot.ts:549-551) returnsfail("slot N is held by epic '<other>' — pick another slot")— after the epic's own file was deleted. Harness (8-slot registry, every other slot busy, epic-a refreshing slot 3 while another session runs a plain auto claim): The epic's session keeps running on the old slot's port block while the registry has handed that slot to another epic — the exact collision the registry exists to prevent, andendcan no longer release it.writeFileSync(tmp, …)sits outsidecreateClaimFile'stry/finally, so a failed temp write (ENOSPC/EDQUOT/EIO/ENAMETOOLONG) or a non-EEXISTlinkSyncfailure returnsfail(…)with the epic's file already gone (deterministic ENAMETOOLONG repro in review round 3).ok:trueon a different slot (e.g.reported slot 1while the session's env block was for slot 3).This is a regression vs
e4c0ac3/ea0e6f5, where refresh was awriteClaim(tmp +renameSync) that could never leave the slot free and left the old claim intact on failure. Suggested minimal fix (keeplinkSync+EEXIST as the collision detector for new claims): for an own-slot refresh, re-verify the file is still ours (epic +claimed_at) and then atomicallyrenameSynca temp overbyEpic.fileinstead of unlink-then-create, so the path is never absent; and movewriteFileSync(tmp, …)inside the try/finally.Prior findings — verified closed on
ff46f3a.lockand with a fresh.lock(3/3 each); no.lockor.claim-*left behind.linkSync+EEXIST is a sound atomic-claim primitive.byEpic.filethat was read; the mismatched-fixture case (1.jsonbody slot 7/epicvictim,7.json/victim2) now leavesvictim2intact..lockare inert and untouched (/tmp/sendrast-slots/.lockincluded); contention failures return at ~5.03 s with no busy-spin; read-onlyslotsDirreturns in 0.36 s.--worktreesfallback,export SLOT=parse, gate message literalsdiff-ed againste4c0ac3).Non-blocking findings (follow-up material)
Actionable
config.ts:512merely warns about aportsname that is not a shell variable;slot.ts:329then emitsexport ${name}=…verbatim into a block the docs tell users tosource. A repo-committed.pi/epic-session.jsonwith"ports": {"BAD NAME; touch /tmp/PWNED": 0}sources into command execution.composeProjectPrefix(slot.ts:326) is unvalidated too. Validation should gate emission, not warn.README.md:96/docs/epic-session.md:86sayEPIC_SESSION_SLOT_SCRIPTerrors when the path does not exist; it is only candidate #1 (slot.ts:156) and resolution silently falls through.docs/epic-session.md:194says the two backends' claims "never mix"; if a script-backed project's script is ever unresolvable, the builtin writes into that project's script registry (e.g./tmp/sendrast-slots) with a reduced env block (2 exports vs dev-slot.sh's 8 ports).slotBase/slotStride/slotCountpass throughnumberOr(asString(...))(config.ts:477-479), so"slotCount": 5silently becomes 9; only string values work and the README does not say so.tsc --strictvsmain:config.ts:374rec.commandisunknown(should becommand.join(" ")) andslot.ts:499'byEpic' is possibly 'undefined'.epic-session/has nopackage.json/tsconfig.jsonand is not referenced in.forgejo/workflows/ci.yaml, so the green CI runs on this PR gated nothing in the changed package.<slot>.jsonblocks slot selection with misleading messages. A filereadClaimscannot parse (invalid JSON, non-object, a directory, a dangling symlink) is invisible toclaims()/list()yet still makeslinkSyncfail EEXIST. Measured: an auto claim burned the whole 5 s budget and reportedslot registry contention …while 6 slots were free;--slot 4answeredslot 4 was taken concurrently — retryfor a directory. The oldrenameSyncsilently recovered.slot.ts:526-529): a non-ENOENT unlink failure leaves two files naming the epic and still returnsok:true, sofindClaimByEpic(sorted by slot) reports the old slot — prior finding #2's symptom returning silently.release(slot.ts:581-585verifies epic +claimed_at;slot.ts:499deletes whatever is at the path). A release+re-claim of that slot inside the read→unlink window deletes a third party's fresh claim.~/.cache/epic-session/slots/<project>and writes noheartbeat, so dev-slot.sh's opt-in TTL has no counterpart: a session killed withoutendholds its slot until a manualend --force --slot N.branchPrefix plane/+worktreeRoot .worktrees; in effect are the generic defaultsepic/<slug>and~/repos/homelab-wt(config.ts:433-441), pinned nowhere in-repo. The existingepic/LD0worktrees make it defensible, but the ticket's verify step cannot be checked as written.Notes
linkSyncneeds hard-link support on theslotsDirfilesystem — fine for~/.cache, but there is no rename fallback, so a network/FUSEslotsDirwould make the builtin backend permanently unable to claim (failure is honest, not silent).slot: 1.5is accepted (slot.ts:466only range-checks via< 1 || > slotCount); dev-slot.sh'svalidate_slotrejects non-integers.index.ts:773-776escalates a refused release toforce:true(same asmain), so the new "claim changed while releasing — not deleting" guard is immediately followed by a forced delete of whichever epic now holds the slot.list/status) are unsynchronised, so the relocation window (new claim written, old unlinked) can momentarily show two files for one epic — never zero, which is the safe ordering.resolveSlotScriptshellsgit showper repo × per ref on every invocation (homelab: 5 repos × 3 refs) with no negative caching.SessionState.backend(state.ts:26-27) is written and never read.apexDirectorhas no example config inepic-session/examples/; it exists only in the machine-level~/.config/epic-session/projects.json.Final fresh-context review of
869a0d8(and follow-up7862520): VERDICT: CONCERNS — no blockers; blocker fixed confirmed (refresh no longer leaves a claimless gap;linkSyncEEXIST + re-select retry sound).Non-blocking findings posted per landing contract (follow-up candidates):
slot.tsrefresh path): a change landing after the ownership verify is still clobbered. Concrete interleave: force-refresh →endreleases → another epic re-claims the slot → rename deletes the foreign claim. Window is microseconds and requires operator-paced release churn; a flock-style OS lock (matching dev-slot.sh) is the durable fix.claimed_atwith 1 ms resolution — same-millisecond rewrite is undetectable (affects refresh and release re-verify equally).!stillOursretry loop previously had no deadline — fixed in7862520(bounded by CLAIM_RETRY_MS + sleep).composeProjectPrefixfallback interpolated unsanitizedprojectName; fixed in7862520(fallback sanitized too).ports[].nameemitted verbatim into the env block — fixed in7862520(skipped, warning retained)..claim-*debris (swept after 1 h;readClaimsignores it) and nofsyncbefore rename (host-crash could expose a partial claim thatreadClaimssilently skips → epic appears claimless). Low impact for a local registry.slotsDir(.claim-*temp names), andreplaceClaimFile's overwriting rename is the one place the one-claim-per-slot invariant relies on verification rather than EEXIST.Merging as-is; #1–#3 + #7 are the worthwhile follow-ups (a shared flock-based serializer would close #1–#3 together).