Skip to content

feat(executor): expose task identity to codex - #2308

Merged
qdaxb merged 1 commit into
wecode-ai:mainfrom
FicoHu:feature/codex-task-identity-env
Jul 29, 2026
Merged

qdaxb merged 1 commit into
wecode-ai:mainfrom
FicoHu:feature/codex-task-identity-env

Conversation

@FicoHu

@FicoHu FicoHu commented Jul 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Documentation

    • Updated local device architecture guidance to describe task identity details passed to Codex through thread-scoped settings.
    • Clarified that task identity is isolated from shared processes and that subtask IDs are not injected into child runtimes.
  • Bug Fixes

    • Codex now receives task IDs, authentication details, and skill identity information through its thread configuration.
    • Prevented task-scoped settings from persisting into shared process launch configurations.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Codex now receives task identity through thread-scoped shell_environment_policy.set.* overrides. Tests verify JSON-RPC propagation and removal from persistent process configuration, while English and Chinese architecture documentation describe the updated behavior.

Changes

Codex task identity propagation

Layer / File(s) Summary
Thread-scoped launch configuration
executor/src/agents/codex.rs
Request task identity values are converted into Codex shell_environment_policy.set.* configuration overrides during launch configuration construction.
Forwarding and persistence validation
executor/src/agents/codex/tests.rs, executor/tests/codex_app_server_contract.rs, docs/*/wegent/developer-guide/local-device-architecture.md
Tests verify thread-only forwarding, JSON-RPC initialization payload values, and removal from persistent process configuration; both architecture documents describe the updated injection rules.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: micro66, qdaxb

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: exposing task identity to Codex through executor configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 970af62943

ℹ️ 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".

Comment on lines +2007 to +2009
launch_config
.config_overrides
.extend(task_identity_config_overrides(request));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep task credentials out of app-server arguments

In the regular AgentProcessEngine Codex path, run_codex_app_server_turn_with_cancel passes this launch_config to spawn_codex_app_server, which serializes every override into -c command-line arguments. Consequently, AUTH_TOKEN and WEGENT_SKILL_IDENTITY_TOKEN are exposed through process listings, /proc/*/cmdline, and process-monitoring telemetry for the lifetime of every Codex task. Keep these overrides exclusively in the thread JSON-RPC config, and filter them from the per-task process launch config as the persistent app-server path already does.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@executor/tests/codex_app_server_contract.rs`:
- Around line 33-36: Update write_fake_codex so JSON-RPC requests are validated
for the expected auth_token and skill_identity_token values but those sensitive
fields are redacted or removed before being appended to log_path. Preserve the
propagation assertions in the test without persisting raw bearer-token values in
the fake Codex log.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a606cb7-33d5-45a7-9d33-448a25b00daa

📥 Commits

Reviewing files that changed from the base of the PR and between b12fb6b and 970af62.

📒 Files selected for processing (5)
  • docs/en/wegent/developer-guide/local-device-architecture.md
  • docs/zh/wegent/developer-guide/local-device-architecture.md
  • executor/src/agents/codex.rs
  • executor/src/agents/codex/tests.rs
  • executor/tests/codex_app_server_contract.rs

Comment on lines +33 to +36
task_id: "task-525".to_owned(),
auth_token: Some("task-jwt".to_owned()),
skill_identity_token: Some("skill-jwt".to_owned()),
user_name: Some("alice".to_owned()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Avoid writing bearer-token values to the fake Codex log.

write_fake_codex appends every JSON-RPC request to log_path (Lines 863-864), so this test now persists the raw auth_token and skill_identity_token values. Keep the propagation check, but make the fake server validate or redact sensitive fields before logging.

As per coding guidelines, “never log tokens, API keys, or local authentication-file contents.”

Also applies to: 73-88

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@executor/tests/codex_app_server_contract.rs` around lines 33 - 36, Update
write_fake_codex so JSON-RPC requests are validated for the expected auth_token
and skill_identity_token values but those sensitive fields are redacted or
removed before being appended to log_path. Preserve the propagation assertions
in the test without persisting raw bearer-token values in the fake Codex log.

Source: Coding guidelines

@qdaxb
qdaxb added this pull request to the merge queue Jul 29, 2026
Merged via the queue into wecode-ai:main with commit 80b609a Jul 29, 2026
38 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants