Repository navigation
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Native acceptance is still incomplete on 94d99fd9, so I am withholding approval pending validation.
On Linux with Python 3.11.13 and the locked pylibseekdb==1.4.0.post2, test_seekdb_legacy_choice_preserves_old_data_and_separates_new_database failed both permitted attempts: the first exceeded the unchanged 360-second process deadline; the second exited with TimeoutError from the existing 300-second scenario timeout while entering the explicitly selected powercontext Server lifespan (seekdb_legacy_choice.py:179). Synchronous diagnostic dumps were retained. The companion legacy CLI scenario passed.
Reproducer after the locked SeekDB/Code install:
POWERCONTEXT_TEST_BOOTSTRAP_SEEKDB=1 .venv/bin/python -m pytest -q -o faulthandler_timeout=0 tests/e2e/test_database_bootstrap.py -k 'legacy_choice or legacy_migration_cli'The host was busy and I have not replayed this native scenario on base, so this is an acceptance limitation, not a confirmed regression introduced by this patch. The schema comparison checks found no behavioral difference or input mutation, and the change adds no database tables or public interfaces.
Current CI also stopped at the unchanged /proc/<pid>/stat process-exit race in test_upgrade_release_gate.py, leaving the native jobs skipped. Please establish passing native/CI evidence for this head, and distinguish the remaining startup timeout from any baseline or environment issue before merging.
Keep master's schema-review history and append this branch's MySQL inventory statement; recompute the persistence digest for the merged managed inputs.
/proc/<pid>/stat can report ESRCH when the task exits between path resolution and read, which made the parent-term cancellation test flaky. Handle ProcessLookupError alongside FileNotFoundError only; permission and resource-exhaustion errors still propagate.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
The pinned skywalking-eyes header action installs with `make install`, whose Makefile targets /usr/local/bin and ignores the unprivileged copy failure, so its next step cannot resolve license-eye. Pass the supported INSTALL_DIR override pointing at runner.temp and publish that directory on GITHUB_PATH. The license check and its inputs are unchanged.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Next derives its default build worker count from the visible CPU count, so page-data collection peak memory scaled with host CPUs and the check-website job was OOM-killed (exit 137) on the internal runner. Cap it at 2 so the build fits the runner's memory limit. Every page is still rendered.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
…ult ACLs The case directory is a temporary directory, and on a runner whose temp root carries a default POSIX ACL that entry is inherited by every hook-created subdirectory and file. A default ACL overrides the umask, so the hooks' own `$HOME/.local`, `$HOME/.local/state/powercontext`, and diagnostics file were recorded as 775/664 instead of the characterized 755/644 even though each hook process already starts with umask 0o022. Remove the inherited default entry at the case root before any hook runs; it is created before the first step, so modes below it then follow the umask alone. Only the root needs handling, and private 0600/0700 expectations are unchanged. The attribute is Linux-specific, so other hosts are left alone, and an unexpected removal error still fails the case. Every other fixture field was identical in the failing CI run.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Re-reviewed a7f24f8d. Both legacy SeekDB bootstrap scenarios now pass in the PR's CI merge build, and the /proc race fix preserves propagation of unrelated errors. No new database tables or public interfaces were added.
Approval remains withheld for integration and validation blockers:
- Merging into current master
9697d5f2produces conflicts inrelease/upgrade-support.jsonandtests/test_upgrade_release_gate.py(independently reproduced withgit merge-tree). Preserve both sides' changes and recompute the persistence declaration against the combined managed sources. - The website job still exits 137 during page-data collection with two workers. Upgrade preparation fails because the distribution checker rejects metadata version 2.5, so the installed-release matrix has no passing result. These failures need resolution or demonstrated infrastructure attribution and successful reruns.
The remaining SeekDB 3.11 shard was interrupted by runner shutdown; it provides incomplete native evidence. I found no additional runtime correctness regression in the changed code. Please reconcile master and obtain passing required checks on the resulting head before merging.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Re-reviewed 716ff071. The previous merge conflicts and website failure are cleared. The schema comparison review found no additional correctness defect, and this change adds no database tables or public interfaces.
I am still withholding approval because current-head acceptance is incomplete:
- On Linux, Python 3.11.13 and locked
pylibseekdb==1.4.0.post2, the legacy database-choice scenario exceeded its 360-second process deadline. Its permitted retry then raisedTimeoutErrorfrom the existing 300-second scenario timeout while initializing the legacytestdatabase. The legacy migration CLI scenario also raisedTimeoutErrorduring its 30-second seed phase, before invoking the CLI. - In the current CI run, the Python 3.14 E2E runner received a shutdown signal; native/backend and installed-release upgrade jobs were subsequently skipped. This is not a reported test assertion failure.
Native reproducer after the locked SeekDB/Code install:
POWERCONTEXT_TEST_BOOTSTRAP_SEEKDB=1 .venv/bin/python -m pytest -q -o faulthandler_timeout=0 tests/e2e/test_database_bootstrap.py -k 'legacy_choice or legacy_migration_cli'The local 16-core host was heavily overloaded (one-minute load exceeded 150), and I have not run a matched base comparison. These timeouts are validation limitations, not confirmed regressions introduced by this patch. Please establish passing native and required CI evidence for this head on an adequately provisioned runner before merging.
| await asyncio.sleep(30) | ||
| print(f"pending phase: {current_phase()}", file=sys.stderr, flush=True) | ||
| # The C watchdog can race frame changes; this synchronous call holds the GIL. | ||
| faulthandler.dump_traceback() |
There was a problem hiding this comment.
[P2] 采样协程覆盖不到"事件循环被阻塞"的挂起，seekdb 挂起时 Python 栈会整体丢失
diagnose() 由同文件第 128 行的 asyncio.create_task 启动，只在事件循环能调度时执行。被替换掉的 faulthandler.dump_traceback_later(45, repeat=True) 是从 C watchdog 线程触发的，事件循环被阻塞时仍然能 dump。
embedded seekdb 的典型挂起恰好是原生调用等待锁——本 PR 的动机（"repeated native watchdog dumps can segfault"）本身就说明这些 dump 打在原生/greenlet 帧上。在这种场景下 diagnose() 既不会打印 pending phase，也不会 dump 栈，父进程 tests/e2e/test_database_bootstrap.py::run_seekdb_scenario 的超时分支只剩 deadline exceeded after 90s 一行：DB 快照和 native 日志还能拿到，但 Python 侧停在哪个 await、哪个 phase 完全丢失。
同一个 PR 里的兄弟 fixture 走的是另一条路：tests/fixtures/seekdb_legacy_migration_cli.py 用 Thread(target=diagnose) 配 faulthandler.dump_traceback(file=2)（第 402–405 行），从普通线程采样，循环被阻塞时也能出栈。两个 fixture 的覆盖不一致。
建议：让这个 fixture 也改成线程采样（stopped.wait(45) 循环 + 从线程调用 faulthandler.dump_traceback()），或者在 run_seekdb_scenario 的超时分支对子进程再补一次取栈。这样既能去掉会被重复安装的 C watchdog，也不丢"循环被卡住"这一类最需要栈的场景。已核对的前提：run_seekdb_migration_test.py 的 90 s 诊断 / 180 s 硬超时、candidate-schema-replay.yml 的 30 min 作业上限、run_atomic_memory_clean_migration.py 的 GNU timeout 逐用例 180 s 都仍然存在，超时边界本身没有被放宽。
There was a problem hiding this comment.
Confirmed the scheduler-level diagnostic gap on 716ff071 with a bounded subprocess probe. I extracted the unchanged diagnose bodies, kept their 30/45-second sampling intervals, started the coroutine and yielded once, then blocked the event-loop thread with time.sleep(600). Before the parent terminated each probe at 52 seconds, the base C watchdog and the sibling CLI fixture's Python thread emitted the blocked Python stack; the current coroutine emitted no stack or periodic phase sample.
The narrower impact is missing stack diagnostics while the loop is blocked but the GIL is available. Existing stage messages remain, and the legacy-choice caller still supplies a 360-second process deadline. This probe does not reproduce a native SeekDB hang or show that native hangs generally have this cause.
Design note (non-blocking): retain a safe diagnostic path independent of loop scheduling, or explicitly document this coverage tradeoff. This evidence does not justify restoring the unsafe C watchdog.
hireview001
left a comment
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
- CI failed: CI result (failure).
- CI failed: Upgrade release gate (failure).
- CI failed: tests (3.14, e2e) (failure).
- CI failed: Main (failure). Failed steps: tests (3.14, e2e) / Run remaining end-to-end tests; Upgrade release gate / Require the selected parallel acceptance workflow; CI result / Require successful selected checks.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Re-reviewed 6c960c58. The merge conflicts are cleared against current master 19f060cb; Python 3.11–3.14 unit/E2E checks and the selected SQLite upgrade matrix pass. I found no additional correctness defect. No database tables or public interfaces are added.
Approval remains withheld pending current-head validation:
- OpenClaw packaging failed downloading
@trycua/cua-driver-darwin-arm64from npm after retries. - Windows installation exceeded the 300-second
bootstrap-cninstaller deadline. Its retained log shows 2m45s resolving dependencies and 2m09s preparing packages, in addition to bootstrap work. The affected installer and OpenClaw paths are unchanged from base; these failures do not establish a regression introduced by this patch.
Please complete or rerun the failed, cancelled and queued required checks on this head. The SQLite-only PR upgrade selection is inherited from master; real native acceptance remains separate. I did not rerun the unchanged native timing probes on the currently overloaded 16-core host (load approximately 190), so this review adds no native acceptance claim.
The existing diagnostic-coverage thread is unchanged and remains a non-blocking design note; its reproduction is already recorded there.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Updated review of 6c960c58: Main, attempt 3 now succeeds, including OpenClaw packaging, Windows installation, the selected SQLite upgrade matrix and both aggregate gates. The previously reported CI blockers are cleared.
There is a new integration blocker against current master 38fb3508: release/upgrade-support.json conflicts in schema_review.persistence_sha256 and schema_review.rationale. GitHub reports CONFLICTING / DIRTY, and git merge-tree --write-tree 38fb3508 6c960c58 reproduces this sole conflict. Both branches append distinct compatibility explanations to the shared rationale. Please retain both explanations and recompute the persistence digest from the combined managed sources, then rerun the affected gate checks.
No additional correctness defect, database table or public interface was found. The existing diagnostic-coverage note is unchanged and non-blocking. Native SeekDB/OceanBase acceptance remains separate from the SQLite-only PR selection; this update adds no native acceptance claim. Approval remains withheld until the integration conflict is resolved.
hireview001
left a comment
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.
- CI failed: CI result (failure).
- CI failed: harness-validation / Atomic Memory clean migration (OceanBase) (failure).
- CI failed: harness-validation / Atomic Memory clean migration (OceanBase shard 1/2) (failure).
- CI failed: Main (failure).
Preserve the current master compatibility rationale and the native schema comparison review, then regenerate the digest from the combined managed sources. Keep the upgrade policy and PR behavior unchanged.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Resolved the conflict in 611c24982 by retaining current master's complete compatibility rationale, appending this PR's schema-comparison rationale, and recomputing the digest from the combined managed sources. GitHub and local merge verification are clean against master 17a67db0. The upgrade policy is unchanged.
Re-reviewed the combined tree: no additional correctness defect, public interface or database table was found. make check, the focused schema/runner/upgrade-gate regressions and the policy check pass.
Native validation remains incomplete. With Linux, Python 3.11.13 and the locked SeekDB dependency, test_seekdb_legacy_choice_preserves_old_data_and_separates_new_database raised TimeoutError under its existing 300-second scenario deadline while awaiting a database response. The sibling legacy migration CLI scenario passed. There is no matched base run, so this is a validation failure, not a demonstrated regression introduced by the patch.
Reproducer:
POWERCONTEXT_TEST_BOOTSTRAP_SEEKDB=1 .venv/bin/python -m pytest -q -o faulthandler_timeout=0 tests/e2e/test_database_bootstrap.py -k 'legacy_choice or legacy_migration_cli'The new head currently has no Main workflow run; earlier CI results belong to 6c960c58. Approval remains withheld pending current-head CI and passing native validation. The existing diagnostic-coverage design note remains non-blocking.
Which issue or RFC does this PR close?
Follow-up to #2168, #2458 and #2394.
Rationale for this change
Repeated native watchdog dumps can segfault during SeekDB tests. Schema compatibility checks also copy complete inventories for each recovery prefix, adding avoidable work near test deadlines.
What changes are included in this PR?
/proctasks while allowing permission and resource errors to propagate; the narrow ESRCH handling is provided by master.upgrade-required.Are there any user-facing changes?
No API, stored-schema, migration-resource or supported-upgrade changes. No timeout, retry budget or acceptance requirement is relaxed.
How was this change tested?
Validation commands cover diagnostic subprocess outcomes, schema compatibility, upgrade-gate evidence, Candidate replay and CI admission:
uv run --locked python -m pytest -q tests/test_seekdb_test_runner.py tests/test_upgrade_release_gate.py tests/test_ci_pipeline.py tests/builtin/persistence/test_mysql_migration_schema.pyuv run --locked python -m pytest -q tests/builtin/persistence/test_candidate_schema_replay.pymake checkThese focused checks do not establish real SeekDB/OceanBase or installed-release acceptance. The current master workflow selects SQLite upgrade acceptance for pull requests and reserves the complete native upgrade matrix for scheduled or manual execution. The Main workflow's manual
upgrade_selection=fullrun provides that separate acceptance path; no passing full-matrix result is claimed. The support policy remainsupgrade-required.AI usage statement
OpenAI Codex assisted implementation, integration and review. Local Bub with
magpie:group/deepseek-flashat high reasoning effort also contributed to this PR.