Skip to content

Stop counting expired Odoo session files as testing use - #5833

Merged
shiny-code-app[bot] merged 5 commits into
mainfrom
work/lp5831-testing-real-use
Nov 9, 2026
Merged

shiny-code-app[bot] merged 5 commits into
mainfrom
work/lp5831-testing-real-use

Conversation

@shiny-code-app

@shiny-code-app shiny-code-app Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

The normal testing restore stayed pending as testing_in_use on authenticated session files with no recent sign-in. The guard counted any matching uid or pre_uid file after the generation cutoff, with no check of whether Odoo still treats that session as live. The decided rule is that such files are stale state, not a person, while real staff work stays protected. Disposition.

Refs #5831

Change

The fixed read-only probe now applies Odoo's own session rules:

  • A file older than Odoo's inactivity limit no longer counts. The limit is sessions.max_inactivity_seconds when set, otherwise SESSION_LIFETIME read from the running image's own source. Odoo's session cleanup and cookie lifetime use the same value.
  • A file whose deletion_time has passed no longer counts. Odoo refuses those sessions.
  • If the limit cannot be read, usage stays UNKNOWN.

A session inside the limit still counts with no recent sign-in, and any recent sign-in still counts on its own. An active browser keeps rewriting its file: Odoo rotates the session after three hours and saves it when it changes. Generation boundaries, drift, holds, needed copies, production separation and originals are unchanged. No session file, hold or record is changed.

The limit is read from odoo/http/session.py on Odoo 20 and odoo/http.py on earlier versions. Odoo 20 moved that module. The parser was checked against the real Odoo 18, 19 and 20 sources.

A saved positive needs no change. It stays protective until its five-minute deadline, and a fresh complete negative then replaces it while needed-copy and production-resource protection stay.

Verification

  • Failing first: an expired file read as in_use on the unchanged probe, through the emitted program and the normal usage reader.
  • tests.test_odoo_testing_usage, tests.test_disposable_lane_collection, tests.test_testing_rebuild_http, tests.test_testing_rebuild_worker, tests.test_testing_initial_generation and tests.test_odoo_preview_qualification: 317 tests passed. After the final wording and test-name commit, tests.test_odoo_testing_usage passed again.
  • ruff and mypy pass on the changed files.
  • Inspection: native PyCharm returned UNKNOWN with native_python_support_plugin_unavailable (Headless inspection workers: confirm project interpreter and Pydantic plugin so type findings stop being noise codex-skills#3363), with no findings reported, on 1a9a92122. Not repeated on the head, whose only change moves comments into a named predicate, because the same missing prerequisite would return the same UNKNOWN.

Not proved here: whether the live 8/9 files are past Odoo's limit. No lane was read for this change. Root's next normal request will show that.

Removed: the file-only positive for sessions Odoo has expired or retired, and the regression that protected an arbitrarily old session when the generation boundary was unavailable.

Net lines: +271 (+288/-17). Regression tests are +228 of that. Product code is +39, to read Odoo's limit instead of copying a number that could drift. Docs are +4.

A session file Odoo would refuse is stale state, not a person. Use Odoo's own inactivity limit and retired-session marker; an unreadable limit stays UNKNOWN.
@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: 1a9a921

Outside review: OpenAI, read-only, on b148adf4b, one round. The helper's JSON reported the model, but I did not keep that field, so I can't name the exact model. The later commit 1a9a92122 changes only one comment, the docs wording and a test name, in response to this review.

Finding 1 (P1, file age is not Odoo's validity check): declined as a code change, and the overclaim is fixed. Odoo's get does load an old file. But a person using the lane sends requests, and Odoo rewrites the file on rotation and on change. So a file past the inactivity limit has had no recent request. With the normal limit, its cookie has expired too. The reviewer's triggers are lowering the limit during a live session and a forward clock jump. Neither was reproduced, and the agreed rule is that old files with no sign-in are stale. The docs said Odoo "would still honor" counted files, which overstated it. They now state the inactivity limit only.

Finding 2 (P2, zero limit differs on Odoo 19): declined as a behavior change. A configured 0 keeps the default on every version, which is Odoo 20's rule and the protective reading on older ones. Its cost is that a lane on Odoo 19 with that setting stays pending. The test name and a code comment now say this.

Other answers from the review: no unknown input now reads as unused, the scan reorder is sound, and a saved positive is bounded by its deadline. It flagged the source parser as the growth to justify. The alternative is a copied number that can drift, which the brief rules out.

Inspection: native PyCharm returned UNKNOWN with native_python_support_plugin_unavailable (cbusillo/codex-skills#3363), with no findings reported.

@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: bba3336

Head bba333600 differs from the reviewed content only by removing comments the repository comment gate refuses and moving the retired-session test into the named predicate abandoned_odoo_session. The disposition in the earlier review note stands for this head. Focused usage, collection and plan tests, ruff, mypy and the Odoo 18/19/20 source check pass on it. Inspection stays UNKNOWN for native_python_support_plugin_unavailable (cbusillo/codex-skills#3363), run on 1a9a92122 with no findings reported.

@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: f53534b

Head f53534b47 adds only the session limit and table to the real Postgres usage test, which the probe now reads. The disposition in the earlier review notes stands for this head. That test runs in CI's Postgres job; the local run could not start because local Docker is out of disk space.

@shiny-code-app shiny-code-app Bot closed this Nov 9, 2026
@shiny-code-app shiny-code-app Bot reopened this Nov 9, 2026
@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Ready for the merge train

Head: f53534b

@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Launchplane completed the merge-train step for this pull request.

Launchplane finished the merge-train step for this pull request.

Launchplane manages this comment and will update it as the train moves.

shiny-code-app Bot added a commit that referenced this pull request Nov 9, 2026
@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: 7962d5a

Head 7962d5a12 only merges main into this branch, because the train block requires a new head. The earlier review notes stand for it. The train block itself came from a candidate whose jobs never ran, not from a test failure; that misclassification is filed separately.

@shiny-code-app

shiny-code-app Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Ready for the merge train

Head: 7962d5a

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.

0 participants