Skip to content

Fix #437: too little flips sent - #438

Open
ekwav-agent wants to merge 2 commits into
Coflnet:mainfrom
ekwav-agent:ekwav-agent/task_syhfagmqkdzcjodiohia
Open

Fix #437: too little flips sent#438
ekwav-agent wants to merge 2 commits into
Coflnet:mainfrom
ekwav-agent:ekwav-agent/task_syhfagmqkdzcjodiohia

Conversation

@ekwav-agent

@ekwav-agent ekwav-agent commented Aug 26, 2026

Copy link
Copy Markdown

Companion regression for #437. The root cause is in the shared flip distributor and is fixed by Coflnet/SkyBackendForFrontend#122.

The original revision re-registered /cofl flip always in FlipCommand. That workaround was removed: it could replace an already-correct tier registration and reproduce the stale-close ownership race instead of fixing it.

This PR now only verifies the SkyModCommands contract: /cofl flip always enables flips without replacing the socket's existing tier registration. The persistent settings and normal tier-registration paths are unchanged.

Verification:

  • patched container suite: 262/262 passed;
  • the reviewed workaround revision fails the new wrapper-identity assertion;
  • final container build passed;
  • separate-session review: approved.

Merge/review the owning-library fix in SkyBackendForFrontend#122 first. This PR can then be considered as companion coverage; it does not contain the root production fix itself.

This PR requires human review and merge.

@ekwav-agent
ekwav-agent requested a review from Ekwav August 26, 2026 06:44
@Ekwav

Ekwav commented Aug 26, 2026

Copy link
Copy Markdown
Member

Please re-open the investigation and correct this PR only if the production evidence supports it. The report trace can now be disambiguated with observe report 111d89 --at 2026-08-25T20:54:11.512Z; use that first, then inspect related traces/logs/metrics as needed. The reporter received roughly 1 of 13 flips, and reconnecting plus /cofl flip always did not fix it. Do not assume the current UpdateConnectionTier patch is the root fix. In particular, verify or disprove the reconnect race where SubToTier replaces a wrapper under a deterministic connection ID and the old socket later calls RemoveConnection(this), potentially deleting the new registration; the shared lifecycle code may be in Coflnet/SkyBackendForFrontend. Base the reproduction and regression test on the real trace behavior. If the required root fix belongs in that related repository and cannot safely be made in this PR, leave this PR unchanged apart from reverting unsupported work and report the exact cross-repository change needed so an ordered follow-up can be launched. Run the configured tests and summarize the trace evidence.

@Ekwav

Ekwav commented Aug 26, 2026

Copy link
Copy Markdown
Member

The diagnostics gateway/client mismatch that blocked follow-up seado5 has been corrected: DevServer 7f4fdbf and Fleet 2307d57f now both support the timestamp qualifier. Start from the current PR head and run observe report 111d89 --at 2026-08-25T20:54:11.512Z before deciding whether the current patch is valid. Continue the trace-first investigation and regression requirements from my preceding comment. If the lookup fails, report the exact new status/body; do not infer a production sequence.

@Ekwav

Ekwav commented Aug 26, 2026

Copy link
Copy Markdown
Member

The second diagnostics contract blocker is fixed and deployed in DevServer 061f73d. Start a clean evidence pass from the current PR head: run observe report 111d89 --at 2026-08-25T20:54:11.512Z, then use the returned private trace reference for bounded observe trace, observe logs, and observe related lookups. Do not publish internal trace or related-reference identifiers. Establish the real registration/removal sequence. Preserve the prior finding that SubToTier replaces by deterministic ID while stale RemoveConnection removes only by ID, but verify it against the production evidence. The /cofl flip always patch is not a root fix if an old close can delete that renewed registration. Revert unsupported changes in this PR and report the exact Coflnet/SkyBackendForFrontend compare-and-remove change and regression needed, or update this PR only if the complete evidence proves a SkyModCommands-local root fix. Run configured tests.

@Ekwav Ekwav mentioned this pull request Aug 26, 2026
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