Repository navigation
fix: complete consolidated cleanup audit - #170
Closed
looptroop-ai wants to merge 12 commits into
Closed
looptroop-ai wants to merge 12 commits into
looptroop-ai wants to merge 12 commits into
Conversation
…alog state Resolve Windows executable siblings through PATHEXT before trust checks, preserve safe arguments across command wrappers, and recognize verified user-namespace mappings. Regenerate both installers from the accepted resolver source. Recover installer locks immediately only for confirmed dead owners, release owned locks on interruption, and refuse unsafe replacement when a damaged binary cannot establish daemon state. Reject empty PowerShell values and restore verified Linux Node guidance. Preserve YAML payload text through formatting repairs, enforce canonical alias priority without changing distinct-field precedence, retain accurate repair warnings, and guard cache invalidation with complete parser-source hashes. Reconcile asynchronous modal history, correct overlay Escape and focus behavior, and release mobile-drawer containment after resizing or losing ticket data. Update app documentation, audit dispositions and the Unreleased changelog; published website changes are reviewed separately. Each packet passed an independent Astra review with focused regression checks. Full repository integration and packaging checks remain scheduled after the remaining audit packets; native Windows and end-to-end checks are not claimed.
Install container dependencies from the release lockfile while retaining the npm tarball runtime layout. Record installed image package versions and attest the finished multi-architecture digest in isolated jobs with explicit registry login. Keep attestation permissions away from dependency builds, limit channel credentials, validate manifest asset names, and isolate Renovate notice artifacts. Add a real affected-file Windows gate and bounded npm fetch retries. Use structured doctor facts in package smokes, strict release-script argument parsing, independent install-catalog fixtures, and accurate npm failure classification. Update contributor guidance, unreleased notes, and audit evidence. This is an integration checkpoint; part-specific PR branches are assembled separately. Native Windows and Docker execution remain CI verification limits.
Use awaited bounded Git mutations with descendant cleanup, opaque NUL-delimited paths, safe refs, generated-file exclusions, staged-deleted handling, and expected-head leases for remote branch deletion. Preserve caller SSH and credential-helper configuration. Validate YAML recovery proofs and complete JSONL rewrites, retain uncertain file generations, handle exact short writes and contained paths, and block startup before hydration when interrupted fallback ownership cannot be established. Use native SQLite transactions for persistent Manual QA evidence locks and handle final symlinks without following external targets. Update app and published-doc companion prose plus workflow descriptions and Details. Distinguish harmless orphan warnings, process-level recovery blocking, transient cleanup and worktree deletion. Checkpoint only independently accepted source snapshots; newer bead and CLI edits remain unstaged. Verification: independent Git 229 tests across 18 files plus earlier 174 downstream checks; durable IO 198 passed with 4 platform skips; metadata 44 tests; website 87 tests/build/site/license checks; scoped lint, types and diff checks. Native Windows, physical power loss, full lifecycle and final part-wide verification remain outstanding. No part2 PR is opened by this checkpoint.
… drafts Checkpoint independently reviewed authoritative bead reads, structured command producers and approval validation, dependency inverse and cycle checks, source-line diagnostics, immutable original draft conflicts, and accessible damaged-tracker UI. Preserve unknown metadata while preventing malformed trackers from reporting false completion or being silently rewritten. Carry forward the already reviewed release-part native SEA runtime and launch diagnostics without mixing active process or remote-session edits. Update workflow descriptions and details, application documentation, changelog and audit evidence; published website prose is committed separately. Stage accepted bead snapshot blobs for shared paths so newer in-progress OpenCode changes remain untouched. Full isolated part verification and individual PR delivery remain separate gates.
…ries Checkpoint independently reviewed cleanup revalidation, successor-safe daemon-state removal under the existing daemon lock, concurrent-start loser protection, captured process identity across executor and supervisor termination, and guarded group escalation. Preserve Git descendant cleanup while refusing missing or recycled process identities. Keep log-follow handoff bytes, incomplete UTF-8 and partial lines intact; reset decoding on rotation, bracket IPv6 origins, redact Windows profile paths and serialize runtime restart ordering. Include focused race regressions and the previously reviewed canonical log test fixture from PR164. Update application guidance, changelog and audit records; publish matching upcoming-release website prose separately. Accepted snapshot staging leaves active OpenCode and interview edits untouched. Hooks and isolated part-three verification remain pending.
Keep resolved question identities suppressed across stale snapshots and fence responses across ticket removal and re-addition. Stabilize question polling, refresh all ticket query families on initial cursor recovery or replay gaps, and retain lightweight ordinary reconnect behavior. Bound shared session-auth probes to five seconds without treating network failures as sign-out. Retry model discovery only for the explicit OpenCode startup response. Add actual-hook race, timeout and retry coverage, with matching changelog and audit dispositions. Fresh source review passed 77 tests, lint, both TypeScript projects and diff checks. Documentation was independently checked against the frozen source; website guidance is published separately with next-release labels. Remaining workflow and log-history work is excluded from this checkpoint.
…integrity Require confirmed remote stops before abandonment, restart, cancellation and cleanup; retain uncertain sessions through durable markers where storage permits. Scope session context and question handling to canonical project ownership, bound finite continuations, and preserve zero as unlimited. Fence interview generation, edits and rollback with durable claims and latest-row content checks; validate batch identity and timestamps, reclaim expired or confirmed-dead ordinary claims, and keep pending-stop ownership non-expiring until confirmed. Update status descriptions and Details, maintainer guidance, changelog and audit dispositions. Integrates only independently reviewed OpenCode and interview-core snapshots; newer approval-save and log-history revisions remain unstaged. Focused regression suites, application/test types, lint and documentation checks passed; native-platform and full lifecycle validation remain manual.
Keep temporary OpenCode step caps out of bead and squash commits while a valid restore marker exists, including app-created configs edited during execution. Preserve conflicting bytes across reset refusal, persist contained hook-validation recovery snapshots, and record bead HEAD before publishing in_progress. Require original content hashes for PRD and interview edit saves. Fence post-approval restart work with the exact durable claim before and after awaited session stopping so expired writers cannot invalidate successor artifacts. Coordinate normal and keepalive UI-state saves with shared generation guards. Preserve failed local drafts through completed refetches, learn server revisions before queued saves start, and acknowledge only the current saved snapshot. Update repository guidance, changelog, status descriptions and Details with conditional recovery and best-effort flush limits. Website documentation is committed separately on its main branch. Integrate only independently reviewed frozen hooks10, saves21 and docs6 manifests. Preserve active workflow, log and form edits outside this checkpoint. Scoped tests and independent race/recovery probes passed; full isolated part checks and PR acceptance remain pending. No E2E or lifecycle tests.
Preserve merge and Manual QA ownership, validate skip receipts, expose supported recovery actions, and retain submitted snapshots across later drafts. Bound log paging work without limiting complete user-requested history or exports, preserve server ordering, and reject expired cursors explicitly. Track meaningful unsaved form changes, protect drafts across asynchronous saves and hydration, improve accessible controls, validate ticket payloads, and fence deleted-ticket caches against late requests and SSE events. Update status descriptions, detailed guidance, changelog, and the audit ledger alongside reviewed published documentation. Integrates 79 explicit files from independently accepted workflow, log, forms, cache, and documentation snapshots. Credential filtering and lint/security work remain separate and unstaged. Full isolated part checks and part-specific PR assembly follow; this integration branch is not a PR.
Validate cookie origins against the actual request scheme and canonical authority, retain explicit development origins, reserve SSE admission before asynchronous work, and validate Manual QA action identifiers. Filter internal daemon credentials after child environment merges while preserving intentional provider credentials and trusted daemon handoffs. Enforce operation-scoped filesystem and process-launch lint boundaries using existing helpers. Update security documentation and audit dispositions; isolate database-backed interview batch tests and use the existing React Query re-export for historical log cancellation. Exact combined tree passed 6458 tests with 13 skips, lint, both TypeScript projects, production/package/version/install/license checks, actionlint and ShellCheck, native Linux Node 26.9.0 SEA build, and amd64 container build/version/help. Fresh independent review verified all 31 overlay paths and ran 129 tests with one platform skip. Remaining part PR delivery is tracked separately; no lifecycle, E2E, CI, merge, or unsupported-platform acceptance claimed.
Record the exact open PR heads and dependency bases, the accepted 154-finding source checkpoint, and combined local verification results. Mark earlier pending delivery counts as historical while preserving native-platform, lifecycle, publication, and CI limitations. Clarify local Host versus Origin parsing scope without changing runtime behavior. This reporting-only update passed 19 documentation tests, exact commit and link checks, whitespace and NUL checks, and a fresh independent review. All seven PRs remain unmerged; CI acceptance is reserved for the owner.
Route the second positional token help or ? through the existing per-command --help path so all three spellings return identical output without running the command. Update the built-in usage text and Unreleased changelog, and add a regression covering all aliases. Top-level --help and unknown-command handling remain unchanged.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedToo many files! This PR contains 359 files, which is 259 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (359)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR successfully implements support for help and ? as aliases for per-command CLI help alongside --help. The implementation is clean, well-tested, and follows existing patterns in the codebase.
Key Changes Reviewed
- CLI Help Aliases: Added support for
looptroop <command> helpandlooptroop <command> ?in addition to--help - Test Coverage: Comprehensive test added to verify all three aliases produce identical output
- Documentation: Updated usage text and comments to reflect the new aliases
Assessment
The changes are minimal, focused, and correct. The implementation properly handles the help aliases at the right point in the CLI argument parsing flow, and the test coverage ensures all three forms (--help, help, ?) work identically. No security, logic, or stability issues were identified in the changes.
With 360 files changed in this consolidated audit PR, I focused my review on the CLI changes mentioned in the PR description and found them to be well-executed. The broader cleanup and audit changes appear to be part of a systematic code quality improvement effort.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 9 medium 7 high |
| Security | 10 critical 15 high |
🟢 Metrics 3541 complexity · 457 duplication
Metric Results Complexity 3541 Duplication 457
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const descendant = [ | ||
| 'const fs = require("node:fs")', | ||
| 'process.on("SIGTERM", () => {})', | ||
| `setTimeout(() => fs.writeFileSync(${JSON.stringify(marker)}, 'survived'), 1000)`, |
PR Summary by QodoComplete consolidated reliability, security, and delivery audit
AI Description
Diagram
High-Level Assessment
Files changed (360)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3037800eaf
ℹ️ 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
+323
to
+326
| if (KNOWN_TICKET_ROOT_ARTIFACTS.has(artifactPath)) return true | ||
| if (/^meta\/(?:ticket\.meta|manual-qa-origin)\.json$/.test(artifactPath)) return true | ||
| if (/^beads\/.+\/\.beads\/issues\.jsonl$/.test(artifactPath)) return true | ||
| if (/^runtime\/(?:owner\.json|state\.yaml|execution-setup-profile\.json|execution-log(?:\.debug|\.ai)?\.jsonl)$/.test(artifactPath)) return true |
There was a problem hiding this comment.
When the process crashes after safeAtomicWrite finishes a temp file but before rename, this allowlist excludes both runtime/opencode-pending-sessions.json and the root-level opencode-steps-restore.json. Startup therefore leaves those valid temps untouched: the first can make a remotely created OpenCode session undiscoverable after its database insert failed, while the second can strand a project opencode.json with LoopTroop's temporary step cap and lose the saved original configuration. Add these known LoopTroop artifacts to the ticket recovery allowlist.
Useful? React with 👍 / 👎.
Comment on lines
+835
to
+837
| } else { | ||
| allConfirmed = false | ||
| console.warn(`[sessionManager] Could not confirm abort for OpenCode session ${session.sessionId}; retaining its active row`) |
There was a problem hiding this comment.
When an active database row outlives its OpenCode session, OpenCodeSDKAdapter.abortSession returns false because directory resolution or the abort request reports the session missing. This branch retains the active row, and cancellation/deletion now requires abortTicketSessions to return true, so every retry is rejected until the daemon is restarted and startup reconciliation abandons the row. Confirm remote absence before aborting, or otherwise treat a definitive not-found response as successful termination.
Useful? React with 👍 / 👎.
Comment on lines
+44
to
+48
| database = new Database(lockPath) | ||
| // A native busy wait would block this process's event loop. Failed | ||
| // attempts return immediately and the caller yields between retries. | ||
| database.pragma('busy_timeout = 0') | ||
| database.exec('BEGIN IMMEDIATE') |
There was a problem hiding this comment.
After upgrading from the immediately preceding file-lock implementation, a process crash can leave index.json.lock containing the old JSON ownership record. Opening that path as SQLite and issuing the pragma fails with SQLITE_NOTADB, which is not classified as busy and is never cleaned up, so all later Manual QA evidence-index mutations remain broken. Detect and safely replace the legacy lock format when no old owner is live, or use a new database filename.
Useful? React with 👍 / 👎.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+180
to
+182
| result = saveApprovedPrdDocument(ticketId, document, rawParsed.data.expectedContentSha256!, { skipInvalidation: true }) | ||
| restart = await preparePlanningRestart(ticketId, 'WAITING_PRD_APPROVAL', planningLock) | ||
| assertPlanningEditClaim(ticketId, planningLock) |
There was a problem hiding this comment.
For a post-approval PRD edit, this writes the new authoritative document before preparePlanningRestart() confirms that active sessions stopped and the caller still owns the planning claim. If session shutdown fails or the claim is lost during that await, the endpoint returns an error but the new PRD remains stored while the previous downstream beads and artifacts remain valid. This can pair a new planning input with outputs generated from the old input. The interview save path has the same ordering in server/routes/ticketHandlers/interviewHandlers.ts.
Knowledge Base Used: Ticket state and durable storage
Code Review by Qodo
1. Evidence updates fail after upgrade
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+254
to
+256
| if (!sessionsStopped || !windowsCleared) { | ||
| console.warn(`[workflow] Could not confirm cancellation cleanup for ticket ${ticketId}; retaining remote-session state`) | ||
| return |
There was a problem hiding this comment.
3. Canceled tickets retain stale state 🐞 Bug ☼ Reliability
attachWorkflowRunner removes cancellationCleanupInFlight after an unconfirmed or failed cleanup without retaining or scheduling another attempt. When recovery starts from an already canceled ticket, a transient session-abort or window-clear failure therefore leaves its remote-session and question bookkeeping indefinitely because later cancel requests reject terminal tickets.
Agent Prompt
### Issue description
Cancellation recovery abandons cleanup after one transient failure, and an already canceled ticket cannot be canceled again to trigger another attempt.
### Fix Focus Areas
- server/workflow/runner.ts[247-265]
- server/routes/ticketHandlers/lifecycleHandlers.ts[402-410]
- server/workflow/phases/state.ts[103-113]
### Recommended Fix
Keep failed cancellation cleanup pending and retry it with bounded backoff until session abort and question-window cleanup are confirmed. Preserve the in-flight deduplication guard while a retry series is active, and only call `cleanupTicketState` and clear the pending record after confirmed success.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1695
to
+1697
| if (planBeads.length === expandedBeads.length) { | ||
| return expandedBeads.map((expandedBead, index) => ({ planBead: planBeads[index], expandedBead })) | ||
| } |
There was a problem hiding this comment.
4. Expansion diffs compare wrong beads 🐞 Bug ≡ Correctness
pairExpansionBeads pairs equal-length parsed arrays by index even though each artifact parser independently drops malformed rows. If different source positions are dropped while the surviving arrays retain equal lengths, the approval view compares unrelated beads and reports incorrect added fields and identifiers.
Agent Prompt
### Issue description
Independent parser filtering can shift equal-length plan and expansion arrays, making positional pairing compare unrelated beads.
### Fix Focus Areas
- src/components/workspace/ArtifactContentViewer.tsx[1694-1703]
- src/lib/beadsDocument.ts[472-474]
- src/lib/beadsDocument.ts[490-514]
### Recommended Fix
Preserve each bead's original source-row position while parsing and pair renamed expansion beads using those positions, leaving gaps for malformed rows rather than compacting both arrays independently. Use exact identifier matches where available and do not infer positional alignment solely from equal filtered lengths.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1330
to
+1333
| const modelSql = `SELECT DISTINCT model_id | ||
| FROM execution_log_projection AS p | ||
| WHERE ${clauses.join(' AND ')} AND ${DEBUG_VISIBLE_ROWS} AND model_id IS NOT NULL | ||
| ORDER BY model_id` |
There was a problem hiding this comment.
5. Native models disappear from filters 🐞 Bug ≡ Correctness
queryDebugPage builds modelIds exclusively from execution_log_projection even though the page and totals also include native OpenCode rows. A model represented only in native history consequently appears in the debug log but cannot be selected through the frontend model filter.
Agent Prompt
### Issue description
The debug model catalog excludes model identifiers stored only in the native-log index, so the UI cannot offer filters for all displayed rows.
### Fix Focus Areas
- server/log/projection.ts[1326-1339]
- server/log/projection.ts[1353-1375]
- src/hooks/useTicketHistoricalLogs.ts[345-355]
### Recommended Fix
Query distinct model identifiers from the active native snapshot using the same ticket and scope filters, merge them with projection model identifiers, deduplicate them, and return a deterministically sorted catalog on the first page.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+442
to
+445
| const authoritative = options.prune !== false && !containsResolvedRequest | ||
| if (authoritative) { | ||
| pruneTicketRequests(setRequests, ticketId, live) | ||
| applyTimer(ticketId, timer) |
There was a problem hiding this comment.
6. Other resolved questions remain visible 🐞 Bug ≡ Correctness
applySnapshot marks an entire successful snapshot non-authoritative when it contains even one tombstoned request, suppressing pruning for every other request on that ticket. If stale resolved request A remains in the server response while live request B has disappeared, B stays visible and A's tombstone is not cleared until a later response finally omits A.
Agent Prompt
### Issue description
One tombstoned request globally disables authoritative processing of an otherwise successful ticket snapshot, retaining unrelated questions that the server omitted.
### Fix Focus Areas
- src/context/AIQuestionContext.tsx[420-446]
- src/context/AIQuestionContext.tsx[157-168]
### Recommended Fix
Continue suppressing individual tombstoned requests during upsert, but still prune locally displayed requests absent from a successful authoritative snapshot and clear only tombstones whose keys are absent. Do not make the whole snapshot non-authoritative merely because one returned key is tombstoned.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| rmSync(lockPath, { force: true }) | ||
| } catch { | ||
| // Gone already, or unreadable; the next acquire attempt decides. | ||
| database = new Database(lockPath) |
There was a problem hiding this comment.
1. Evidence updates fail after upgrade 🐞 Bug ☼ Reliability
tryAcquire now opens the existing index.json.lock as a SQLite database and rethrows errors that are not lock contention. Installations that retain the JSON lock file written by the previous implementation therefore receive a “file is not a database” error whenever the manual-QA evidence index reaches withFileLock.
Agent Prompt
## Issue description
Existing JSON lock files are incompatible with the new SQLite-backed lock implementation, so upgrades can permanently block evidence-index updates.
## Fix Focus Areas
- server/io/fileLock.ts[40-53]
- server/phases/manualQa/storage.ts[442-459]
## Recommended Fix
Before opening a lock path as SQLite, detect the legacy JSON lock format and safely remove or migrate it only when it is confirmed to be an old abandoned lock. Preserve normal SQLite corruption failures rather than treating arbitrary malformed files as removable.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+173
to
+176
| const current = readPendingSessionOwnership(ticketId) | ||
| if (!current.readable) return false | ||
| if (current.records.some(candidate => candidate.sessionId === record.sessionId)) return true | ||
| return writePendingSessionOwnership(ticketId, [...current.records, record]) |
There was a problem hiding this comment.
2. A coding session escapes cleanup 🐞 Bug ☼ Reliability
rememberPendingSessionOwnership performs an unlocked read-append-write of the complete pending-session marker file. When two processes persist fallback ownership for the same ticket, the later write can overwrite the earlier record, so ticket cleanup and restart recovery cannot find that remote session.
Agent Prompt
## Issue description
Concurrent fallback ownership writes can overwrite one another, losing a live remote session from durable cleanup and restart recovery.
## Fix Focus Areas
- server/opencode/sessionManager.ts[156-184]
- server/opencode/sessionManager.ts[405-500]
- server/opencode/sessionManager.ts[748-892]
## Recommended Fix
Protect reads and writes of the pending-session ownership artifact with a cross-process lock or an atomic compare-and-retry merge. Apply the same serialization to removal so an older cleanup cannot overwrite a concurrent addition.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+466
to
+470
| // `terminateProcessTree` signals the negative pid on POSIX. A detached | ||
| // child starts its own process group, so Git hooks, filters, and their | ||
| // descendants receive the same timeout signal instead of surviving | ||
| // after the runner has returned. | ||
| detached: process.platform !== 'win32', |
There was a problem hiding this comment.
7. Timeout can kill unrelated processes 🐞 Bug ☼ Reliability
runAsyncRaw starts a detached process group but retains it across timeout, output-limit, and delayed force-kill callbacks without capturing and passing the process start token that terminateProcessTree needs to verify the leader’s identity. If Git exits and its PID or group is reused before cleanup runs, the negative-PID signal can reach an unrelated replacement process or process group.
Agent Prompt
## Issue description
Asynchronous Git termination retains only the detached child PID and does not confirm that it still identifies the spawned process, so delayed overrun, timeout, or force-kill cleanup can signal a recycled PID and an unrelated process or process group.
## Fix Focus Areas
- server/git/runCommand.ts[460-471]
- server/git/runCommand.ts[499-502]
- server/git/runCommand.ts[551-562]
- server/lib/processTree.ts[101-149]
## Recommended Fix
Capture the child process start token immediately after spawning Git and pass it through a shared, guarded process-tree escalation path for every output-limit overrun, timeout, and delayed force-kill. Skip destructive group signals and report the command as abandoned if identity capture fails or later verification shows that the leader identity changed, while retaining safe descendant cleanup when the original process group remains proven.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+288
to
+292
| const pid = child.pid | ||
| // Capture the identity at spawn time. A pid alone can be recycled while a | ||
| // health wait or shutdown is in progress, so every later termination check | ||
| // must compare against this original token rather than infer one late. | ||
| const startToken = pid === undefined ? null : readProcessStartToken(pid) |
There was a problem hiding this comment.
8. Opencode server survives shutdown 🐞 Bug ☼ Reliability
spawnAndWait stores a managed child even when its one-time readProcessStartToken call returns null. The new guarded termination methods refuse both graceful and forced termination for a null token, so a live child whose initial identity lookup transiently fails remains running and can retain the service port.
Agent Prompt
## Issue description
A managed OpenCode child with no captured process identity cannot be terminated by the new fail-closed shutdown path.
## Fix Focus Areas
- server/opencode/supervisor.ts[75-108]
- server/opencode/supervisor.ts[288-311]
- server/opencode/supervisor.ts[434-463]
## Recommended Fix
Retry identity capture before registering the child as managed, and fail the launch with safe owned-child cleanup if no token can be obtained. Do not retain a managed child whose shutdown path is guaranteed to refuse termination.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Owner
Author
|
PR #170 currently has merge conflicts and several failing checks that stem from branch history:
|
looptroop-ai
force-pushed
the
fix/consolidated-cleanup-audit
branch
2 times, most recently
from
September 20, 2026 13:27
3037800 to
d0b4c2f
Compare
CI failed: 5 test failures in server/io/__tests__/atomicIO.test.ts due to macOS symlink path resolution differences with `/var/folders`.Overview5 test failures were found in server/io/tests/atomicIO.test.ts across 2 jobs, all sharing the same root cause related to macOS symlink resolution for temporary directories. FailuresrecoverOrphanTmpFiles Path Resolution Failures (confidence: high)
Summary
Code Review ✅ Approved🔴 High risk Completes consolidated cleanup, recovery, security, and delivery audit across multiple subsystems including CLI, workflow, client state, and filesystem operations. Adds Review coverageRules No rules evaluated Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Important Your trial ends in 4 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more. Was this helpful? React with 👍 / 👎 | Gitar |
|
Owner
Author
|
Superseded by the clean, main-based CLI-only PR #171. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
helpand?as aliases for per-command CLI help alongside--help.Verification
npm exec -- vitest run --project server-integration server/cli/__tests__/cliUpdate.test.tsnpm run build:servernpm exec -- eslint server/cli/cli.ts server/cli/__tests__/cliUpdate.test.tsnpm run verify:site