[cold-review] feat: session-attached long-running jobs (#1404) - #117
[cold-review] feat: session-attached long-running jobs (#1404)#117heavygee wants to merge 110 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 371df38680
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const heartbeat = setIntervalFn(() => { | ||
| void updateSessionJob({ | ||
| ...clientOpts, | ||
| jobKey: options.jobKey, | ||
| body: { | ||
| detail: options.detail, | ||
| status: 'running' | ||
| } |
There was a problem hiding this comment.
Serialize heartbeat writes before the terminal status
If a timer heartbeat is still in flight when the child exits, clearing the interval does not await or cancel that request; the terminal completed/failed PATCH can reach the hub first, followed by this delayed status: 'running' PATCH, leaving a finished process permanently displayed as running. Track/serialize the heartbeat promise and await it before sending the terminal update, or enforce monotonic terminal status server-side.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| const prev = Array.isArray(meta.jobsAcceptedFromSessionIds) | ||
| ? meta.jobsAcceptedFromSessionIds.filter((id): id is string => typeof id === 'string') | ||
| : [] | ||
| if (prev.includes(fromSessionId)) return | ||
| meta.jobsAcceptedFromSessionIds = [...prev, fromSessionId] |
There was a problem hiding this comment.
Carry redirect ancestry through repeated merges
When job-owning session A is merged and deleted into B, then B is merged and deleted into C, this records only B on C and drops B's existing jobsAcceptedFromSessionIds entry for A. A supervisor still using A's $HAPI_SESSION_ID can therefore no longer resolve the owner and every subsequent heartbeat returns 404; propagate the source session's accepted IDs along with fromSessionId.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| const existing = getSessionJob(db, toSessionId, job.key) | ||
| if (existing) { | ||
| db.prepare('DELETE FROM session_jobs WHERE session_id = ? AND job_key = ?') | ||
| .run(fromSessionId, job.key) |
There was a problem hiding this comment.
Preserve the running job when merge keys collide
When the source and target sessions both contain a common key such as build, this unconditionally deletes the source record even if it is the currently running job and the target record is completed or stale. The merged list temporarily loses the live meter, and the redirected source supervisor's next heartbeat reactivates and mutates the unrelated target record with its old progress; collision handling should preserve both jobs or deterministically retain the live/newer record.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| const body: AttachedJobUpsert = { | ||
| label: options.label, | ||
| status: 'running', | ||
| ...(options.done !== undefined ? { done: options.done } : {}), | ||
| ...(options.total !== undefined ? { total: options.total } : {}), | ||
| ...(options.remaining !== undefined ? { remaining: options.remaining } : {}), | ||
| ...(options.unit !== undefined ? { unit: options.unit } : {}), | ||
| ...(options.detail !== undefined ? { detail: options.detail } : {}) |
There was a problem hiding this comment.
Reset startedAt for each supervised run
When hapi job run is invoked again with the same stable key, this PUT omits startedAt, so the hub deliberately preserves the completed prior run's start time. The newly spawned process then reports elapsed time from the previous invocation—potentially days too large—rather than its actual wall-clock runtime; include a fresh start timestamp when launching each child.
AGENTS.md reference: AGENTS.md:L159-L162
Useful? React with 👍 / 👎.
| WHERE session_id = ? AND status = 'running' | ||
| ORDER BY started_at ASC, job_key ASC | ||
| LIMIT 1` |
There was a problem hiding this comment.
Prevent stale jobs from masking healthy jobs
If an abandoned job remains running, starting a second job under another key never makes the healthy job visible in the session summary because the permanently older started_at always wins. Since stale jobs intentionally remain running and merely turn amber after missed heartbeats, one forgotten record can mask every later supervised job indefinitely; prefer a non-stale/recently-heartbeating job before applying the stable start-time ordering.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| if (movedJobs.moved > 0 || movedJobs.collided > 0) { | ||
| this.recordJobsAcceptedFromSession(newSessionId, oldSessionId, namespace) | ||
| if (!options.deleteOldSession) { | ||
| this.recordJobsTransferredToSession(oldSessionId, newSessionId, namespace) |
There was a problem hiding this comment.
Keep a source redirect until destructive merges commit
During a deleting merge, jobs are moved before several later operations that can throw, but this deliberately omits the source-side redirect on the assumption that the old row will be deleted. If scratchlist file migration, model preservation, or the final deletion fails, the source session remains accessible with no jobs; owner resolution stops at that row instead of consulting the target's acceptance metadata, so the running supervisor receives 404 on every heartbeat. Record a temporary source redirect until deletion succeeds or make the transfer compensating/atomic.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
371df38 to
525aa29
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 525aa294dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return sessions.filter(session => | ||
| session.active | ||
| || session.id === selectedSessionId | ||
| || hasRunningAttachedJob(session) |
There was a problem hiding this comment.
Keep attached-job stubs in the sidebar
When an inactive session has a running attached job but lacks an agentSessionId, name, and summary, prepareSidebarSessions() removes it as an empty stub before this new active-only exception runs (the call order is at SessionList.tsx:1194-1200). Consequently the job never reaches the In progress bucket and its progress remains invisible; include running attached jobs in the earlier sidebar-visibility predicate as well.
AGENTS.md reference: AGENTS.md:L152-L160
Useful? React with 👍 / 👎.
| } else if (job.done !== undefined && job.total !== undefined) { | ||
| parts.push(`${job.done}/${job.total}${job.unit ? ` ${job.unit}` : ''}`) | ||
| } | ||
| const elapsedSec = Math.max(0, Math.round((Date.now() - job.startedAt) / 1000)) |
There was a problem hiding this comment.
Stop elapsed time when a job reaches a terminal state
When hapi job list is run after a supervised job has completed or failed, the record remains persisted but this formatter always subtracts startedAt from the current time. A seconds-long completed job therefore reports hours or days of elapsed runtime when listed later; include updatedAt in the formatter input and use it as the endpoint for terminal statuses.
AGENTS.md reference: AGENTS.md:L161-L162
Useful? React with 👍 / 👎.
| 'hapi_session_job', | ||
| 'happy__session_job', | ||
| 'mcp__hapi__session_job', | ||
| 'session-attached job' |
There was a problem hiding this comment.
Bind job auto-approval to the HAPI MCP server
In ACP-backed integrations the permission handler derives toolName from the request title before considering the qualified tool identity, so any other configured MCP tool titled Session-Attached Job matches this process-wide allowlist. If that tool has unrelated or destructive side effects, it is automatically approved even in default or read-only modes; scope this exemption to a verified HAPI server/tool identifier rather than an unqualified display title.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 727e0a2676
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| unit: patch.unit === null ? undefined : (patch.unit ?? existing.unit), | ||
| detail: patch.detail === null ? undefined : (patch.detail ?? existing.detail), | ||
| heartbeatAt: patch.heartbeatAt ?? now, | ||
| updatedAt: now |
There was a problem hiding this comment.
Make attached-job versions strictly monotonic
When two job writes occur in the same millisecond—particularly a fast job run registration followed by its terminal PATCH—both receive the same updatedAt. The web client only accepts attached-job patches whose version is strictly greater than its current watermark, so after applying the running event it ignores the same-version completed/failed event and can display a finished job as running until a refetch; derive each version as at least the previous value plus one, and do the same when emitting DELETE/null events.
AGENTS.md reference: AGENTS.md:L154-L161
Useful? React with 👍 / 👎.
| if (signal) { | ||
| resolve(128 + (signal === 'SIGINT' ? 2 : signal === 'SIGTERM' ? 15 : 1)) | ||
| return |
There was a problem hiding this comment.
Preserve the child's signal exit status
When the supervised child is terminated by any signal other than SIGINT or SIGTERM—for example SIGKILL from an OOM killer—this returns 129 regardless of the actual signal, so hapi job run reports the wrong exit status to scripts and operators (SIGKILL conventionally yields 137). Map every available signal number to 128 + signalNumber rather than collapsing the remaining signals to 1.
AGENTS.md reference: AGENTS.md:L156-L160
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a423e383a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (movedJobs.moved > 0 || movedJobs.collided > 0) { | ||
| this.recordJobsAcceptedFromSession(newSessionId, oldSessionId, namespace) |
There was a problem hiding this comment.
Record the job redirect for every merge
When a merge occurs before the source's first job row exists—for example, hapi job run resolves the old $HAPI_SESSION_ID and then races a resume/dedup merge—movedJobs is zero, so this skips the redirect before deleting the source. The subsequent PUT cannot discover the new owner and returns 404, preventing the supervised child from starting; persist the source-to-target mapping regardless of whether jobs existed at merge time.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0afd5d4e55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| || (typeof meta?.supersededBySessionId === 'string' | ||
| && meta.supersededBySessionId.trim()) |
There was a problem hiding this comment.
Preserve job ownership across fresh-session clears
When a session with an attached job completes a fresh-session /clear, supersededBySessionId points to the replacement, but no clear path transfers the session_jobs rows. This resolver therefore sends the original supervisor's PATCH requests to the replacement, where the key does not exist, so heartbeats and the terminal update return 404 while the old row remains permanently running and turns stale. Transfer jobs when establishing the clear redirect, or do not follow supersededBySessionId for job APIs.
AGENTS.md reference: AGENTS.md:L154-L159
Useful? React with 👍 / 👎.
| if (arg === '--started-at') { | ||
| result.startedAt = parseOptionalNumber('--started-at', flagArgs[++i]) | ||
| continue |
There was a problem hiding this comment.
Reject --started-at outside set
When hapi job update includes --started-at together with a progress field, the parser accepts the flag but the update body silently drops it, and the command reports success while leaving the elapsed-time origin unchanged. This is especially misleading for the documented late-attach correction workflow; reject this flag for non-set actions (and likewise reject other action-inapplicable flags) rather than silently ignoring it.
AGENTS.md reference: AGENTS.md:L161-L161
Useful? React with 👍 / 👎.
| jobsAcceptedFromSessionIds: z.array(z.string()).optional(), | ||
| jobsTransferredToSessionId: z.string().optional(), |
There was a problem hiding this comment.
Protect job redirect metadata from CLI updates
These redirect fields are hub-owned routing state, but preserveHubOwnedMetadata() still protects only supersededBySessionId and opencodeClearOperation, and the store's metadata merge carry-forward lists omit both new keys. After a merge, any CLI metadata replacement that lacks these fields can therefore erase jobsAcceptedFromSessionIds or jobsTransferredToSessionId; supervisors retaining the pre-merge session ID then receive 404s on every heartbeat. Add both fields to the hub-owned preservation path rather than treating them as ordinary agent metadata.
AGENTS.md reference: AGENTS.md:L154-L159
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0352c60c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // session_job is intentionally NOT in this name-only allowlist: ACP titles are | ||
| // spoofable (tool-name derivation prefers backend title). Auto-approve only via | ||
| // trusted HAPI bridge config (Codex tools map / Claude --allowedTools). |
There was a problem hiding this comment.
Treat the job title as write-like in read-only mode
Fresh evidence in this revision shows the title-spoof issue remains in read-only mode: although this removes session_job from name-only approval, resolveToolAutoApprovalDecision() still approves every read-only tool whose displayed name does not match AUTO_APPROVE_WRITE_TOOL_HINTS (lines 102–104), and Session-Attached Job matches none of those hints. Consequently an unrelated MCP tool using this title—and the genuine hub-writing job tool—still bypasses approval in read-only mode; add the job title to write-sensitive handling or validate a qualified trusted tool identity.
Useful? React with 👍 / 👎.
| private attachedJobEmitVersion = new Map<string, number>() | ||
|
|
||
| allocateAttachedJobVersion(sessionId: string): number { | ||
| const prev = this.attachedJobEmitVersion.get(sessionId) ?? 0 | ||
| const version = Math.max(Date.now(), prev + 1) | ||
| this.attachedJobEmitVersion.set(sessionId, version) |
There was a problem hiding this comment.
Evict job watermarks when sessions are removed
Every /api/sessions snapshot calls allocateAttachedJobVersion() for every returned session, permanently inserting its ID into this map, but none of the session-removal paths (refreshSession, deleteSession, or deleting merges) removes the corresponding entry. A long-running hub that creates and deletes many sessions therefore retains one map entry per historical session indefinitely; delete this watermark alongside the other per-session cache state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 404491cc76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sourceWins = | ||
| (job.status === 'running' && existing.status !== 'running') | ||
| || (job.status === existing.status && job.updatedAt > existing.updatedAt) | ||
| if (sourceWins) { |
There was a problem hiding this comment.
Keep both running jobs when merge keys collide
Fresh evidence beyond the previously flagged terminal-target case: when both source and target jobs are running under the same key, this condition retains only whichever row has the newer updatedAt. Both supervisors are then redirected to that single row, so the losing process's heartbeat mutates the winner and its exit can mark the shared job completed or failed while the other process is still running. Preserve distinct ownership for both live jobs or prevent their supervisors from targeting the collided row.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| export function attachedJobFraction(job: AttachedJob): number | null { | ||
| if (job.done !== undefined && job.total !== undefined && job.total > 0) { | ||
| return Math.max(0, Math.min(1, job.done / job.total)) |
There was a problem hiding this comment.
Match the progress bar to the displayed remaining count
When a job contains remaining, done, and total—which the API permits—the text prioritizes remaining, but this function prioritizes done. Because PATCH preserves unspecified fields, updating only remaining after an earlier done/total update can display 20 left beside a 10% bar instead of the implied 80%; prioritize remaining here as the formatter does, or validate that all supplied counts are consistent.
AGENTS.md reference: AGENTS.md:L160-L160
Useful? React with 👍 / 👎.
| inflightHeartbeat = updateSessionJob({ | ||
| ...clientOpts, | ||
| jobKey: options.jobKey, | ||
| body: options.detail !== undefined ? { detail: options.detail } : {} | ||
| }).catch((error: unknown) => { |
There was a problem hiding this comment.
Stop heartbeats from restoring the launch detail
When the supervised child or a companion wrapper updates this job's detail, every subsequent timer heartbeat sends the original options.detail again and overwrites the newer stage/current-item text. This makes an otherwise healthy self-updating job repeatedly display stale progress; send an empty heartbeat PATCH here so the hub stamps only heartbeatAt, leaving explicit child updates authoritative.
AGENTS.md reference: AGENTS.md:L158-L160
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1f306365a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| status: 'running', | ||
| // Supervised child: always this run's clock. Omitting startedAt would | ||
| // sticky-reuse a prior completed/failed row's startedAt on key reuse. | ||
| startedAt: Date.now(), |
There was a problem hiding this comment.
Stamp supervised starts at actual launch
When the CLI uses a remote hub—or authentication, session lookup, or registration is slow—this records the runner's Date.now() before those requests, while the child is not spawned until after they finish. The web then computes elapsed time using its own clock, so setup latency is counted as runtime and clock skew can make a new job appear inflated or remain at 0s; have the hub reset startedAt for a fresh supervised run, or re-stamp it from a server timestamp immediately before launch.
AGENTS.md reference: AGENTS.md:L158-L161
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29ed79a0e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| resolveAttachedJobSessionId(sessionId: string, namespace: string): string { | ||
| let current = sessionId | ||
| for (let hop = 0; hop < 5; hop += 1) { |
There was a problem hiding this comment.
Follow attached-job redirects beyond five retained merges
When a running job passes through six consecutive mergeSessionHistory operations, each retained source row adds another jobsTransferredToSessionId hop, but this loop stops after five and returns the penultimate session instead of the current owner. Because that session still exists, resolveJobOwnerSession() accepts it and every subsequent heartbeat or terminal PATCH targets a row with no job and returns 404, leaving the actual job permanently running/stale; follow redirects with a visited-set or flatten the chain when recording transfers.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| heartbeatAt: z.number().optional(), | ||
| startedAt: z.number().optional() |
There was a problem hiding this comment.
Reject fractional attached-job timestamps
When a client sends a fractional startedAt or heartbeatAt—for example --started-at 1.5 or a raw PATCH with heartbeatAt: 1.5—these schemas accept it, but the new STRICT SQLite table stores the fields as INTEGER, so the write throws cannot store REAL value in INTEGER column and the route returns a server error instead of the intended 400 validation response. Constrain both timestamp fields to integers (and apply the same constraint to PATCH heartbeatAt).
AGENTS.md reference: AGENTS.md:L61-L61
Useful? React with 👍 / 👎.
| ...(parsed.heartbeatSec !== undefined | ||
| ? { heartbeatMs: Math.max(5, parsed.heartbeatSec) * 1000 } | ||
| : {}), |
There was a problem hiding this comment.
Reject heartbeat intervals beyond the timer range
When --heartbeat-sec exceeds roughly 2,147,483 seconds, multiplying it by 1,000 produces a delay larger than the 32-bit range supported by Node/Bun timers. setInterval then emits TimeoutOverflowWarning and clamps the delay to 1 ms, so a long-running child can flood the hub with about a thousand PATCH requests per second instead of heartbeating rarely; validate or cap this option before converting it to milliseconds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3e7363e95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!redirects || typeof redirects !== 'object' || Array.isArray(redirects)) { | ||
| return jobKey | ||
| } | ||
| const mapped = (redirects as Record<string, unknown>)[`${requestedSessionId}/${jobKey}`] |
There was a problem hiding this comment.
Flatten inherited key redirects on repeat collisions
When A's running build is remapped to build.a during A→B, then B→C encounters another live collision on build.a, the row is remapped again to build.a.b, but the inherited mapping remains A/build → build.a while only B/build.a → build.a.b is added. This single direct lookup therefore routes A's surviving supervisor to C's unrelated build.a job; its heartbeat or exit can mutate or terminal-mark that process while the actual build.a.b row becomes stale. Flatten inherited mappings when a key is remapped again, or resolve the mapping transitively.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce5b1f8042
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ON CONFLICT(session_id, job_key) DO UPDATE SET | ||
| label = excluded.label, | ||
| status = excluded.status, |
There was a problem hiding this comment.
Isolate overlapping runs that reuse a job key
When two hapi job run invocations overlap on the same session and key, this unconditional upsert makes both supervisors own the same row. The older process can then exit and PATCH that row to completed or failed while the newer process is still running; a delayed terminal retry can cause the same ABA race after apparent key reuse. Refuse a second live owner or attach a generation token that every heartbeat and terminal update must match.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| const child: ChildProcess = spawnFn(options.command[0]!, options.command.slice(1), { | ||
| stdio: 'inherit', | ||
| env: process.env | ||
| }) |
There was a problem hiding this comment.
Mark the job failed when spawning throws
When command[0] is empty, such as hapi job run ... -- "", both Node and Bun throw synchronously from spawn() instead of emitting the registered error event. Because the job was already registered as running, this exception bypasses all terminal-status handling and leaves a permanently running record that eventually turns stale; validate the executable before registration or catch synchronous spawn failures and mark the job failed.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
ce5b1f8 to
eb63e98
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb63e98c82
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** Active sessions that warrant the optional pinned In progress section. Quiet actives stay in directory groups. */ | ||
| function isPinnedInProgressSession(session: SessionSummary): boolean { | ||
| function hasRunningAttachedJob(session: SessionSummary): boolean { | ||
| return session.attachedJob?.status === 'running' |
There was a problem hiding this comment.
Keep job-bearing rows through sidebar deduplication
When two active session rows share an agentSessionId, the hub intentionally retains both (sessionCache.ts:1792-1797), but prepareSidebarSessions() deduplicates them before this predicate is used and its active/recency ordering does not account for attached jobs. If the running job belongs to the losing duplicate, the row and its progress chip disappear even in jobs mode; exclude job-bearing rows from deduplication or merge their job presentation into the retained row.
AGENTS.md reference: AGENTS.md:L154-L154
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2533a54dca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| started_at INTEGER NOT NULL, | ||
| updated_at INTEGER NOT NULL, | ||
| PRIMARY KEY (session_id, job_key), | ||
| FOREIGN KEY (session_id) REFERENCES sessions(id) ON DELETE CASCADE |
There was a problem hiding this comment.
Protect jobs in every session-deletion path
When Codex Desktop consolidates inactive duplicate sessions, hub/src/web/routes/codexDesktop.ts:1338-1343 deletes each source through engine.deleteSession() or the store directly, bypassing the new running-job check in the ordinary REST delete route. This cascade therefore removes a source's still-running attached job; its supervisor then receives 404 for every heartbeat and terminal update even though the process continues. Transfer or reject running jobs in the shared deletion layer before allowing either consolidation path to delete the session.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
015e256 to
3a25249
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a25249388
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| engine.transferAttachedJobs(source.sessionId, canonical.sessionId, options.namespace) | ||
| await engine.deleteSession(source.sessionId) | ||
| } else { | ||
| options.store.sessionJobs.transfer(source.sessionId, canonical.sessionId) |
There was a problem hiding this comment.
Persist redirects in the store-only merge path
When getSyncEngine() is null, this branch moves the running rows and then deletes the source session without recording jobsAcceptedFromSessionIds or any collision key redirects. A supervisor retaining the source $HAPI_SESSION_ID therefore receives 404s after a Codex duplicate merge, leaving the transferred job permanently running and stale. Fresh evidence beyond the earlier deletion finding is that the new fallback now preserves the row but still makes it unreachable; route this path through redirect-aware transfer logic or reject the merge until the engine is available.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| nextSummary.metadataVersion = patch.metadata.version | ||
| } | ||
| if ( | ||
| patch.attachedJob !== undefined |
There was a problem hiding this comment.
Keep job-only patches out of detail invalidation
When the running job belongs to the currently open session, each heartbeat emits an attachedJob-only patch that is applied here to the summary, but the same event is first passed to patchSessionDetail(). Because applySessionDetailPatch() neither consumes nor explicitly ignores attachedJob, it returns null, causing queueSessionDetailInvalidation() to refetch /sessions/:id on every heartbeat; with an allowed short --heartbeat-sec this creates continuous redundant detail requests despite the payload already being delivered inline. Treat this summary-only field as successfully handled for the detail cache.
AGENTS.md reference: AGENTS.md:L158-L160
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82782a322c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!parsed.success) { | ||
| return c.json({ error: 'Invalid body', issues: parsed.error.issues }, 400) | ||
| } | ||
| const result = engine.upsertSessionJob(sessionResult.sessionId, jobKey, parsed.data) |
There was a problem hiding this comment.
Isolate post-merge PUT key collisions
When a merge redirects source session A to B before A has a row for build, a later job run A build resolves to B's unmodified build key. If B already has a running build, this upsert replaces B's row instead of allocating a source-specific key, so B's supervisor starts receiving run-mismatch responses and its still-running process disappears from the meter. Allocate and persist a key redirect when a redirected PUT collides with an existing live job, not only for rows present during the merge.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| // Supervised child: always this run's clock. Omitting startedAt would | ||
| // sticky-reuse a prior completed/failed row's startedAt on key reuse. | ||
| // startedAt is also the run-generation fence on later PATCHes. | ||
| const startedAt = Date.now() |
There was a problem hiding this comment.
Use a unique generation for supervised runs
Fresh evidence in this revision is that the new run fence is only the millisecond-valued Date.now(): if two hapi job run processes start with the same session/key during one clock tick, both register the same startedAt, so both supervisors pass expectedStartedAt checks and the older process can still heartbeat or terminal-mark the newer run. Use a hub-issued or otherwise collision-resistant generation rather than the display timestamp as the ownership token.
AGENTS.md reference: AGENTS.md:L156-L160
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff338fa098
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : (existing?.startedAt ?? now) | ||
| // Every PUT gets a unique run generation unless the client supplies one | ||
| // (supervisors mint UUID; hub mints when omitted so key reuse still fences). | ||
| const runId = body.runId ?? randomUUID() |
There was a problem hiding this comment.
Preserve the supervisor generation during clock correction
When hapi job set ... --started-at corrects the clock of a job currently owned by hapi job run, the CLI sends no runId, so this generates and persists a new one. Every subsequent supervisor heartbeat and terminal PATCH then fails its expectedRunId check, leaving the corrected job permanently running and eventually stale; make clock correction preserve or CAS against the existing generation rather than silently replacing it.
AGENTS.md reference: AGENTS.md:L158-L162
Useful? React with 👍 / 👎.
7549148 to
adbad8c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 742f102910
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ...(parsed.heartbeatSec !== undefined | ||
| ? { heartbeatMs: Math.max(5, parsed.heartbeatSec) * 1000 } |
There was a problem hiding this comment.
Keep configured heartbeats inside the freshness window
When --heartbeat-sec is greater than 900 (for example, 1800), the supervisor follows that interval but the web client unconditionally declares the job stale after 15 minutes (web/src/lib/attachedJob.ts:3-4). A healthy process therefore spends part or most of every interval shown in amber as “no heartbeat,” contradicting the attached job’s truthful status; cap the interval below the stale threshold or make the freshness window account for the configured cadence.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
742f102 to
4510e2e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4510e2e1b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| busyMarkers: AGY_BUSY_MARKERS, | ||
| idleReadyMs: AGY_IDLE_READY_MS, | ||
| idleMarkers: AGY_IDLE_MARKERS, | ||
| thinkingSilenceTimeoutMs: null, |
There was a problem hiding this comment.
Preserve split idle markers when disabling the watchdog
When AGY's final idle footer is split across PTY chunks after a busy marker—for example, one chunk ends with Generating...\n? for shor and the next begins with tcuts—the busy branch in runAgentPty() clears promptBuffer, discarding the partial idle marker. With the silence watchdog now disabled, no fallback clears thinking or restores inputReady, so remote messages can remain queued indefinitely. Retain the possible idle-marker suffix across the busy transition or keep a bounded recovery path.
Useful? React with 👍 / 👎.
| // duplicate at all: it is the news that a new request has started | ||
| // failing. Only an exact repeat is dropped, which is what OpenCode | ||
| // republishes while a backoff is pending. | ||
| if (retry.attempt === lastSeenAttempt) return; |
There was a problem hiding this comment.
Keep distinct retry reasons at the same attempt number
When separate provider requests in one OpenCode turn each retry at the same attempt number without an intervening non-retry status—the 1, 1, 1 sequence documented above this state—the second event is dropped even if its provider message is different. This can leave the timeline claiming the original rate-limit or overload reason while the current request is failing for another reason; deduplicate identical retry payloads rather than attempt numbers alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9f15c22ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Move outliving jobs before writing supersededBySessionId. resolveAttachedJobSessionId | ||
| // follows that link; without a transfer, heartbeats on the retained source id hit the | ||
| // empty replacement while the meter row stays frozen on the archived source. | ||
| this.transferAttachedJobs(sessionId, replacementSessionId, namespace) |
There was a problem hiding this comment.
Transfer jobs from the resolved owner during clear
Fresh evidence in this revision is that the new clear transfer still assumes sessionId physically owns its rows. When the source is a retained mergeSessionHistory donor whose jobs already live behind jobsTransferredToSessionId, this call transfers zero rows and then overwrites that redirect to the replacement; subsequent supervisor heartbeats follow the new link and 404 while the actual job remains stranded on the former owner. Resolve and transfer the current attached-job owner, or preserve a route to it, before linking the clear replacement.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
| WHERE session_id = ? AND status = 'running' | ||
| ORDER BY started_at ASC, job_key ASC | ||
| LIMIT 1` |
There was a problem hiding this comment.
Prefer a fresh heartbeat when choosing the primary job
When an older job loses its supervisor and becomes stale, then a different-key job starts and continues heartbeating, this ordering permanently selects the older stale row because both remain running. Since /api/sessions enriches only this primary row and SessionRowSummary renders only that payload, the healthy job's progress is hidden until the operator explicitly clears the orphan; prioritize fresh jobs before applying the stable earliest-start ordering, or surface all running jobs.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75e3fbee75
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const targetWasRedirectedToSource = | ||
| this.resolveAttachedJobSessionId(toSessionId, namespace) === fromSessionId | ||
| const movedJobs = this.store.sessionJobs.transfer(fromSessionId, toSessionId) |
There was a problem hiding this comment.
Route transfers through the target's current job owner
When the target already redirects to a third session—for example, B transferred its jobs to C, then A is consolidated into B—this special case does not clear B's outgoing redirect, yet the rows are moved physically from A to B. The new A→B redirect then resolves transitively to C, so A's supervisor heartbeats and terminal PATCHes target C, return 404, and leave the row on B permanently stale. Move the rows to B's resolved owner or flatten/clear every outgoing target redirect before installing A→B.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
65eb544 to
c751690
Compare
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c751690c27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| this.forwardSessionInfoUpdate(sessionId, update); | ||
| this.captureUsageUpdate(update); | ||
| this.notifyAgentActivity(update); |
There was a problem hiding this comment.
Suppress load replays before bumping thinking
On Cursor ACP resume, cursorAcpRemoteLauncher.ts installs this listener before calling backend.loadSession(), and session/load deliberately replays historical session/update notifications (the probe drains and counts them in hub/src/cursor/acpVerifyProbe.ts). This unconditional notification classifies replayed message/tool chunks as live activity and calls onThinkingChange(true); with ACP v1 there is no state_update: idle, and the launcher does not clear thinking until a later prompt finishes, so an idle resumed session can appear indefinitely as thinking. Suppress activity notifications during load/replay or arm the listener after replay completes.
Useful? React with 👍 / 👎.
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba7c433be0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await setSessionJob({ | ||
| ...clientOpts, | ||
| jobKey: options.jobKey, | ||
| body | ||
| }) |
There was a problem hiding this comment.
Retry ambiguous job registration before aborting
When a remote hub commits this PUT but its response is lost or exceeds the 15-second client timeout, setSessionJob() throws and this function exits before spawning the child, leaving a never-started job recorded as running indefinitely. Because the freshly generated runId makes this registration idempotent, retry the PUT or reconcile/terminal-mark that generation before aborting so the session list does not retain a false running job.
AGENTS.md reference: AGENTS.md:L154-L158
Useful? React with 👍 / 👎.
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bcb3da761
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const body: AttachedJobPatch = { | ||
| ...(args.label !== undefined ? { label: args.label } : {}), | ||
| ...(args.status !== undefined ? { status: args.status } : {}), | ||
| ...(args.done !== undefined ? { done: args.done } : {}), | ||
| ...(args.total !== undefined ? { total: args.total } : {}), |
There was a problem hiding this comment.
Fence MCP updates with the current run ID
When an agent lists a supervised job and its key is reused before a subsequent MCP update arrives, this body cannot include expectedRunId because the MCP schema and argument type expose no such field. The hub therefore accepts the stale update and it can change progress or terminal-mark the newer process, bypassing the generation fence used by hapi job run and CLI wrappers. Expose the current runId in MCP list output and pass an expectedRunId through update calls.
AGENTS.md reference: AGENTS.md:L158-L160
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error) | ||
| console.error(`[hapi job run] failed to mark job ${terminalStatus}: ${message}`) | ||
| } |
There was a problem hiding this comment.
Fail when the terminal job status cannot be recorded
When a child exits successfully but every terminal PATCH attempt fails because the hub remains unavailable, this catch only logs the failure and runSessionJob() still returns the child's zero exit code. The shell therefore reports a successful supervised run while the persisted job remains running and eventually becomes stale, defeating the command's exit-status supervision contract; propagate a nonzero result or reconcile the terminal generation before returning success, while continuing to treat an expected run_mismatch as supersession.
AGENTS.md reference: AGENTS.md:L154-L160
Useful? React with 👍 / 👎.
MCP update-before-create and hapi job run -- sleep while real work runs elsewhere can leave a misleading running meter. Return job-run recipe on not_found, warn when remaining hits zero without completion, and tighten session_job agent instructions. Co-authored-by: Cursor <cursoragent@cursor.com>
MCP HTTP server bundles can load duplicate SessionJobError identities so update-before-create fell through to bare "job not found". Detect not_found by code and message, not instanceof alone. Co-authored-by: Cursor <cursoragent@cursor.com>
Clear on an already-absent job returns success without steering to hapi job run. Parser rejects --heartbeat-sec on set/update/clear/list. Co-authored-by: Cursor <cursoragent@cursor.com>
Parser now fails fast when list/clear receive label/status/progress fields, or when run receives --status, instead of silently discarding them. Co-authored-by: Cursor <cursoragent@cursor.com>
MCP list/clear reject incompatible fields; CLI rejects -- <cmd> unless action is run so clear/set cannot silently drop a trailing command. Co-authored-by: Cursor <cursoragent@cursor.com>
Started-at corrections via job set must not mint a new generation and break an in-flight supervisor's expectedRunId fence. Docs heartbeat recipe now shows run-id + expected-run-id. Co-authored-by: Cursor <cursoragent@cursor.com>
job set defaults to running; remaining=0 alone does not finish the meter. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep Jobs in the In progress section after upstream split Active/Running. Preserve Cursor ACP retry fields and Codex prompt test names from main. Quiet connected sessions stay in directories (tiann#1404 stand). Co-authored-by: Cursor <cursoragent@cursor.com>
Connected-but-idle sessions stay in project folders in every pin mode. UI mode label is now Working & pending; drop the unreachable Active pin bucket and the copy that claimed quiet actives floated under All. Co-authored-by: Cursor <cursoragent@cursor.com>
Document hub-row vs worker HAPI_SESSION_ID bug and that --remaining alone does not move the progress bar without total. Co-authored-by: Cursor <cursoragent@cursor.com>
`export … from` does not put the helper in module scope, so attachedJob list patches failed typecheck after the rebase onto upstream's extract. Guide copy now matches Working & pending; quiet connected stays in folders. Co-authored-by: Cursor <cursoragent@cursor.com>
A failed metadata write after sessionJobs.transfer left the old session id pointing at an emptied source, so heartbeats 404ed. Co-authored-by: Cursor <cursoragent@cursor.com>
Named sessionPatch imports must keep markMessagesIndeterminate/Requeued from upstream delivery_state work; conflict markers slipped into the tip. Co-authored-by: Cursor <cursoragent@cursor.com>
AGENTS and recipe copy no longer treat \$HAPI_SESSION_ID as always safe; remote Cursor may export a worker row. Cursor resume bootstrap test asserts the requested existingSessionId is exported into the child env. Co-authored-by: Cursor <cursoragent@cursor.com>
persistAttachedJobRedirects wrote SQLite while resolveAttachedJobSessionId still read stale cache; heartbeats via the old id could 404 during attachment I/O. Refresh both sessions immediately after persist. Co-authored-by: Cursor <cursoragent@cursor.com>
state_update_running chatter while the queue is idle was setting hub thinking and floating quiet sessions (with jobs) into Working. Permission and requires_action still bump; prompt-owned running still bumps. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Strip VT/ANSI and remaining C0 controls from job key/label/unit/detail/runId in formatJobLine so list/set/update cannot emit OSC escapes from persisted fields. Co-authored-by: Cursor <cursoragent@cursor.com>
Move terminalText/formatJobLine into modules so CLI stdout and MCP tool results both strip VT/ANSI/C0 before agents render job fields. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
When a merged-away session registers a new runId for a key the target already owns as a different running generation, allocate a remapped key and persist jobKeyRedirects so the target's live meter is not overwritten. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Bare process.kill only hit the immediate child; a shell grandchild could keep running after the wrapper marked the job terminal. Use killProcessByChildProcess. Co-authored-by: Cursor <cursoragent@cursor.com>
Forward the received signal through killProcessByChildProcess so Ctrl-C stays exit 130, and wait for verified tree termination before writing the terminal job meter. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
After rebasing onto main, mark-all-read and app-badge helpers need attachedJob / attachedJobUpdatedAt for SessionSummary typecheck. Co-authored-by: Cursor <cursoragent@cursor.com>
741272d to
b625adc
Compare
Rebase left conflict markers in the pin-mode/appBadge locale block; typecheck fails with TS1185. Co-authored-by: Cursor <cursoragent@cursor.com>
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44bb0abf41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } = useSessions(api, { enabled: appBadgeEnabled }) | ||
| useAppBadge({ | ||
| enabled: appBadgeEnabled, | ||
| scope: baseUrl, |
There was a problem hiding this comment.
Include the namespace in the badge read-state scope
When the same installed PWA is used with two CLI_API_TOKEN:<namespace> logins on one hub, both badge sessions pass the identical baseUrl scope. initializeSessionLastSeen() stores its baseline under that scope, so after the first namespace initializes it, the second namespace skips seeding its existing sessions and counts them all as unread, producing a bogus OS badge after reauthentication. Scope the baseline/read state by hub plus namespace or another stable auth identity.
AGENTS.md reference: AGENTS.md:L173-L173
Useful? React with 👍 / 👎.
| const ordinary = response.rateLimits; | ||
| const recovered = response.ordinaryUsageAllowed != null | ||
| && (response.ordinaryUsageAllowed || ordinary.credits?.hasCredits || ordinary.credits?.unlimited) | ||
| && response.rateLimitUpsell == null | ||
| && ordinary.spendControlReached !== true && ordinary.rateLimitReachedType == null; |
There was a problem hiding this comment.
Read recovery flags from the ordinary quota bucket
When the usage response places a model-specific or Reserve bucket in top-level rateLimits and the ordinary bucket under rateLimitsByLimitId.codex—a response shape publishUsage() explicitly supports—this recovery check reads the Reserve bucket's credits, spend-control, and reached-type fields instead. An exhausted Reserve bucket can therefore keep recovered false even after ordinaryUsageAllowed becomes true, leaving the thread permanently on Luna. Resolve the ordinary bucket here with the same logic used by publishUsage().
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| await reserve.initialize(); | ||
| this.usageTimer = setInterval(() => { void refreshUsage(); }, 60_000); |
There was a problem hiding this comment.
Avoid touching session activity on unchanged quota polls
Every remote Codex session runs this timer once per minute, and LunaReserve.refresh() calls publishUsage() even when the quota payload is unchanged or unavailable. The publish callback always invokes updateAgentState, whose hub persistence path updates the session's updated_at; quiet Codex sessions therefore continually jump to the top of the session list and appear to have unread activity solely because quota was polled. Compare the published usage with the previous value or persist this telemetry without touching session activity.
Useful? React with 👍 / 👎.
Fork-side cold-review stage PR. Not for merge.
Cursor Opus cold review pass 3 (session d3184c43) Ready-for-PR YES @ tip including JWT refresh.
Upstream issue: tiann#1404