Repository navigation
fix: accept optional native login phase transitions - #91
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
There was a problem hiding this comment.
ℹ️ No critical issues — two minor suggestions inline.
Reviewed changes
I reviewed the native phased-login transition change, its docs and the qualifier update at 4a13b7e. tests/test_bc_login.py passes locally with Chromium (71 passed).
- Phase transitions in
_execute_login: after each submission,next_phasepolls the finalsuccesslocator first, then later intermediate markers from latest to current, all within onetimeout_msdeadline. It only moves forward, and a timeout takes the samesettle_attempt→login_failedpause path as before. record_proofextraction: a foreign run still raisesStepAbortErrorinside the reservedtry, so settlement and no-replay are unchanged.- Unchanged guarantees: the reservation is still taken before the first submission, each phase still submits once,
{{OTP}}still resolves at submit time, and resume still only verifies. Skipped phases also skip their preparationsteps, which is the intended optional-chooser behavior. - Docs and changelog:
doc/SAFE_LOGIN.mddescribes the new rule, andCHANGELOG.mdhas aFixedentry. - Image qualifier: it now runs
test_bc_login.pyin the image, loadspytest_asyncio.pluginwithasyncio_mode=auto, and copies thepytest-asynciodist-info so the plugin can find its version. - Test coverage: the new test covers 4 paths × 2 delays (password → final, password → OTP, password → chooser → OTP, existing session) through
save_recipe/load_recipeand the real_run_recipedispatcher.
claude-opus-5-5 | 𝕏
There was a problem hiding this comment.
✅ No new issues found. Both earlier suggestions are addressed.
Reviewed changes
I reviewed commit 000ffa1, which follows up on the review of 4a13b7e.
- Shared phase markers:
_execute_loginnow buildsmarkersonce, before the startup scan. The scan callsmarkers[index].wait_for(...)instead of building each locator again. Playwright locators are lazy, andmarkerswas already built for every intermediate phase, so behavior does not change. Startup detection and post-submitnext_phasenow use the same locator for each phase. - Proof map: the requirement-to-proof map in
doc/SAFE_LOGIN.mdnow liststest_optional_login_phases_follow_actual_session, including the guarantee that OTP is never filled after final proof.
claude-opus-5-5 | 𝕏

Behavior
Native login currently waits only for the configured next phase after a submission. A valid final session after password submission, or direct OTP entry that skips a device chooser, therefore fails.
Check final authentication and all later phase markers within one timeout after each submission. Final proof wins. Keep the durable transaction reservation, fresh OTP resolution and verification-only resume. No recipe schema, database, control or exit-code change. Owner: infra-zlth.85; recipe follow-up: infra-zlth.52.
Verification
PLAYWRIGHT_BROWSERS_PATH=/path/to/chromium python -m pytest tests/test_bc_login.py -qtimeoutbut received the generic browser error. Its 1.5-second traversal budget ran during concurrent browser/image work. That unchanged case passes alone (1 passed), and the complete calendar suite passes (33 passed). Required full CI remains a merge gate.sha256:ad0f32e347e9c32966497066600184bd536c8d9eba507c54ca6728a7e4c7ee91, revision000ffa1f5ed7676fd6d7f4ce1fb78992a5ddb1fc: 159 tests passed, packaged source hashes match, runtime checks and all seven legacy scenarios pass (8 synthetic PDFs). UID/GID 1003:1000, network none, proof container retained.python tests/qualify_bourse_direct_image.py --image billcollector-login85:000ffa1 --engine "$PWD" --proof-dir /path/to/new-proof --container-name unique-proof-name. Network none; synthetic credentials and temporary DB/profile/output only. The qualifier now runs native login tests and supplies pytest-asyncio metadata/configuration.Infra handoff
N/A: engine bug fix and recipe-agent handoff only. No release, production access or deployment. Schema and DB are unchanged. Revert the focused commits for code rollback.
Fixtures replace vault/notification transports, browser launch/teardown in dispatcher assertions, display services and consumers. These checks do not qualify a real broker, vault sidecar, recipe selectors, scheduling or Paperless delivery.
AI assistance
Codex assisted implementation, synthetic reproduction, source review and verification. Author checked reservation ownership, monotonic phase progression, final proof and the semantic diff.
Pullfrog review
Require a completed Pullfrog review of the final head and required CI. Final review: #91 (review) at
000ffa1f5ed7676fd6d7f4ce1fb78992a5ddb1fc: no new issues; both earlier suggestions addressed. All threads resolved. Final exact-image qualification passed. Required CI remains the final merge gate.