Skip to content

[cold-review] feat: session-attached long-running jobs (#1404) - #117

Open
heavygee wants to merge 110 commits into
mainfrom
feat/session-attached-jobs
Open

[cold-review] feat: session-attached long-running jobs (#1404)#117
heavygee wants to merge 110 commits into
mainfrom
feat/session-attached-jobs

Conversation

@heavygee

@heavygee heavygee commented Aug 8, 2026

Copy link
Copy Markdown
Owner

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

@heavygee heavygee added the cold-review-clean Fork-side bot review is satisfactory; safe to promote to upstream PR label Aug 8, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +78 to +85
const heartbeat = setIntervalFn(() => {
void updateSessionJob({
...clientOpts,
jobKey: options.jobKey,
body: {
detail: options.detail,
status: 'running'
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment thread hub/src/sync/sessionCache.ts Outdated
Comment on lines +1302 to +1306
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]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread hub/src/store/sessionJobs.ts Outdated
Comment on lines +257 to +260
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +40 to +47
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 } : {})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +91 to +93
WHERE session_id = ? AND status = 'running'
ORDER BY started_at ASC, job_key ASC
LIMIT 1`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread hub/src/sync/sessionCache.ts Outdated
Comment on lines +1183 to +1186
if (movedJobs.moved > 0 || movedJobs.collided > 0) {
this.recordJobsAcceptedFromSession(newSessionId, oldSessionId, namespace)
if (!options.deleteOldSession) {
this.recordJobsTransferredToSession(oldSessionId, newSessionId, namespace)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@heavygee
heavygee force-pushed the feat/session-attached-jobs branch from 371df38 to 525aa29 Compare August 8, 2026 23:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread cli/src/commands/job.ts Outdated
} 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))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +133 to +135
if (signal) {
resolve(128 + (signal === 'SIGINT' ? 2 : signal === 'SIGTERM' ? 15 : 1))
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread hub/src/sync/sessionCache.ts Outdated
Comment on lines +1205 to +1206
if (movedJobs.moved > 0 || movedJobs.collided > 0) {
this.recordJobsAcceptedFromSession(newSessionId, oldSessionId, namespace)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +1437 to +1438
|| (typeof meta?.supersededBySessionId === 'string'
&& meta.supersededBySessionId.trim())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread cli/src/commands/job.ts
Comment on lines +177 to +179
if (arg === '--started-at') {
result.startedAt = parseOptionalNumber('--started-at', flagArgs[++i])
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread shared/src/schemas.ts
Comment on lines +125 to +126
jobsAcceptedFromSessionIds: z.array(z.string()).optional(),
jobsTransferredToSessionId: z.string().optional(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +42 to +44
// 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).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +608 to +613
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +265 to +268
const sourceWins =
(job.status === 'running' && existing.status !== 'running')
|| (job.status === existing.status && job.updatedAt > existing.updatedAt)
if (sourceWins) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread web/src/lib/attachedJob.ts Outdated
Comment on lines +48 to +50
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))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +86 to +90
inflightHeartbeat = updateSessionJob({
...clientOpts,
jobKey: options.jobKey,
body: options.detail !== undefined ? { detail: options.detail } : {}
}).catch((error: unknown) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread hub/src/sync/sessionCache.ts Outdated
*/
resolveAttachedJobSessionId(sessionId: string, namespace: string): string {
let current = sessionId
for (let hop = 0; hop < 5; hop += 1) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread shared/src/schemas.ts Outdated
Comment on lines +399 to +400
heartbeatAt: z.number().optional(),
startedAt: z.number().optional()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread cli/src/commands/job.ts
Comment on lines +343 to +345
...(parsed.heartbeatSec !== undefined
? { heartbeatMs: Math.max(5, parsed.heartbeatSec) * 1000 }
: {}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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}`]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +151 to +153
ON CONFLICT(session_id, job_key) DO UPDATE SET
label = excluded.label,
status = excluded.status,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +111 to +114
const child: ChildProcess = spawnFn(options.command[0]!, options.command.slice(1), {
stdio: 'inherit',
env: process.env
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@heavygee
heavygee force-pushed the feat/session-attached-jobs branch from ce5b1f8 to eb63e98 Compare August 9, 2026 02:00

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread web/src/components/SessionList.tsx Outdated
/** 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'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread hub/src/store/index.ts
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@heavygee
heavygee force-pushed the feat/session-attached-jobs branch from 015e256 to 3a25249 Compare August 9, 2026 10:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread hub/src/web/routes/codexDesktop.ts Outdated
engine.transferAttachedJobs(source.sessionId, canonical.sessionId, options.namespace)
await engine.deleteSession(source.sessionId)
} else {
options.store.sessionJobs.transfer(source.sessionId, canonical.sessionId)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread web/src/hooks/useSSE.ts
nextSummary.metadataVersion = patch.metadata.version
}
if (
patch.attachedJob !== undefined

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread hub/src/store/sessionJobs.ts Outdated
: (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()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@heavygee
heavygee force-pushed the feat/session-attached-jobs branch from 7549148 to adbad8c Compare August 9, 2026 12:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread cli/src/commands/job.ts
Comment on lines +343 to +344
...(parsed.heartbeatSec !== undefined
? { heartbeatMs: Math.max(5, parsed.heartbeatSec) * 1000 }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@heavygee
heavygee force-pushed the feat/session-attached-jobs branch from 742f102 to 4510e2e Compare August 10, 2026 03:05

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread cli/src/agy/agyPty.ts Outdated
busyMarkers: AGY_BUSY_MARKERS,
idleReadyMs: AGY_IDLE_READY_MS,
idleMarkers: AGY_IDLE_MARKERS,
thinkingSilenceTimeoutMs: null,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +95 to +97
WHERE session_id = ? AND status = 'running'
ORDER BY started_at ASC, job_key ASC
LIMIT 1`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread hub/src/sync/sessionCache.ts Outdated
Comment on lines +1097 to +1099
const targetWasRedirectedToSource =
this.resolveAttachedJobSessionId(toSessionId, namespace) === fromSessionId
const movedJobs = this.store.sessionJobs.transfer(fromSessionId, toSessionId)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@heavygee
heavygee force-pushed the feat/session-attached-jobs branch from 65eb544 to c751690 Compare August 11, 2026 03:04

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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);

ghost Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +106 to +110
await setSessionJob({
...clientOpts,
jobKey: options.jobKey,
body
})

ghost Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +157 to +161
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 } : {}),

ghost Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +198 to +201
} catch (error) {
const message = error instanceof Error ? error.message : String(error)
console.error(`[hapi job run] failed to mark job ${terminalStatus}: ${message}`)
}

ghost Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

HeavyGee and others added 27 commits September 9, 2026 03:06
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>
@heavygee
heavygee force-pushed the feat/session-attached-jobs branch from 741272d to b625adc Compare September 9, 2026 03:07
Rebase left conflict markers in the pin-mode/appBadge locale block;
typecheck fails with TS1185.

Co-authored-by: Cursor <cursoragent@cursor.com>

ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread web/src/App.tsx
} = useSessions(api, { enabled: appBadgeEnabled })
useAppBadge({
enabled: appBadgeEnabled,
scope: baseUrl,

ghost Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +241 to +245
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;

ghost Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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);

ghost Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cold-review-clean Fork-side bot review is satisfactory; safe to promote to upstream PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants