Repository navigation
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. |
There was a problem hiding this comment.
🟡 Changes recommended
Cold AgentApp restarts with --token-stdin fail because stdin has already been consumed and closed, causing the CLI to exit before restoring the claimed task or registering cleanup.
1 open issue
What changed in this PR
This PR restarts cold AgentApps after runtime dependency installation so app dependencies load in a fresh interpreter, avoiding conflicts with cached modules. Preloaded workers retain their existing path.
Changes:
- Preserve claimed task input in a private file and restore it once after restart.
- Prepare dependencies and restart before consuming the initial chat prompt.
- Add regression tests for fresh imports, input restoration, cleanup, and prompt ordering.
The changes address one cohesive runtime issue; no PR split is needed.
| File | Description |
|---|---|
framework/py/flwr/supercore/task_process/agent/run_agentapp.py |
Adds interpreter restart and task-input restoration before prompt consumption. |
framework/py/flwr/supercore/task_process/agent/run_agentapp_test.py |
Tests dependency isolation, private input handoff, cleanup, and restart ordering. |
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The restart via os.execve discards the running log uploader and undrained queue, losing pending Runtime logs; the queue must be flushed before restarting without finalizing the claimed task.
1 open issue
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a71590a4b2
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58ff633861
ℹ️ 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".


What
Restart cold AgentApp processes after runtime dependency installation, with the app environment on the import path from Python startup. Preserve the already-claimed task input in a private file, restore it once after restart, and consume the chat prompt only afterward. For stdin-authenticated launches, hand the resolved token to the existing token-file CLI path using a private file. The restart does not put the token in argv or repeat the startup acknowledgement. A host-only supervisor now starts the app interpreter, forwards termination, and reports a failed task if incompatible app dependencies prevent startup. The restart handoff is authenticated and restricted to the configured runtime root and claimed run before cleanup ownership is accepted. Preloaded workers keep their existing path.
Why
New Chat can fail with
cannot import name 'Converter' from 'attr'. The runner has already cached an olderattrmodule when the app environment supplies a newerattrspackage. Changingsys.pathcannot replace those cached imports. A fresh interpreter avoids mixing the two versions without pinning the app's dependencies or unloading live modules.How to manually test
With the framework development environment installed, run this exact block from the repository root:
Expected: all tests pass. The subprocess regression first reproduces a cached dependency's missing
Converterimport, then checks fresh imports after restart, restoration without a second task-input claim, private input-file permissions, and exactly-once finalization and cleanup. A separate test checks that restart happens before consuming the initial prompt. The stdin-authenticated CLI subprocess regression checks token recovery through a private file, no token in argv or captured output, exactly one startup acknowledgement, one-time input restoration, and cleanup. New regressions cover real HTTP failure reporting when an app shadows httpx, rejection of invalid restart paths and handoffs without deleting unrelated files, and forwarding of SIGINT, SIGTERM, and SIGQUIT where available. The signal test covers both PID- and group-directed delivery, with the child in a separate POSIX session so it can finalize without duplicate delivery.Validation