Skip to content

🤖 fix: keep stopped sub-agents stopped after restart - #4172

Merged
ammario merged 12 commits into
mainfrom
fix-stopped-task-startup
Sep 9, 2026
Merged

ammario merged 12 commits into
mainfrom
fix-stopped-task-startup

Conversation

@ammar-agent

@ammar-agent ammar-agent commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Keep stopped sub-agents stopped across application restart and chat replay, preventing false startup activity and local-concurrency warning flashes. This is the lower PR in a stack; #4173 contains the loading-UI cleanup.

Implementation

  • Reuse the initialization metadata snapshot through identity admission and retry derivation rather than repeatedly scanning all workspaces.
  • Probe through shared transient-session ownership, retaining only real activity or adopted sessions; deferred disposal preserves physical I/O leases without holding startup open.
  • Restore question-blocked guidance per ID into the existing queue without dispatching a stream, preserving dispatch mode and deduplicating against live queued guidance. Applicable Stop/opt-out blockers still win. Live and replayed guidance share acceptance, cancellation, failure cleanup, and background stream handoff.
  • Preference I/O failures retry without manufacturing enabled defaults or poisoning the load cache; newly owed Stop markers survive a failed initial read. Stop settlement cancels only captured guidance IDs, and each normalization contains returned/thrown failures.
  • Before admitting queued work, normalize durably stopped/opted-out inactive children through the existing task-stop lifecycle so they release parallel capacity. Question/read-failure blockers keep their slots; active streams/preparations and live pause semantics remain unchanged. Nested drain wakes stay suppressed until normalization is complete.
  • Give TaskService sole ownership of child startup recovery. Generic chat replay cannot recover children or unknown workspace identities.
  • Honor persisted Stop markers, retry opt-outs, and unanswered questions before restart nudges or completion prompts, including legacy statusless tasks. Scoped Stops remain authoritative until history establishes newer accepted user/delegated intent. Live pause/resume and live retry semantics are unchanged.
  • Use strict partial reads for startup admission and reuse bounded, cancellable startup read retries for transient failures; malformed JSON and schema-invalid partials are ignored rather than retried forever, and other HistoryService callers retain their lenient behavior.
  • Classify admission as stopped, blocked, waiting for a question, interrupted, or idle in one read. Legacy statusless children need an interrupted tail only for a generic nudge; durable compaction and queued guidance keep priority.
  • Bound each complete admission probe, including preference loading, with a cancellable deadline. Timeouts fail closed for that task while startup proceeds; physical read leases stay attached until actual I/O settles.
  • Keep disk-read retry delays independent of provider idleness and refresh candidate identities/statuses after asynchronous inspections and at each task dispatch, preventing startup stalls or rerunning tasks that just completed.
  • Normalize inactive-workflow children before filtering recovery blockers, so cancelled tasks cannot retain active status and consume capacity.
  • Recover compaction follow-ups with durable acceptance awaited but provider execution handed to the existing background path, before generic restart prompts or queued parent guidance, then queue that guidance without losing either payload; contain read/send failures per task instead of aborting app startup.
  • Remove obsolete child-specific startup precedence and duplicate history/error/model-normalization logic.

Validation

  • All 2,381 service/history/startup tests pass across 51 files (~31s); local make static-check passes.
  • Regressions cover stopped task statuses, scoped/unscoped/stale markers, long/unreadable history, real transient and persistent partial-file failures, pending questions, missing identity, inactive workflows, genuine crash recovery, and compaction continuation precedence.
  • Existing virtual-clock coverage verifies bounded retries, recovery-stage checkpoints, shutdown drainage, and manual-successor safety.

Subtractive ledger

P1: 553 production lines added, 556 removed; net −3. Removed the duplicate child recovery path and child-only option precedence, shared the existing read-retry loop and interrupted-tail predicate, reused existing error/model/agent-ID normalization, and shared read plumbing, send settings, and log context without dropping diagnostics; shared per-ID guidance settlement replaces duplicated batch/live cleanup. These retain safety checks while reducing competing recovery paths; no new persisted state. Tests are excluded.

Risks

Startup ownership changes across chat replay and task reconciliation. Admission conservatively preserves Stop/question state when disk reads remain unavailable; explicit resume and live retry remain covered by the service suites.

Pains

Review exposed legacy task states and workflow/recovery interactions, requiring additional disk-backed regressions. Existing retry timing is virtualized in persistent-error tests to keep validation deterministic.


Generated with xum • Model: openai:gpt-6-astra • Thinking: xhigh • Cost: $406.76

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-09T18:27:56.282336Z e52e2a6 Manual request
🔒 Security Review Completed 2026-09-09T18:26:20.417912Z e52e2a6 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

@ammario
ammario added this pull request to stack #4174 September 9, 2026 02:02
@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 4b46c7bd60

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@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: 4b46c7bd60

ℹ️ 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 src/node/services/agentSession.ts Outdated
Comment thread src/node/services/taskService.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/taskService.ts Outdated
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

Addressed all four findings in 52cf430:

  • 3964014297 / PRRT_kwDOPxxmWM6geo1Y: persisted retry opt-outs now block task startup even without an aborted marker; the real persisted opt-out regression covers this path.
  • 3964014301 / PRRT_kwDOPxxmWM6geo1b: pending-compaction failures become per-task Result errors, are counted/logged, and do not abort recovery of other tasks. Added wrapper rejection and failed-task isolation coverage.
  • 3964014309 / PRRT_kwDOPxxmWM6geo1d: accepted synthetic guidance can supersede a scoped old Stop; unaccepted snapshots/notices cannot. Added a durable-history regression.
  • 3964014315 / PRRT_kwDOPxxmWM6geo1h: pending questions in partial or history block restart, including statusless children. Error-preserving partial reads also prevent a real filesystem failure from being mistaken for no question.

All 2,050 focused service/history tests and make static-check pass. Live retry and explicit pause/resume semantics remain unchanged.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 52cf430737

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@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: 52cf430737

ℹ️ 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 src/node/services/taskService.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

Addressed both latest findings:

  • PRRT_kwDOPxxmWM6ge6yg: inactive-workflow normalization now runs over all startup candidates before blocker filtering; the regression keeps child blockers enabled and verifies cancellation still wins.
  • PRRT_kwDOPxxmWM6ge6yj: blocker inspection now shares the existing bounded, cancellable startup-read retry loop. Real filesystem regressions cover transient recovery and persistent fail-closed behavior.

All 2,149 service/history tests and local static checks pass.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 1c20871bd8

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@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: 1c20871bd8

ℹ️ 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 src/node/services/agentSession.ts Outdated
Comment thread src/node/services/taskService.ts Outdated
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

Addressed the latest findings:

  • PRRT_kwDOPxxmWM6gfMvT: malformed partial JSON returns no usable partial rather than permanently blocking recovery; transient filesystem failures still retry and fail closed. Added a real crash-truncated-file regression.
  • PRRT_kwDOPxxmWM6gfMvV: compaction continuation dispatch precedes queued parent guidance; guidance is still sent/queued afterward, including legacy statusless children. Existing descendant and already-streaming gates remain intact. Parameterized regression verifies ordering and guidance consumption.

All 2,152 service/history tests and local static checks pass. Production delta remains subtractive: +175/-180, net -5.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 197308eed9

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

CI hit a WorkflowRunner sandbox null-reference error in an untouched test (expected the deliberately thrown agent error). The exact file passes locally: all 79 tests in 2.75s. Retrying the Unit job without code changes; all other code/build jobs passed.

@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: 197308eed9

ℹ️ 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 src/node/services/agentSession.ts Outdated
Comment thread src/node/services/taskService.ts Outdated
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

Addressed both startup-race findings:

  • PRRT_kwDOPxxmWM6gfczc: blocker reads now use the extracted cancellation-aware delay only, not the stream-idleness coordinator. A regression keeps the provider streaming while a transient read retries and completes.
  • PRRT_kwDOPxxmWM6gfczg: retain inspected IDs, then reload config, candidate statuses, and descendant indexes after inspections. A task completing during inspection is excluded instead of restarted.

All 2,154 service/history tests (~30s) and local static checks pass. Existing cancellation/checkpoint and inactive-workflow queue-drain coverage remains green. Production delta: +188/-191, net -3.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 1b59bb27ec

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@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: 1b59bb27ec

ℹ️ 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 src/node/services/taskService.ts
Comment thread src/node/services/historyService.ts
Comment thread src/node/services/taskService.ts Outdated
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

Addressed all three findings:

  • PRRT_kwDOPxxmWM6gfpMj: one typed admission snapshot distinguishes blocked/interrupted/idle. Generic nudges for statusless children require interrupted-tail evidence; compaction and queued guidance still run first. Regressions cover complete versus partial assistant tails and legacy versus explicit task statuses.
  • PRRT_kwDOPxxmWM6gfpMn: parsed partials now use the existing readable-message guard before normalization. Invalid JSON, null, missing fields, and malformed parts cannot permanently brick recovery.
  • PRRT_kwDOPxxmWM6gfpMp: the entire preference/history probe has a fail-closed deadline. Deadline cancellation stops further reads/backoff, while cached preference and history producers retain physical leases until I/O actually settles. Gated tests cover hung preference, partial, and history reads, including disposal waiting for the real producer.

All 2,170 service/history/shared-helper tests and make static-check pass. Startup read plumbing, send settings, diagnostics, and ID normalization are shared to retain a net-negative production delta (+271/-273, net -2), without dropping diagnostics or safety checks.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: 9e7fb2dd08

ℹ️ 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 src/node/services/taskService.ts
Comment thread src/node/services/taskService.ts
@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 9e7fb2dd08

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

Addressed both findings:

  • PRRT_kwDOPxxmWM6gf23B: startup follow-up dispatch opts into the existing background stream handoff. It still awaits durable acceptance/history persistence, but no longer waits for provider execution. A gated regression proves the continuation is persisted and dispatch returns while provider work remains pending; other compaction callers keep their existing await semantics.
  • PRRT_kwDOPxxmWM6gf23D: both recovery loops share admission that rechecks live task status after desktop/workflow admission. New running/awaiting-report regressions complete a later task during an earlier dispatch and verify it is not restarted.

All 2,173 service/history tests and local static checks pass. Production delta remains +302/-304, net -2.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

@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: b9f2f59f16

ℹ️ 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 src/node/services/taskService.ts
Comment thread src/node/services/agentSession.ts
…dates

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh` • Cost: `$236.73`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=236.73 -->
_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh` • Cost: `$264.85`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=264.85 -->
<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=264.85 -->
Reuse bulk metadata for identity admission and interrupted retry derivation, retaining the snapshot across deferred reruns.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh` • Cost: `$316.14`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=316.14 -->
Restore durable parent guidance behind restarted interactive questions without dispatching a turn. Reuse per-ID queue dedupe and settlement; preserve Stop and opt-out precedence.

Validation: 1083 targeted regression tests; make static-check.
Production ledger: +98/-101, net -3.
Share live/restored guidance background handoff and cancellation settlement. Reuse transient ownership for startup admission, recovery, and empty compaction checks; detach abandoned probes before deferred physical cleanup while preserving adopted sessions.

Validation: 1093 tests passed across five lifecycle suites; make static-check passed.
Production ledger: +83/-83, net zero.
Distinguish durable Stop/opt-out from indeterminate startup blockers and settle stopped child lifecycles before queued work is admitted. Preserve question waits, read-failure blockers, and live work. Suppress redundant nested handle drains until the complete task-stop batch settles.

Simplify stale-starting recovery and reuse the initial queue drain; avoid creating sessions just to clear absent queues.

Validation: 1228 tests passed across six lifecycle suites; make static-check passed.
Production ledger: +52/-50 (net +2); the lower PR remains neutral against its prior -2 ledger.
Fail closed on preference I/O errors, retry boundedly, and discard rejected load caches without losing owed Stop markers or unknown opt-out state. Cancel only captured durable guidance IDs during stop settlement and contain each startup normalization failure independently.

Consolidate preference defaulting and replace manual startup-tail scanning with findLast using identical predicates.

Validation: 1810 tests passed across 38 lifecycle/session files, including tail/goal/compaction; make static-check passed.
Production ledger: +72/-72, net zero.
ammar-agent added a commit that referenced this pull request Sep 9, 2026
## Summary

Record the user preference to use `gh stack` for GitHub-native stacked PRs, keeping every PR linked in the native stack rather than only chaining base branches. `AGENTS.md` points to `docs/AGENTS.md`; the generated built-in skill copy is synchronized.

## Stack

Fourth PR in native stack #4174, directly above #4194: #4172#4173#4194 → this PR. Stack membership is managed with `gh stack link`.

## Validation

- Formatting checked against the resolved `AGENTS.md` target.
- `make static-check` and `git diff --check`.
- No prose-only assertion tests added.

## Subtractive ledger

Production logic: +0 / −0. One documentation line and its generated string mirror; no behavior changes or unrelated subtractions.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh` • Cost: `$459.97`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=459.97 -->
@ammar-agent
ammar-agent force-pushed the fix-stopped-task-startup branch from 59d03f9 to e52e2a6 Compare September 9, 2026 17:49
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Rebased onto fresh main b03d253 with gh stack; current head is e52e2a6. The four-PR native stack is preserved. Local static checks and 2,476 backend/UI regression tests passed on the full rebased stack. Please review this new head, including code and security checks.

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: e52e2a6fd1

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Security review successfully completed on e52e2a6 with no findings. Please retry the code review, whose previous attempt failed in checkout before examining this same published head. No code changes since the successful security review.

@chatgpt-codex-connector

This comment has been minimized.

1 similar comment
@chatgpt-codex-connector

This comment has been minimized.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Retrying the unchanged rebased head after the checkout-service failures. Successful reviews on this stack confirm the published refs are available; please complete code and security review. No source changes or gate bypasses have been made.

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: e52e2a6fd1

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: e52e2a6fd1

ℹ️ 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".

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

Both code and security reviews now successfully approve e52e2a6 with no findings. The eleven earlier missing-ref checkout errors are superseded by these successful reviews and are being marked outdated. All build/test checks pass; rerunning the comments/Required gate after cleanup.

@ammario
ammario merged commit df0a29c into main Sep 9, 2026
49 of 56 checks passed
@ammario
ammario deleted the fix-stopped-task-startup branch September 9, 2026 20:44
ammario pushed a commit that referenced this pull request Sep 9, 2026
## Stack

**Stacked on #4172** (`fix-stopped-task-startup`). Merge the startup fix
first, then retarget this PR to `main`. This diff contains only the
loading UI cleanup.

## Summary

Remove the permanent transcript gutter reserved for the transient
“Loading messages…” badge. Initial uncached history keeps the existing
Vercel-style shimmer skeleton; cached replay keeps real rows visible and
shows an accessible shared shimmer strip over the existing dock edge
without occupying transcript space.

## Implementation

- Replace the spinner badge with a zero-layout-height cached-replay
shimmer strip and delete unconditional transcript bottom padding.
Initial loads reuse the existing full shimmer skeleton.
- Preserve the existing skeleton/reveal gate, sticky composer,
zero-height bottom sentinel, and scroll ownership—no new timing state,
effects, or animation implementation.
- Update stories to assert zero reserved space, stable
dock/message/scroll geometry through catch-up, compact-tail visibility,
active feedback, and read-only replay; pin an initial-loading phone
story.

## Validation

- All 65 hydration/scroll/story-contract tests and all 7 chat geometry
integration tests pass after stacking.
- `make static-check` passes on the integrated stack.
- Three Chromium Storybook plays pass, including composed Pixel metadata
and actual 390px phone bounds. Earlier manager validation also covered a
375px phone. Cached catch-up preserves dock/message/scroll geometry, and
the full initial shimmer stays onscreen with zero reserved padding or
document scroll.

## Subtractive ledger

**P1: 20 production lines added, 23 removed; net −3.** Removed the
separate loading badge and the permanent gutter it required, reusing
shared shimmer for an accessible overlay confined to the existing dock
edge. No unrelated offsets.

## Risks

Removing reserved padding changes settled transcript geometry
intentionally. Regression checks protect bottom anchoring, cached
catch-up stability, and narrow-screen composer visibility.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh`
• Cost: `$236.73`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh
costs=236.73 -->
ammar-agent added a commit that referenced this pull request Sep 9, 2026
## Summary

Make PR readiness use GitHub's complete check rollup instead of the name-deduplicated `gh pr checks` projection, and record the preference for `gh stack` to manage native stacked PRs.

## Background

On #4194, successful `Required` and `Codex Comments` checks from one workflow run hid two still-failing checks with the same names in another run. The previous readiness workflow incorrectly reported success. The new discovery path was tested against that live failure and returns exit 1 with both failing job URLs.

## Implementation

- Share paginated, commit-pinned head/merge check discovery between the readiness waiter and log extractor; retain independent suite results while relying on GitHub's rollup to exclude superseded attempts.
- Fail closed on API errors or incomplete pagination, and keep unknown check states pending.
- Do not infer that a pending Pixel status explains GitHub's `BLOCKED` state. Failure output prioritizes failing checks and identifies their commits.
- Run offline behavioral regressions in CI and update AGENTS.md plus its generated skill copy.

## Stack

Remains directly above #4194 in native stack #4174; #4172 and #4173 have merged.

## Validation

- 19 new PR-check regressions and 12 existing Codex-gate tests passed.
- Shellcheck, `make static-check`, and `git diff --check` passed.
- Live reproduction on #4194 surfaced both hidden failures from run `34385271008`; retrying that exact run subsequently passed.

## Risks

Readiness becomes intentionally stricter: independent failures and unexplained GitHub blockers can no longer be masked by same-name successes or optional Pixel statuses.

## Production ledger

Workflow scripts/config: +100 / −49 (net +51), excluding tests and documentation. Removed repeated state predicates, duplicated discovery, and the unsafe Pixel/BLOCKED shortcut; added complete paginated discovery and validation.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh` • Cost: `$591.32`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=591.32 -->
ammario pushed a commit that referenced this pull request Sep 9, 2026
## Stack

Third PR, stacked on #4173 (`fix-chat-loading-gap`), which is stacked on
#4172. Merge bottom-up; this diff contains only the concurrency-warning
change.

## Summary

Replace changing agent names with a stable compact count: **“N other
local agents running — may interfere.”** Agent activity handoffs
previously switched the selected name and caused random-looking flashes.
Names no longer participate in warning rendering.

## Implementation

- Subscribe to the active count instead of the first active workspace
identity.
- Preserve existing brief handoff protection, same-family suppression,
and project/runtime isolation rules. Scope the held count to sorted
eligible workspace IDs so reordering is harmless and eligibility changes
clear stale warnings.
- Keep the single-row composer decoration and use tabular numerals for
stable count typography. No new timers, effects, animations, or
tooltips.
- Cover multi-agent handoffs and metadata reordering, with desktop and
explicitly pinned phone stories.

## Validation

- All 47 warning/Storybook contract tests pass, including 10 warning
handoff/filtering cases.
- Three Chromium Storybook renders pass: single-agent desktop,
multiple-agent desktop, and fixed-390px phone. Phone checks cover
overflow, tabular numerals, and composer clearance.
- `make static-check` passes on the integrated three-PR stack.

## Subtractive ledger

**P1: 27 production lines added, 33 removed; net −6.** Removed
selected-agent/name resolution and simplified the ChatPane interface.
The existing eligibility and handoff safeguards remain. Tests/stories
are excluded; no unrelated offsets.

## Risks

The warning now reports quantity rather than identity, including for one
other agent. Counts change only with activity/eligibility, not with
whichever agent happens to appear first.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh`
• Cost: `$436.55`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh
costs=436.55 -->
ammar-agent added a commit that referenced this pull request Sep 9, 2026
## Summary

Make PR readiness use GitHub's complete check rollup instead of the name-deduplicated `gh pr checks` projection, and record the preference for `gh stack` to manage native stacked PRs.

## Background

On #4194, successful `Required` and `Codex Comments` checks from one workflow run hid two still-failing checks with the same names in another run. The previous readiness workflow incorrectly reported success. The new discovery path was tested against that live failure and returns exit 1 with both failing job URLs.

## Implementation

- Share paginated, commit-pinned head/merge check discovery between the readiness waiter and log extractor; retain independent suite results while relying on GitHub's rollup to exclude superseded attempts.
- Fail closed on API errors or incomplete pagination, and keep unknown check states pending. Revalidate PR refs and merge/review state before accepting collected checks; a concurrent push or base update returns pending.
- Do not infer that a pending Pixel status explains GitHub's `BLOCKED` state. Failure output prioritizes failing checks and identifies their commits.
- Run offline behavioral regressions in CI and update AGENTS.md plus its generated skill copy.

## Stack

Remains directly above #4194 in native stack #4174; #4172 and #4173 have merged.

## Validation

- 22 new PR-check regressions and 12 existing Codex-gate tests passed.
- Shellcheck, `make static-check`, and `git diff --check` passed.
- Live reproduction on #4194 surfaced both hidden failures from run `34385271008`; retrying that exact run subsequently passed.

## Risks

Readiness becomes intentionally stricter: independent failures and unexplained GitHub blockers can no longer be masked by same-name successes or optional Pixel statuses.

## Production ledger

Workflow scripts/config: +125 / −51 (net +74), excluding tests and documentation. Removed repeated state predicates, duplicated discovery, and the unsafe Pixel/BLOCKED shortcut; added complete paginated discovery and validation.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh` • Cost: `$591.32`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=591.32 -->
ammar-agent added a commit that referenced this pull request Sep 9, 2026
## Summary

Record the user preference to use `gh stack` for GitHub-native stacked PRs, keeping every PR linked in the native stack rather than only chaining base branches. `AGENTS.md` points to `docs/AGENTS.md`; the generated built-in skill copy is synchronized.

## Stack

Fourth PR in native stack #4174, directly above #4194: #4172#4173#4194 → this PR. Stack membership is managed with `gh stack link`.

## Validation

- Formatting checked against the resolved `AGENTS.md` target.
- `make static-check` and `git diff --check`.
- No prose-only assertion tests added.

## Subtractive ledger

Production logic: +0 / −0. One documentation line and its generated string mirror; no behavior changes or unrelated subtractions.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh` • Cost: `$459.97`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=459.97 -->
ammar-agent added a commit that referenced this pull request Sep 9, 2026
## Summary

Make PR readiness use GitHub's complete check rollup instead of the name-deduplicated `gh pr checks` projection, and record the preference for `gh stack` to manage native stacked PRs.

## Background

On #4194, successful `Required` and `Codex Comments` checks from one workflow run hid two still-failing checks with the same names in another run. The previous readiness workflow incorrectly reported success. The new discovery path was tested against that live failure and returns exit 1 with both failing job URLs.

## Implementation

- Share paginated, commit-pinned head/merge check discovery between the readiness waiter and log extractor; retain independent suite results while relying on GitHub's rollup to exclude superseded attempts.
- Fail closed on API errors or incomplete pagination, and keep unknown check states pending.
- Do not infer that a pending Pixel status explains GitHub's `BLOCKED` state. Failure output prioritizes failing checks and identifies their commits.
- Run offline behavioral regressions in CI and update AGENTS.md plus its generated skill copy.

## Stack

Remains directly above #4194 in native stack #4174; #4172 and #4173 have merged.

## Validation

- 19 new PR-check regressions and 12 existing Codex-gate tests passed.
- Shellcheck, `make static-check`, and `git diff --check` passed.
- Live reproduction on #4194 surfaced both hidden failures from run `34385271008`; retrying that exact run subsequently passed.

## Risks

Readiness becomes intentionally stricter: independent failures and unexplained GitHub blockers can no longer be masked by same-name successes or optional Pixel statuses.

## Production ledger

Workflow scripts/config: +100 / −49 (net +51), excluding tests and documentation. Removed repeated state predicates, duplicated discovery, and the unsafe Pixel/BLOCKED shortcut; added complete paginated discovery and validation.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh` • Cost: `$591.32`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=591.32 -->
ammar-agent added a commit that referenced this pull request Sep 9, 2026
## Summary

Make PR readiness use GitHub's complete check rollup instead of the name-deduplicated `gh pr checks` projection, and record the preference for `gh stack` to manage native stacked PRs.

## Background

On #4194, successful `Required` and `Codex Comments` checks from one workflow run hid two still-failing checks with the same names in another run. The previous readiness workflow incorrectly reported success. The new discovery path was tested against that live failure and returns exit 1 with both failing job URLs.

## Implementation

- Share paginated, commit-pinned head/merge check discovery between the readiness waiter and log extractor; retain independent suite results while relying on GitHub's rollup to exclude superseded attempts.
- Fail closed on API errors or incomplete pagination, and keep unknown check states pending. Revalidate PR refs and merge/review state before accepting collected checks; a concurrent push or base update returns pending.
- Do not infer that a pending Pixel status explains GitHub's `BLOCKED` state. Failure output prioritizes failing checks and identifies their commits.
- Run offline behavioral regressions in CI and update AGENTS.md plus its generated skill copy.

## Stack

Remains directly above #4194 in native stack #4174; #4172 and #4173 have merged.

## Validation

- 22 new PR-check regressions and 12 existing Codex-gate tests passed.
- Shellcheck, `make static-check`, and `git diff --check` passed.
- Live reproduction on #4194 surfaced both hidden failures from run `34385271008`; retrying that exact run subsequently passed.

## Risks

Readiness becomes intentionally stricter: independent failures and unexplained GitHub blockers can no longer be masked by same-name successes or optional Pixel statuses.

## Production ledger

Workflow scripts/config: +125 / −51 (net +74), excluding tests and documentation. Removed repeated state predicates, duplicated discovery, and the unsafe Pixel/BLOCKED shortcut; added complete paginated discovery and validation.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh` • Cost: `$591.32`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh costs=591.32 -->
ammario pushed a commit that referenced this pull request Sep 10, 2026
…4196)

## Summary

Make PR readiness use GitHub's complete check rollup instead of the
name-deduplicated `gh pr checks` projection, and record the preference
for `gh stack` to manage native stacked PRs.

## Background

On #4194, successful `Required` and `Codex Comments` checks from one
workflow run hid two still-failing checks with the same names in another
run. The previous readiness workflow incorrectly reported success. The
new discovery path was tested against that live failure and returns exit
1 with both failing job URLs.

## Implementation

- Share paginated, commit-pinned head/merge check discovery between the
readiness waiter and log extractor; retain independent suite results
while relying on GitHub's rollup to exclude superseded attempts.
- Fail closed on API errors or incomplete pagination, and keep unknown
check states pending. Revalidate PR refs and merge/review state before
accepting collected checks; a concurrent push or base update returns
pending.
- Do not infer that a pending Pixel status explains GitHub's `BLOCKED`
state. Failure output prioritizes failing checks and identifies their
commits.
- Run offline behavioral regressions in CI and update AGENTS.md plus its
generated skill copy.

## Stack

Only remaining open PR in native stack #4174, now based on main after
#4172, #4173, and #4194 merged.

## Validation

- 22 new PR-check regressions and 12 existing Codex-gate tests passed.
- Shellcheck, `make static-check`, and `git diff --check` passed.
- Live reproduction on #4194 surfaced both hidden failures from run
`34385271008`; retrying that exact run subsequently passed.

## Risks

Readiness becomes intentionally stricter: independent failures and
unexplained GitHub blockers can no longer be masked by same-name
successes or optional Pixel statuses.

## Production ledger

Workflow scripts/config: +125 / −51 (net +74), excluding tests and
documentation. Removed repeated state predicates, duplicated discovery,
and the unsafe Pixel/BLOCKED shortcut; added complete paginated
discovery and validation.

---

_Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `xhigh`
• Cost: `$591.32`_

<!-- mux-attribution: model=openai:gpt-6-astra thinking=xhigh
costs=591.32 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants