fix(copilot): use stable kernel identity for foreground ownership by YuuuXie · Pull Request #2293 · devswha/ovis

YuuuXie · 2026-10-28T21:55:46Z

Problem

Copilot's process-local foreground cache used processStartedAt() as a lifetime
identity. On WSL, a clock correction can change /proc/stat's btime while the
PID and /proc/PID/stat start ticks stay unchanged. The resulting epoch start
time changes even though the same process is still running.

That invalidates the foreground checkpoint and its denial history. Besides
discarding a valid binding, it can incorrectly make report/marker bootstrap
eligible again after a registration was revoked or continuity was lost.

Fix

Reuse the existing readProcessBirth() helper: boot identity plus kernel start
ticks, not an epoch estimate.
Use the same identity for normal lookup and cache-capacity cleanup.
An unknown birth still fails closed without discarding denial state; a
genuinely different birth resets the cache.
Leave processStartedAt() and every chronological caller unchanged.

Evidence

Three focused regressions fail on the original implementation and pass with
this fix: preserved binding generation across epoch corrections, preserved
denial across those corrections, and recognizing a genuinely different birth.
The existing failed-birth handoff regression now exercises the reused helper.

Copilot unit/contract and process-owner suites: 96 tests, 1,011 assertions.
Typecheck passed.
Independent code/results review found no substantive issues.

An initial full fast run had 8,382 passing tests and one failure in the unchanged
installer's wall-clock duration assertion (18,532 ms versus a 19,000 ms minimum).
The installer/test blobs exactly match the previously instrumented baseline
that demonstrated a backward wall-clock step during a real 20-second wait.
No assertion, timeout, clock, permission or trust setting was changed.

A second, clock-monitored full run also finished with 8,382 passes, one failure
and 34 skips. The installer passed this time; the sole failure moved to the
unchanged browser-lane summary test (scripts/ci-browser.test.ts:111, one
expected script row missing). That harness passed separately afterward
(8 tests, 46 assertions). The monitor recorded 13 backward wall-clock steps
of approximately 1.5 seconds, but the cause of the missing summary row was not
captured. Neither full run is claimed as a successful full-fast result.

This fixes a demonstrated cache-identity defect. It does not claim that clock
correction was proven to cause a previously observed intermittent Chat fallback.

Follow-up verification

The handoff contract's missing-birth probe now mocks readProcessBirth(), so
it continues to exercise the helper actually used by this fix. The final clean
414ae278 focused run used bun run check run isolation: 96 passed, 1,011
assertions. Independent follow-up review found no substantive issues.

The final full-fast attempt on that unchanged, clean head again finished with
8,382 passes, one failure and 34 skips. Only the same unchanged installer
duration assertion failed (18,494 ms versus 19,000 ms); workflow syntax,
generated-file freshness, typecheck and build passed. Its report confirms
unchanged source, but correctly marks the overall verification unsuccessful.

The original published head's native GitHub Fast checks, Integration tests,
Windows install and macOS identity checks passed. Its browser UI-regression
lane failed waiting for a terminal resize frame in the unmodified
scripts/chat-size-regression.ts:1203 scenario; that scenario creates a shell
pane, not a Copilot agent. The browser failure's cause is not established, and
the upstream browser gate is not claimed green.

Reuse kernel process births for foreground cache lookup and capacity cleanup, preserving denial state when process birth is unknown. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512

coderabbitai · 2026-10-28T21:56:12Z

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: devswha/ovis
Branch: main
File: .coderabbit.yaml
Commit: 72f2edf
Layers: Repository YAML
Review profile: CHILL
Plan: Advanced
Run ID: 0c8d6357-ac16-4093-afbe-90cefec89948

Commits

Reviewing files that changed from the base of the PR and between 72f2edf and d619ea0.

Files selected for processing: 3

changelog.d/copilot-stable-process-identity.md
server/copilot-store.ts
server/copilot.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Summary

Summary by CodeRabbit

Bug Fixes
- Copilot conversation ownership now remains stable when system clock corrections affect process start times.
- Previously denied fallback access is no longer re-enabled by clock corrections.
- Foreground log scanning is unavailable when the process identity cannot be verified or the platform is unsupported.
- Process tracking now retains entries when identity cannot be determined, reducing the chance of losing valid ownership information.

