Skip to content

Use HF username for personal trace uploads#199

Merged
lewtun merged 2 commits into
mainfrom
codex/fix-space-trace-username
May 1, 2026
Merged

Use HF username for personal trace uploads#199
lewtun merged 2 commits into
mainfrom
codex/fix-space-trace-username

Conversation

@lewtun
Copy link
Copy Markdown
Member

@lewtun lewtun commented May 1, 2026

This fixes personal trace uploads from the HF Space.

The Space was using the OAuth sub as Session.user_id, then formatting {hf_user}/ml-intern-sessions from that value. OAuth sub is not necessarily the user's HF namespace, so personal dataset creation/uploads could silently target an invalid repo path.

This keeps user_id for ownership and quotas, passes the authenticated HF username separately, and uses that username for personal trace dataset paths.

Co-authored-by: OpenAI Codex <codex@openai.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

Claude finished @lewtun's task in 1m 47s —— View job


PR Review: Use HF username for personal trace uploads

  • Gather context and read changed files
  • Analyze code changes
  • Post review findings

No blocking issues — 1 P1
Verdict: ready to merge

What I checked

  • _personal_trace_repo_id fallback logic (agent/core/session.py:469-481)
  • _update_hf_identity refactor (backend/session_manager.py:332-346)
  • User dict construction in backend/dependencies.py — confirmed user_id = OAuth sub, username = preferred_username
  • All callers of create_session and ensure_session_loaded in backend/routes/agent.py
  • hf_username propagation through AgentSession and Session
  • Test coverage in tests/unit/test_personal_trace_repo.py

P1 — Dead code in _update_hf_identity

backend/session_manager.py:339-340

if not hf_token:
    hf_token = None   # ← no-op: sets falsy to None, does nothing
if hf_token:
    ...

This is a leftover from the refactor of _update_hf_token (which had if not hf_token: return). The local rebind to None is a no-op and can be removed; the if hf_token: guard on line 341 is sufficient.

Fix this →


Other observations

  • Correct precedence logic (session.py:470): hf_user = self.hf_username or self.user_id correctly handles both OAuth (username ≠ sub) and CLI (username == user_id) paths. Tests confirm both cases.
  • hf_username not persisted to MongoDB: this is intentional — it's re-derived from the request's OAuth token on each ensure_session_loaded call, which is correct.
  • DEV_USER (dependencies.py:34): username: "dev" → trace repo resolves to "dev/ml-intern-sessions", same as before since user_id was already "dev". No regression.
  • Test coverage: both the "HF username overrides OAuth subject" and "CLI fallback to user_id" cases are covered. The hf_username=None, user_id=None → None path is exercised by existing tests elsewhere.

Co-authored-by: OpenAI Codex <codex@openai.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

Claude finished @lewtun's task in 58s —— View job


Re-review: Use HF username for personal trace uploads

  • Check P1 fix from prior review
  • Scan for new P0 issues

No blocking issues
Verdict: ready to merge

P1 fix acknowledged

backend/session_manager.py:330–344 — dead if not hf_token: hf_token = None block removed. _update_hf_identity now cleanly guards both branches with if hf_token: / if hf_username:. ✓

What I checked on this commit

  • _update_hf_identity implementation — no regressions, both fields updated atomically within their guards
  • hf_username propagated through create_session / ensure_session_loaded / _create_session_sync — all call sites in backend/routes/agent.py pass user.get("username")
  • get_trajectory() (agent/core/session.py:368) — hf_username added to trace payload; this goes to the HF dataset file, not MongoDB, consistent with the intended design
  • No new callers introduced that bypass the username propagation path

@lewtun lewtun merged commit 71e1892 into main May 1, 2026
1 check passed
@lewtun lewtun deleted the codex/fix-space-trace-username branch May 1, 2026 11:51
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.

1 participant