fix(feedback): answer the first advisory cycle after a reopen - #2228
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5b189498c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.answering_advisory_cycle_owner( | ||
| registered_project_root.as_deref(), | ||
| &deadline, | ||
| ) | ||
| .await |
There was a problem hiding this comment.
When an advisory-cycle payload arrives with an already-cancelled CancellationContext while the owner is mounting or publication is warming, this newly added wait runs before execute_feedback_advisory_cycle performs its cancellation check. The request can therefore remain in flight until owner publication or its deadline instead of immediately returning cancelled_before_admission; check the payload cancellation before entering this wait or make the wait cancellation-aware.
AGENTS.md reference: AGENTS.md:L201-L203
Useful? React with 👍 / 👎.
| match self | ||
| .code_index_schedulers | ||
| .latest_feedback_generation_for_scope(&self.project_root, &self.scope) | ||
| .await | ||
| { | ||
| Some(_) => DaemonAdvisoryCycleMountV1::Mounting, | ||
| None => DaemonAdvisoryCycleMountV1::Answers, |
There was a problem hiding this comment.
When deferred::try_mount returns Attempt::Terminal after an LSP grant, owner registration, or advisory composition failure, the sealed generation remains present but no task remains capable of publishing the full owner. This implementation consequently reports Mounting forever, so every subsequent advisory-cycle call waits out its deadline and returns a timeout rather than promptly surfacing the terminal unavailable state; track the deferred attempt's terminal outcome rather than inferring active mounting solely from generation presence.
AGENTS.md reference: AGENTS.md:L189-L190
Useful? React with 👍 / 👎.
1 participant
Root cause
I first tried the reported scenario literally: a daemon left idle past the 10-minute resident-memory window. It did not reproduce. The decode and graph engine were released (
resident_owner_released,graph_engine_released reason="hibernate"), and the next advisory-cycle call still answeredevidence:latest_feedback_generation_for_scopekeeps serving the retained text generation.The failing first call comes from the project being (re)opened over a ready sealed generation, which is what a daemon restart or a project reopen does. The advisory cycle mounts after project open publishes, so a request arriving during the open met one of two retryable pre-mount answers:
runtimes.advisory_cycleisNone, which answersThe advisory feedback cycle authority is unavailable.The advisory feedback cycle mounts once the first code-index generation is sealed.Both answers are
retryable: true,retry_after_millis: 250, so the second call succeeded. Timed restart run on master (the daemon had already sealed the generation in an earlier session):What changed
ProjectRuntimeRegistryV1has apublished_changedwatch. It is bumped when a component is published, whenpublish_advisory_atomicallyswaps in the full cycle, and when a project's publication stage finishes.advisory_cycle_viewreturns the owner and the publication stage together with a receiver subscribed before the read, so no publication between the read and the wait is missed.DaemonAdvisoryCycleInvocationPort::mount()reportsAnswersby default. The pre-mount owner reportsMountingexactly when a ready sealed generation exists for its scope. It still answers at once when the code index is disabled or there is no indexable source (those states are terminal).answering_advisory_cycle_ownerwaits, within the request's own deadline, while the owner isMounting, or while the owner is absent and the project's publication stage is stillWarming. Then it invokes the owner that answers. A finished publication without an owner returns at once, and so does a mounted owner.ponytail:comment: if the deferred mount fails terminally, a waiting request runs out its own deadline before it gets the warming answer.Fail before / pass after
daemon::production_harness::advisory_cycle_language_journey_test::first_advisory_cycle_after_a_reopen_answers_without_a_retryopens a Rust checkout, settles its advisory cycle, and shuts the composition down. It then reopens the same profile and makes exactly onetracedecay_feedback_advisory_cyclecall.Before (this branch with the dispatch wait reverted):
After:
ok, the outcome isevidence.Runtime journey
Debug
tracedecay-cli --no-default-features --features production, one daemon undersystemd-run --user --scope -p MemoryMax=6G -p MemorySwapMax=1G. The profile is the isolated one from the #2221 journey: rust-lang/log at PR #741's head, with a sealed generation from an earlier daemon. The daemon was restarted, and then one call was made:The call waited about 1.8 s for the mount and answered on the first try.
Checks
cargo test -p tracedecay --lib -- production_harness::advisory_cycle_language_journey_test(after rebase): 3 passed.cargo test -p tracedecay --lib -- daemon::tests::invocation_ownership daemon::tests::feedback_impact daemon::project_open_owners production_harness: 40 passed.cargo test -p tracedecay-daemon-service --lib: 319 passed, 1 failed. The failure,adoption_observation::tests::census_counts_each_composed_family_and_omits_uncomposed_families, is pre-existing: the retrieval catalog now has 41 operations and the test pins 34. This change does not touch the catalog or adoption census.cargo clippy -p tracedecay-daemon-service -p tracedecay --all-targets --features tracedecay/test-helpers -- -D warnings: clean.cargo fmt --all -- --check: clean.