Copilot foreground ownership now uses kernel process birth identity. Foreground log scanning is unavailable outside Linux or when the process birth is unknown. Capacity cleanup retains entries with unknown identity and removes entries for absent or differently identified processes.

Changes: Copilot ownership tracking

Layer / File(s)	Summary
Stable process-birth ownership `server/copilot-store.ts`, `server/copilot.test.ts`, `changelog.d/copilot-stable-process-identity.md`	The store uses identified process births for foreground tracking and capacity cleanup. Linux-only tests cover clock corrections, unknown identity, and process replacement. The changelog describes the ownership change.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

Issue 2184 — The PR addresses the Copilot foreground-cache process-identity failure described in the issue. The issue’s plugin lifecycle changes remain outside this PR.

Possibly related PRs

devswha/ovis#2283: Both changes update process-identity evidence used by Copilot ownership tracking.
devswha/ovis#1955: Both changes concern process and lifecycle evidence used for Copilot handoffs.
devswha/ovis#1652: This PR changes the ownership implementation used for native Copilot transcript selection.

Merge Risk: Minimal · up to d619e

No merge-blocking regression was established. The reported full fast runs did not complete cleanly.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title is concise, specific, and accurately identifies the main change: using stable kernel process identity for Copilot foreground ownership.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description follows the repository template. It has the Change section with the problem, the resulting behavior, and the fix. It also has the Validation section with test counts, typecheck results…

Full details: Description check

Explanation

The description follows the repository template. It has the Change section with the problem, the resulting behavior, and the fix. It also has the Validation section with test counts, typecheck results, and the status of two full fast runs. The description does not link an issue. The template asks for an issue link only for features, and this PR is a fix, so the omission does not fail the check. The description is long and includes more run-by-run detail than the template needs, but all required sections are present and substantive.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

🧪 Generate unit tests (beta)

Create a new PR

🛠️ Fix failing CI checks: 💡

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Keep the existing failed-identity contract probe aligned with the stable process-birth helper used by foreground discovery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512

devswha · 2026-10-28T22:51:51Z

Merging 414ae27 (external, @YuuuXie). The merge pass read the whole diff before approving CI, and a strict reviewer checked it as access-control code; it passes with no MEDIUM or higher. Copilot's per-process ownership cache (and with it each verified binding and its denial history) is now keyed by the kernel's process birth (boot id plus start ticks, readProcessBirth) instead of an epoch start time derived from /proc/stat btime, which moves when the clock is corrected (WSL), so a correction no longer resets a binding or makes a denied fallback eligible again. Verified: a btime change leaves the birth identical while new start ticks or a new boot id change it; a recycled pid gets fresh state and cannot inherit the old binding or handoff; an unreadable birth denies ownership in every mode, and unknown -> original birth does not reopen a revoked registration; the cache stays bounded at 256 (unknown incumbents block admission rather than grow it); non-Linux behaviour is unchanged (Copilot ownership was already Linux-only). Copilot and process-owner tests (91) and typecheck pass. The review's one LOW, a contract test still mocking the old helper, is fixed in 414ae27 (it now mocks readProcessBirth returning unknown); the merge pass read that delta. Thank you, @YuuuXie.

…ize (#2295) Closes #2294. ## Why `scripts/ui-contexts-regression.ts` failed twice per job at `scripts/chat-size-regression.ts:1203` on PRs that do not touch the terminal (#2290 @49a7457c, #2293 @d619ea0f, and two more runs). A scan of every ui-regression job since 2026-10-27 12:00Z puts the first failure at 2026-10-28 18:46Z, after #2272 and #2255 merged. `checkFocusedWindowReclaimsTerminalSize` waited 10 s for an outgoing `resize` frame after the desktop window's attach. Since #2272 a new terminal attaches only once its first font is set (`usePaneAttach.ts`, `fontSettled`), so the attach already carries the final grid. The font effect's earlier resize is dropped by `ws.ts` `resize()` because the pane is not attached yet, and often nothing resizes after the attach. The server sizes the pty from the attach itself (`server/core/terminals.ts`, `ensureAttachment` and `drivePty`). **This is a test race, not a product bug.** Locally the page sent only `attach 121x41` (no `keep_size`), and the pane's shell reported `stty size` = `41 121`. ## What (test-only) - The check records every grid a window asks for: a `resize`, or an `attach` without `keep_size`. It does this through its own init script, so the shared `openRecording` recorder and the five assertions that compare whole attach frames stay unchanged. - It waits, under the same 10 s deadline, until the pane's shell reports the last asked grid. This replaces both the `resize` wait and the separate `sizeBecomes` for the two opening windows. - Changelog fragment under Maintenance. ## Verification - Before: `CHAT_SIZE_CASE=reclaim bun scripts/chat-size-regression.ts` failed 4 of 4 runs locally with the same timeout. - After: the same command passed 3 of 3 runs. The reviewer ran it twice more in an isolated herdr session, and both passed. - The new wait can fail: with the server's attach driving the pty to `cols - 1`, the check fails with `the pane is 41 120, not 41 121`. Skipping `drivePty` on attach still passes, because a first attach creates the pty at the asked grid anyway. - `bun run check fast` is green: typecheck, build, and 8380 unit tests. - Fixed sleeps: the new pause sits in a poll with a deadline, and the file's count stays at 5 (`conventions.test.ts` passes). ## Review The fix is test-only, so this PR had the omo-native-code-reviewer only, per the lane rules. The verdict was approve with notes, with no CRITICAL, HIGH or MEDIUM findings. The reviewer traced the attach path and found no path where the pty keeps a stale size. LOW notes, not changed here: - The new recorder does not filter by `pane_id`. This is harmless while each page opens a single `?pane=`. - A late font face that resizes after the first match would fail a later step as a flake, never as a false pass. The old wait had the same window. - The comment at `src/lib/useTerminalRefits.ts:139` still says the refit runs "right after the attach". It now runs before a deferred first attach, so it is worth rewording separately.

…ading (#2296) (#2297) Closes #2296 ## Problem `server/file-pool.test.ts` hung CI's Fast checks on run [40779586477](https://github.com/devswha/ovis/actions/runs/40779586477) (#2293, which does not touch it): the job was cancelled at its 10-minute limit with this file the only one never reported. The test "keeps a descriptor its thread is reading open until that thread is gone, when the last server stops" holds its file thread with the fake worker's `readGate` (`server/file-worker.fake.ts`, from #2256). That gate said the thread was reading (`Atomics.notify` on cell 1) **before** it opened the FIFO for reading, a blocking `open(2)`. The test then ends the thread (`release()`) and opened the FIFO's writer with a blocking `openSync(fifo, "w")` on its main thread. A thread ended between the notify and its own open never opens the reader, so the writer's open waits forever, and Bun's per-test timeout cannot fire while the main thread is blocked in a system call. #2265's PR body named this gap as a candidate flake. ## Change (test-only) - `readGate` opens the FIFO read-write (an open that never waits) **before** it records the descriptor and notifies, then blocks in a one-byte read of it, as `authorizeGate` (#2265) already does. Once the test sees the thread running, the FIFO always has a reader, even after that thread is ended. - The test releases the read with `O_WRONLY | O_NONBLOCK` and a one-byte write, which never waits (and would fail with `ENXIO` rather than hang if there were no reader). - Both FIFO gates (`readGate`, `authorizeGate`, now one `holdOnFifo`) name their FIFO descriptor in a third gate cell, and the test closes it once the thread is gone: a worker ended inside that read never runs its `finally` on Bun, so before this each run left 2 FIFO descriptors open (found from CodeRabbit's review; `/proc/self/fd` count at the end of the file: 2 -> 0). `server/file-pool.ts` is not changed: the server's thread never waits on a file thread (it posts a task, ends a thread with `terminate()` without awaiting it, and closes descriptors), so the product cannot block this way. Only the tests await `fileThreadsClosed()`. ## Verification - Widened window, scratch only (a 500 ms `Atomics.wait` right after the notify in `readGate`): - on main 72f2edf: `timeout 30 bun test server/file-pool.test.ts` is killed at 30 s (exit 124), stopped in that test; - with this fix: exit 0 in 1 s, 12 pass. - 50 runs of `bun test server/file-pool.test.ts`, 10 at a time pinned to 2 cores (`taskset -c 0,1`), each under `timeout 60`: 50 pass, 0 hangs. Again pinned to one core (`taskset -c 0`): 50 pass, 0 hangs. - Leaked FIFO descriptors at the end of the file (`/proc/self/fd`, scratch probe): 2 before, 0 after; on 80b8cb2 the widened window again passes in 1 s and 50/50 runs pinned to one core pass. - `bun run check fast`: green on 80b8cb2 (typecheck, build, generated files, workflow syntax; unit 8380 pass, 0 fail, 34 skip in 535 files). ## Review omo-native-code-reviewer: PASS (APPROVE), no CRITICAL/HIGH/MEDIUM. It found no path left that blocks the test's main thread (the remaining waits are the non-blocking open and write, `hold.ran()`'s 10 s bounded wait, and awaited promises), confirmed the release cannot hit ENXIO/EAGAIN, and checked that a failing assertion while the thread is parked fails fast instead of hanging. A second omo round on 80b8cb2 (the FIFO-descriptor take-back): PASS (APPROVE), reproduced 2 leaked FIFO descriptors at 65a8a71 and none at 80b8cb2. No Codex review: test-only, `server/file-pool.ts` unchanged. ## Known limits (LOW, not fixed here) - If the thread is ended after its notify but before it enters the FIFO read, the test still passes but does not exercise a read in flight in that run (a false pass, never a hang). Every gate that notifies before it blocks has this window, `authorizeGate` included. The reviewer's findings and how each was resolved are in a PR comment.

@Yoonwoo-Ha

## Release 0.4.20 The release PR, prepared by step 1 of [docs/development.md#releasing](https://github.com/devswha/ovis/blob/main/docs/development.md#releasing), initially on main `bd676637a16471e8744d62c7311026ddb43a39eb` (#2335, xterm 6), then merged with final-scope main `ef482f2e66b3344658fb548fa1110744a92929c9` (#2338) and test-only follow-up main `269e9ce19558b8d197959250075da56b25b9668f` (#2345): - **Version:** `version` is 0.4.20 in `package.json` and `herdr-plugin.toml`. - **Changelog:** `bun scripts/changelog-fold.ts 0.4.20` initially folded 122 fragments into the 2026-10-29 section of `CHANGELOG.md`, linked their PRs, and deleted the fragments. After merging main, the modified `letta-native-chat.md` from #2338 was folded by hand with its current wording and PR link. No other late fragment landed; `changelog.d` holds only `README.md`. Four additional entries for external PRs without their own fragments bring the section to 157 entries. - Added 22 `by @login` credits across all 19 outside-contributor PRs in the release range. Preserved the existing credit to @Yoonwoo-Ha for #1634. See the complete ancestry-based audit below. - **Release summaries:** en, ko, ja and zh each have 5 new, 5 improved and 5 fixed lines, matching 0.4.19's shape. They include the xterm 6 improvement: terminal frames no longer mix old and new output. Every English line is at most 80 characters. - **Remote bundle:** `REMOTE_BUNDLE_VERSION` rises from 38 to 39 because the bundle inputs changed since remote-v38. - **Telemetry:** the guard confirms no Worker deploy or deployed fingerprint update is needed. - **iOS upload note:** CURRENT_PROJECT_VERSION raised to 3 for App and OvisNotifications (last uploaded: 2, source: `docs/native-app.md:99`, commit `54fdadc1c65da5d50a6afef411e3ed8f23b6c255`). Both targets inherit project-level Debug and Release settings, and every `CURRENT_PROJECT_VERSION` entry is 3. `app/store/ios.md` now records the prepared build. This is the last documented upload, not a live App Store Connect query; check for later uploads before uploading. The marketing version remains 1.0. - The changelog explicitly distinguishes delivery: removing WebKit's form bar (#2109) requires the new iOS app build, while Korean composition (#2307) comes with the PC update and needs no new iOS build. The Korean-input summary says the same in all four languages, with no change to list counts. ## Validation - Final `dori heavy release-check -- bun run check fast`: passed, exit 0; workflow syntax, generated types/themes, typecheck, build and unit suite (8,591 pass, 0 fail, 34 platform skips in 549 files). This is the merged tree containing #2338, #2345, the contributor corrections and iOS build 3. - `bun test scripts/changelog-fold.test.ts server/release-notes.test.ts`: 33 pass, 0 fail. - `bun scripts/release-notes.ts 0.4.20`: passed, with all patch-note lists and the full changelog. - `bun scripts/release-guards.ts` and CI mode with `--base ef482f2`: passed on the final merged tree: "Release 0.4.20: remote bundle, telemetry Worker and release metadata are in order." - `bun scripts/release-guards.ts --release 0.4.20`: expected exit 1 only because "remote-v39 is not published." The merge pass publishes it after merging. ## Merge pass **Do not merge before the owner's OK.** - [ ] Merge only after the owner's OK and required checks pass. - [ ] After merging, push `remote-v39` on the merged commit and wait for the **Remote PC bundles** workflow to publish the manifest and all archives. - [ ] Then run **Release** on `main` with version `0.4.20`, without `v`. The Release workflow creates the app tag; do not push it by hand. - [ ] From this PR's merge until the Release run is done, merge nothing touching `server/`, `shared/`, `bun.lock` or `patches/`. - [ ] A changelog fragment merged after this PR must be moved into 0.4.20 by hand. This lane stops at the open PR. No tag, workflow dispatch, merge or telemetry deploy was performed. **Completed release follow-up:** merged main `ef482f2e` including #2338 (`fix/letta-no-live-box`), folded its revised fragment into 0.4.20, and retained the contributor-credit commit `184dc120` and iOS-build commit `0ffa3d45`. The Letta summary already describes conversation support without promising a live-terminal box, so it needs no change. Merged test-only #2345 from main `269e9ce1`: the two usage browser checks now wait for the earlier shared account-order save and start the sign-in check from a cleared order. No product behavior or late fragment was added; `changelog.d` still contains only `README.md`. The iOS delivery clarification and resolved review thread are retained. ## Complete external-contributor audit Checked every merged PR whose merge commit is in `git log v0.4.19..origin/main`: 133 PRs through final-scope main `ef482f2e`, or 134 including test-only #2345 on `269e9ce1`, with 19 PRs by 12 external non-bot authors. Both rescans found no additional external-author gaps; #2345 is by the maintainer. Added the four missing entries: #2271 and #2274 by @CharleoYang, #2290 by @DravenDou, and #2282 by @fivetaku. The tab-reopening summary now includes the history-page improvement in all four languages; list counts remain 5/5/5. Full external PR list checked: #1911 @jjsteffen; #2068 @YuuuXie; #2072 @Noobear; #2087 @JJLiebig; #2145 @mattmarcin; #2176 @jpcarranza94; #2231 @philipprass-prog; #2235 @vuongnmhvn; #2270, #2271, #2274 @CharleoYang; #2278 @jpcarranza94; #2282 @fivetaku; #2283, #2292, #2293 @YuuuXie; #2301 @jpcarranza94; #2321 @kilhyeonjun. Every PR has a linked `by @login` credit in 0.4.20. --------- Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>

YuuuXie changed the title ~~fix: use stable kernel identity for Copilot foreground ownership~~ fix(copilot): use stable kernel identity for foreground ownership Oct 28, 2026

devswha merged commit c6ba077 into devswha:main Oct 28, 2026
27 of 29 checks passed

devswha mentioned this pull request Oct 29, 2026

chore(release): 0.4.20 #2337

Merged

5 tasks

Navigation Menu

Repository navigation

fix(copilot): use stable kernel identity for foreground ownership - #2293

fix(copilot): use stable kernel identity for foreground ownership#2293
devswha merged 2 commits into
devswha:mainfrom
YuuuXie:fix/copilot-stable-process-identity

YuuuXie commented Oct 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading

Summary by CodeRabbit

Uh oh!

devswha commented Oct 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

YuuuXie commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Evidence

Follow-up verification

Uh oh!

coderabbitai Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

devswha commented Oct 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

YuuuXie commented Oct 28, 2026 •

edited

Loading

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading