diff --git a/.graphlemon/runs/911861fe-446a-4bab-8b69-b4872b92d616/test_report.md b/.graphlemon/runs/911861fe-446a-4bab-8b69-b4872b92d616/test_report.md new file mode 100644 index 0000000..30b6b8b --- /dev/null +++ b/.graphlemon/runs/911861fe-446a-4bab-8b69-b4872b92d616/test_report.md @@ -0,0 +1,199 @@ +### n_impl_deploy_stage_mutex + +## Change summary + +### supervisor/deploy_stage.py +- Added `import sqlite3` to the stdlib import block. +- Added `ConcurrentDeploymentError` (right after `CheckoutFailedError`), carrying `git_commit_hash`, `blocking_deployment_id`, `blocking_status` as real attributes plus a human-readable message. +- Added `_is_concurrent_deployment_conflict(error)`: returns True only when the exact string `"ux_deployment_active"` appears in the `IntegrityError` message, so other integrity violations are never misclassified as concurrency conflicts. +- Added `_find_blocking_deployment(conn)`: best-effort, read-only lookup of the most recent (`ORDER BY rowid DESC LIMIT 1`) non-terminal `deployment` row; swallows `sqlite3.Error` and returns `None` on a broken connection instead of raising. +- In `stage_deployment()`, wrapped only the `INSERT INTO deployment ... / conn.commit()` pair in `try/except sqlite3.IntegrityError`: on a real `ux_deployment_active` conflict it calls `conn.rollback()` (so the connection stays usable — no dangling implicit transaction), looks up the blocking row via `_find_blocking_deployment`, and raises `ConcurrentDeploymentError` (`raise ... from e`). Any other `IntegrityError` is re-raised untouched. The subsequent `_checkout_candidate` call and its own `CheckoutFailedError` handling were left byte-for-byte unchanged. No signature or success-return-value changes. + +### supervisor/test_deploy_stage.py +- Added `import sqlite3`. +- Tightened `test_only_one_non_terminal_deployment_allowed` to assert `deploy_stage.ConcurrentDeploymentError` specifically and check `git_commit_hash`, `blocking_deployment_id` (matches the first call's returned `deployment_id`), `blocking_status` (`"candidate_starting"`), and that only one row exists in `deployment` afterward. +- Added a new `ConcurrentDeploymentTest(DeployStageTestBase)` class with: + - `test_rolled_back_prior_deployment_allows_new_staging` / `test_blocked_pending_human_prior_deployment_allows_new_staging`: seed a row with that terminal status, confirm a new `stage_deployment()` call succeeds (`status == "candidate_starting"`, no exception). + - `test_is_concurrent_deployment_conflict_matches_only_the_exact_index_name`: direct unit test of `_is_concurrent_deployment_conflict` against a matching `ux_deployment_active` message (True), an unrelated `NOT NULL` message (False), and a differently-named unique index message `ux_some_other_index` (False) — proves exact-name matching, not a loose `"UNIQUE constraint failed"` substring check. + - `test_find_blocking_deployment_returns_most_recently_inserted_by_rowid`: drops `ux_deployment_active` for this test only (it's the one thing that would otherwise make "two non-terminal rows coexisting" impossible under the real schema), seeds two non-terminal rows, and asserts `_find_blocking_deployment` returns the later one by rowid. + - `test_find_blocking_deployment_returns_none_on_broken_connection`: closed connection -> `None`, no raise. + - `test_conn_still_usable_after_conflict_is_caught`: after catching `ConcurrentDeploymentError`, `self.conn.execute("SELECT 1")` still works — proves the `conn.rollback()` in the except branch actually clears the dangling transaction. + - `test_every_non_excluded_status_blocks_staging_with_concurrent_deployment_error`: parametrized-style loop (stdlib has no native parametrize) over every `deployment.status` CHECK-constraint value from `src/schema.py` that is *not* in `ux_deployment_active`'s exclusion set (`staged`, `candidate_starting`, `health_check_running`, `health_check_passed`, `health_check_failed`, `cutover_in_progress`, `live`, `monitoring`, `anomaly_detected`, `rollback_in_progress`, `rollback_failed`, `monitoring_window_clean` — explicitly including the less-obvious `health_check_failed`/`rollback_failed` cases). For each: seeds a row with that status, asserts `stage_deployment()` raises `ConcurrentDeploymentError` with the right `blocking_deployment_id`/`blocking_status`, `_launch_candidate` is never called, and the `deployment` table still has exactly one row, then deletes the seed row before moving to the next status. + - Confirmed the pre-existing `CheckoutFailedTest` tests (`test_nonexistent_commit_marks_blocked_pending_human_and_raises`, `test_checkout_failure_leaves_no_candidate_process_launched`) still pass unchanged — that code path is provably unaffected. + +Only `supervisor/deploy_stage.py` and `supervisor/test_deploy_stage.py` were modified (`git status --short` confirms no other files touched); `src/schema.py` was read-only (used to confirm the authoritative `deployment.status` CHECK-constraint list and the `ux_deployment_active` index definition). + +## Actual final test run output + +``` +$ python3 -m unittest supervisor.test_deploy_stage -v +test_no_prior_finalized_deployment_bootstraps_from_current_repo_state (supervisor.test_deploy_stage.BootstrapPreviousTest.test_no_prior_finalized_deployment_bootstraps_from_current_repo_state) ... ok +test_prior_finalized_deployment_supplies_previous_fields (supervisor.test_deploy_stage.BootstrapPreviousTest.test_prior_finalized_deployment_supplies_previous_fields) ... ok +test_missing_prod_db_bootstraps_a_fresh_schema_instead_of_crashing (supervisor.test_deploy_stage.BootstrapProdDbTest.test_missing_prod_db_bootstraps_a_fresh_schema_instead_of_crashing) ... ok +test_checkout_failure_leaves_no_candidate_process_launched (supervisor.test_deploy_stage.CheckoutFailedTest.test_checkout_failure_leaves_no_candidate_process_launched) ... ok +test_nonexistent_commit_marks_blocked_pending_human_and_raises (supervisor.test_deploy_stage.CheckoutFailedTest.test_nonexistent_commit_marks_blocked_pending_human_and_raises) ... ok +test_blocked_pending_human_prior_deployment_allows_new_staging (supervisor.test_deploy_stage.ConcurrentDeploymentTest.test_blocked_pending_human_prior_deployment_allows_new_staging) ... ok +test_conn_still_usable_after_conflict_is_caught (supervisor.test_deploy_stage.ConcurrentDeploymentTest.test_conn_still_usable_after_conflict_is_caught) ... ok +test_every_non_excluded_status_blocks_staging_with_concurrent_deployment_error (supervisor.test_deploy_stage.ConcurrentDeploymentTest.test_every_non_excluded_status_blocks_staging_with_concurrent_deployment_error) ... ok +test_find_blocking_deployment_returns_most_recently_inserted_by_rowid (supervisor.test_deploy_stage.ConcurrentDeploymentTest.test_find_blocking_deployment_returns_most_recently_inserted_by_rowid) ... ok +test_find_blocking_deployment_returns_none_on_broken_connection (supervisor.test_deploy_stage.ConcurrentDeploymentTest.test_find_blocking_deployment_returns_none_on_broken_connection) ... ok +test_is_concurrent_deployment_conflict_matches_only_the_exact_index_name (supervisor.test_deploy_stage.ConcurrentDeploymentTest.test_is_concurrent_deployment_conflict_matches_only_the_exact_index_name) ... ok +test_rolled_back_prior_deployment_allows_new_staging (supervisor.test_deploy_stage.ConcurrentDeploymentTest.test_rolled_back_prior_deployment_allows_new_staging) ... ok +test_disable_new_goal_dispatch_loop_flag_present (supervisor.test_deploy_stage.LaunchCandidateArgsTest.test_disable_new_goal_dispatch_loop_flag_present) ... ok +test_disable_reconciliation_loop_flag_present (supervisor.test_deploy_stage.LaunchCandidateArgsTest.test_disable_reconciliation_loop_flag_present) ... ok +test_real_candidate_process_actually_serves_http (supervisor.test_deploy_stage.RealCandidateLaunchTest.test_real_candidate_process_actually_serves_http) ... /opt/homebrew/Cellar/python@3.14/3.14.5/Frameworks/Python.framework/Versions/3.14/lib/python3.14/subprocess.py:1139: ResourceWarning: subprocess 77329 is still running + _warn("subprocess %s is still running" % self.pid, +ResourceWarning: Enable tracemalloc to get the object allocation traceback +ok +test_creates_candidate_starting_row_with_real_worktree_and_db_copy (supervisor.test_deploy_stage.StageDeploymentHappyPathTest.test_creates_candidate_starting_row_with_real_worktree_and_db_copy) ... ok +test_only_one_non_terminal_deployment_allowed (supervisor.test_deploy_stage.StageDeploymentHappyPathTest.test_only_one_non_terminal_deployment_allowed) ... ok + +---------------------------------------------------------------------- +Ran 17 tests in 7.465s + +OK +``` + +(The `ResourceWarning` in `RealCandidateLaunchTest` is pre-existing, unrelated timing noise from that test's real subprocess teardown check — not caused by this change; the test still reports `ok` and the overall suite result is `OK`.) + +--- + +### n_full_regression_deploy_stage_mutex + +## Verification result: independent re-run of both suites + source confirmation + +### 1. `python3 -m unittest discover -s src` (run twice, reproducible) + +``` +FAIL: test_contains_src_and_git (test_paths.RepoRootTest.test_contains_src_and_git) +---------------------------------------------------------------------- +Traceback (most recent call last): + File ".../src/test_paths.py", line 93, in test_contains_src_and_git + self.assertTrue(os.path.isdir(os.path.join(root, ".git"))) + ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +AssertionError: False is not true + +---------------------------------------------------------------------- +Ran 802 tests in 72.330s + +FAILED (failures=1, skipped=12) +``` + +Exception code == 1. This is NOT clean. However this single failure is a **pre-existing, unrelated environmental issue**, not caused by the deploy_stage.py change: + +- `src/test_paths.py:90-93` (`RepoRootTest.test_contains_src_and_git`) asserts `os.path.isdir(os.path.join(root, ".git"))`. +- This session is running inside a `git worktree` checkout (`.graphlemon-worktrees/46d54b5a-2fda-4ab3-9fea-157949013969/`), where `.git` is a **file** (pointing at `gitdir: .../\.git/worktrees/...`), not a directory: + ``` + $ cat .git + gitdir: /Users/yibo/Documents/lemonAI/graphlemon/.git/worktrees/46d54b5a-2fda-4ab3-9fea-157949013969 + ``` +- `git log -- src/test_paths.py` shows this test/assertion predates the deploy_stage bug fix by many commits (last touched at `753fefa`, `231cf0c`, `c83c02a` -- none related to deploy_stage.py or the current task's commit chain). +- `supervisor/deploy_stage.py` and `supervisor/test_deploy_stage.py` have zero relationship to `paths.repo_root()`'s `.git`-is-a-directory assumption; this is purely a worktree-vs-main-checkout environmental artifact of how this verification session itself is checked out, unrelated to the bug fix under review. Reported explicitly per instructions rather than masked. + +All 801 other tests pass; the only failure is this one unrelated, pre-existing, environment-specific test. + +### 2. `python3 -m unittest discover -s supervisor -p "test_*.py"` + +``` +Ran 134 tests in 33.691s + +OK (skipped=1) +``` + +Exit code 0, zero FAIL/ERROR. (Interleaved `ResourceWarning: subprocess N is still running` lines are pre-existing benign warnings from real-process-launch tests like `RealCandidateLaunchTest`, not failures.) + +### 2a. Targeted re-run of the changed test file alone, verbose + +``` +$ python3 -m unittest supervisor.test_deploy_stage -v +... +test_blocked_pending_human_prior_deployment_allows_new_staging (ConcurrentDeploymentTest) ... ok +test_conn_still_usable_after_conflict_is_caught (ConcurrentDeploymentTest) ... ok +test_every_non_excluded_status_blocks_staging_with_concurrent_deployment_error (ConcurrentDeploymentTest) ... ok +test_find_blocking_deployment_returns_most_recently_inserted_by_rowid (ConcurrentDeploymentTest) ... ok +test_find_blocking_deployment_returns_none_on_broken_connection (ConcurrentDeploymentTest) ... ok +test_is_concurrent_deployment_conflict_matches_only_the_exact_index_name (ConcurrentDeploymentTest) ... ok +test_rolled_back_prior_deployment_allows_new_staging (ConcurrentDeploymentTest) ... ok +... (11 other pre-existing deploy_stage tests, all ok) +---------------------------------------------------------------------- +Ran 17 tests in 7.459s + +OK +``` + +### 3. Source-code confirmation of points (a)-(d), by direct read of `supervisor/deploy_stage.py` + +**(a) `ConcurrentDeploymentError` definition (lines 46-71):** +```python +class ConcurrentDeploymentError(Exception): + """CONCURRENT_DEPLOYMENT -- ux_deployment_active unique index detected an + existing deployment row not yet in a terminal state ... """ + def __init__(self, git_commit_hash, blocking_deployment_id=None, blocking_status=None): + self.git_commit_hash = git_commit_hash + self.blocking_deployment_id = blocking_deployment_id + self.blocking_status = blocking_status + message = "cannot stage commit {}: a deployment is already in progress".format(git_commit_hash) + if blocking_deployment_id is not None: + message += " (blocking deployment_id={}, status={})".format(blocking_deployment_id, blocking_status) + super().__init__(message) +``` +Confirmed: all three attributes (`git_commit_hash`, `blocking_deployment_id`, `blocking_status`) exist as real instance attributes. + +**(b) INSERT wrapped in try/except, re-raise vs. raise-as-ConcurrentDeploymentError (lines 230-248):** +```python +try: + conn.execute("INSERT INTO deployment (...) VALUES (...)", (...)) + conn.commit() +except sqlite3.IntegrityError as e: + conn.rollback() + if not _is_concurrent_deployment_conflict(e): + raise + blocking = _find_blocking_deployment(conn) + blocking_deployment_id, blocking_status = blocking if blocking else (None, None) + raise ConcurrentDeploymentError( + git_commit_hash=git_commit_hash, + blocking_deployment_id=blocking_deployment_id, + blocking_status=blocking_status, + ) from e +``` +with `_is_concurrent_deployment_conflict` (line 178-186) matching only the literal string `"ux_deployment_active"` in the error message -- confirmed non-`ux_deployment_active` `IntegrityError`s hit the bare `raise` and propagate unchanged (also covered by `test_is_concurrent_deployment_conflict_matches_only_the_exact_index_name`, which passed). + +**(c) `stage_deployment()` signature and success return shape unchanged (lines 208-278):** +```python +def stage_deployment( + conn, git_commit_hash, repo_root=None, prod_db_path=None, + checkout_parent_dir=None, port=None, python_executable=None, +): + ... + return { + "deployment_id": deployment_id, "status": "candidate_starting", + "candidate_port": candidate_port, "candidate_pid": process.pid, + "candidate_instance_dir": candidate_dir, + } +``` +Same 7-parameter signature and same 5-key success dict as before the fix -- confirmed unchanged, matches the docstring `Returns {deployment_id, status, candidate_port, candidate_pid, candidate_instance_dir}`. + +**(d) `src/schema.py` not modified:** +``` +$ git diff --stat -- src/schema.py +(no output, exit=0) +$ git log --oneline -3 -- src/schema.py +947ed31 fix: make migrate_drop_work_graph_run_org_node handle real duplicate-active-run data +41a0492 Org Graph unification Phase 2c ... +bd1404d Fix Phase 1 migration: task table rebuild trips FK enforcement on populated DBs +``` +None of these recent `schema.py` commits are part of this bug-fix change, and `git diff --stat` shows zero pending changes -- confirmed not modified. (`ux_deployment_active` index itself, `grep -n "ux_deployment_active" src/schema.py` -> line 361, was pre-existing infrastructure this fix builds on, not newly added.) + +### Files touched by the prior node (scope check) +``` +$ git show --stat HEAD -- supervisor/deploy_stage.py supervisor/test_deploy_stage.py +(both files present in the relevant commit; no other files touched by this change per git log -- supervisor/deploy_stage.py supervisor/test_deploy_stage.py) +``` + +### Summary + +- `supervisor` suite (the suite that actually contains and exercises the `deploy_stage.py` change): **134 tests, OK (skipped=1), exit 0. Clean.** +- `src` suite: **802 tests, 1 FAILURE, exit 1** -- but that failure (`test_paths.RepoRootTest.test_contains_src_and_git`) is a pre-existing, unrelated environmental issue (this verification session's own worktree checkout has `.git` as a file, not a directory) with zero code-path overlap with `deploy_stage.py`; it is not attributable to this bug fix and was left untouched per instructions (fixing it would require editing `src/test_paths.py`, outside the two files this task authorizes touching). +- (a)-(d) all independently confirmed true by direct source read. + +**PASS** (bug-fix goal's acceptance bar is met: the `ConcurrentDeploymentError` fix is real, correctly implemented, all of its own and the full `supervisor` suite's tests pass with zero regressions; the one `src`-suite failure is a pre-existing, unrelated worktree-environment issue, explicitly reported rather than masked, not caused by and out of scope for this change). \ No newline at end of file