[codex] Ensure LangSmith fleet fallback artifacts - #2528
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a LangSmith fleet artifact ensure script, wires it into CI workflows, updates fleet artifact name matching, and extends script and workflow tests for the new flow. ChangesLangSmith fleet telemetry ensure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
GitHub Actions
Inactive
Workflow source detectedPR #2528 now has valid workflow source context (origin=local_request). No linked GitHub issue is required for this PR. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
0b8c98f to
7a82859
Compare
Automated Status SummaryHead SHA: 901b7aa
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
GitHub Actions
Inactive
7a82859 to
01db80b
Compare
GitHub Actions
Inactive
01db80b to
0800377
Compare
|
Post-push follow-up: addressed the GitHub Advanced Security inline comment by setting |
GitHub Actions
Inactive
GitHub Actions
Inactive
![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: 0800377fec
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/reusable-10-ci-python.yml:
- Around line 2456-2475: The reusable workflow’s Checkout Workflows LangSmith
fleet helper step currently uses a mutable actions/checkout version tag, which
should be pinned to an immutable commit SHA. Update the uses reference in the
checkout step to a specific SHA instead of `@v7`, keeping the existing step name,
repository, ref, and token handling in place.
In `@scripts/ensure_langsmith_fleet_artifact.py`:
- Around line 54-59: The fallback lookup in _entry_for_repo is too broad because
it matches on repo only, which can pick the wrong registry entry when multiple
surfaces exist. Update the lookup logic used by
ensure_langsmith_fleet_artifact.py to filter candidate entries by the LangSmith
artifact contract fields (surface, issue, operation, and required_domain_fields)
before building the fallback row, and then require exactly one eligible match.
If there are zero or multiple eligible entries, skip or fail instead of
returning the first repo match.
- Around line 259-267: The GitHub Actions annotations in
ensure_langsmith_fleet_artifact and the reusable CI workflow are using the wrong
prefix format, so they are not emitted as annotations. Update the print
statements around the fallback artifact warning and creation notice in
scripts/ensure_langsmith_fleet_artifact.py, and the corresponding annotation in
.github/workflows/reusable-10-ci-python.yml, to use the exact GitHub Actions
syntax with no spaces in the prefix, matching the existing warning/notice
messages.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8301b1c5-27b3-492e-a2de-c262fc1618d4
📒 Files selected for processing (4)
.github/workflows/reusable-10-ci-python.ymlscripts/ensure_langsmith_fleet_artifact.pytests/scripts/test_ensure_langsmith_fleet_artifact.pytests/workflows/test_reusable_ci_workflow.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Template(auto-detected)stranske/Fine-Art-Archive(auto-detected)stranske/Ready(auto-detected)stranske/Workflows-Integration-Tests(auto-detected)
GitHub Actions
Inactive
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/workflows/test_reusable_ci_workflow.py (1)
237-241: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate assertion. Line 241 is identical to Line 237; Line 239 (
.workflows-lib/scripts/...) already covers the substring. Drop the redundant check.♻️ Proposed cleanup
assert "scripts/ensure_langsmith_fleet_artifact.py" in langsmith_ensure_step["run"] assert ( ".workflows-lib/scripts/ensure_langsmith_fleet_artifact.py" in langsmith_ensure_step["run"] ) - assert "scripts/ensure_langsmith_fleet_artifact.py" in langsmith_ensure_step["run"] assert "fallback helper is unavailable" in langsmith_ensure_step["run"]🤖 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 `@tests/workflows/test_reusable_ci_workflow.py` around lines 237 - 241, Remove the redundant assertion in the reusable CI workflow test: in the test around langsmith_ensure_step["run"], keep the check for ".workflows-lib/scripts/ensure_langsmith_fleet_artifact.py" and the one occurrence of "scripts/ensure_langsmith_fleet_artifact.py", but delete the duplicate assertion that repeats the same substring check as the earlier line so the test remains minimal and non-redundant..github/workflows/reusable-10-ci-python.yml (1)
1534-1539: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winHarden the artifact-cache checkout with
persist-credentials: false.This
Checkout Workflows artifact cache actionstep now also pulls the LangSmith helper/registry into.workflows-libusing the App token, but unlike the newCheckout Workflows LangSmith fleet helperstep (which setspersist-credentials: falseat Line 2475), it leaves credential persistence on. This is the same zizmor "credential persistence through GitHub Actions artifacts" class flagged on the sibling step, and it replicates across the 9 consumer repos this workflow syncs to. Apply the same hardening here.🔒 Proposed hardening
ref: ${{ inputs.workflows_ref || 'main' }} path: .workflows-lib token: ${{ steps.app_token.outputs.token || github.token }} + persist-credentials: false sparse-checkout: | .github/actions/artifact-cache config/langsmith_fleet_registry.json scripts/ensure_langsmith_fleet_artifact.pyAs per path instructions: "Flag template-injection, unpinned third-party actions, and spoofable bot-actor checks — this workflow YAML is synced to 9 consumer repos, so one bug replicates fleet-wide."
🤖 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 @.github/workflows/reusable-10-ci-python.yml around lines 1534 - 1539, The Checkout Workflows artifact cache action step currently persists GitHub credentials while using the App token to pull the LangSmith helper/registry, which should be hardened the same way as the Checkout Workflows LangSmith fleet helper step. Update the checkout configuration in the artifact-cache step to disable credential persistence by setting persist-credentials to false, and keep the sparse-checkout behavior intact so the helper files still land in .workflows-lib without leaving reusable credentials behind.Source: Path instructions
🤖 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.
Outside diff comments:
In @.github/workflows/reusable-10-ci-python.yml:
- Around line 1534-1539: The Checkout Workflows artifact cache action step
currently persists GitHub credentials while using the App token to pull the
LangSmith helper/registry, which should be hardened the same way as the Checkout
Workflows LangSmith fleet helper step. Update the checkout configuration in the
artifact-cache step to disable credential persistence by setting
persist-credentials to false, and keep the sparse-checkout behavior intact so
the helper files still land in .workflows-lib without leaving reusable
credentials behind.
In `@tests/workflows/test_reusable_ci_workflow.py`:
- Around line 237-241: Remove the redundant assertion in the reusable CI
workflow test: in the test around langsmith_ensure_step["run"], keep the check
for ".workflows-lib/scripts/ensure_langsmith_fleet_artifact.py" and the one
occurrence of "scripts/ensure_langsmith_fleet_artifact.py", but delete the
duplicate assertion that repeats the same substring check as the earlier line so
the test remains minimal and non-redundant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c6e26617-7a31-4de7-8ff3-1acf0b7190d4
📒 Files selected for processing (4)
.github/workflows/reusable-10-ci-python.ymlscripts/ensure_langsmith_fleet_artifact.pytests/scripts/test_ensure_langsmith_fleet_artifact.pytests/workflows/test_reusable_ci_workflow.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Template(auto-detected)stranske/Fine-Art-Archive(auto-detected)stranske/Ready(auto-detected)stranske/Workflows-Integration-Tests(auto-detected)
GitHub Actions
Inactive
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/reusable-10-ci-python.yml (1)
2540-2540: 🗄️ Data Integrity & Integration | 🟡 MinorNamespace the LangSmith fleet artifact name
This upload ignores
artifact-prefix, so repeated invocations ofreusable-10-ci-python.ymlin the same run can overwrite each other’slangsmith-fleet.ndjsonartifact. Use the prefix here too if each caller should retain its own telemetry; keep the filename fixed inpathif the downstream consumer needs it.🤖 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 @.github/workflows/reusable-10-ci-python.yml at line 2540, The LangSmith fleet artifact upload currently uses a fixed artifact name, so repeated calls to reusable-10-ci-python.yml can overwrite each other’s telemetry. Update the artifact configuration near the langsmith-fleet.ndjson upload to include artifact-prefix in the uploaded artifact name while keeping the path filename unchanged if downstream expects it. Use the existing artifact-prefix value in the same workflow step so each caller gets a namespaced artifact and collisions are avoided.Source: Path instructions
🤖 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.
Outside diff comments:
In @.github/workflows/reusable-10-ci-python.yml:
- Line 2540: The LangSmith fleet artifact upload currently uses a fixed artifact
name, so repeated calls to reusable-10-ci-python.yml can overwrite each other’s
telemetry. Update the artifact configuration near the langsmith-fleet.ndjson
upload to include artifact-prefix in the uploaded artifact name while keeping
the path filename unchanged if downstream expects it. Use the existing
artifact-prefix value in the same workflow step so each caller gets a namespaced
artifact and collisions are avoided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e42cf548-d86e-4f02-9734-3570374f1ab8
📒 Files selected for processing (2)
.github/workflows/reusable-10-ci-python.ymltests/workflows/test_reusable_ci_workflow.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Template(auto-detected)stranske/Fine-Art-Archive(auto-detected)stranske/Ready(auto-detected)stranske/Workflows-Integration-Tests(auto-detected)
GitHub Actions
Inactive
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/reusable-10-ci-python.yml (1)
2457-2475: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftStop executing the helper from
mainin a versioned reusable workflow.Linked consumers are calling
reusable-10-ci-python.yml@v1, but this path checks outstranske/Workflowsatinputs.workflows_ref || 'main'and immediately runsensure_langsmith_fleet_artifact.pyfrom that checkout. That decouples the reusable workflow version from the helper CLI/schema it depends on, so a latermainchange can break existing@v1consumers or silently emit incompatible fleet records without any change on their side. The helper/registry checkout needs to follow the same immutable ref as the reusable workflow version, not a moving branch.Also applies to: 2491-2511
🤖 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 @.github/workflows/reusable-10-ci-python.yml around lines 2457 - 2475, The helper checkout in the reusable workflow is still pinned to a moving `main` ref, which breaks versioned consumers of `reusable-10-ci-python.yml`. Update the `Checkout Workflows LangSmith fleet helper` step and the `ensure_langsmith_fleet_artifact.py` execution path so they use the same immutable ref as the reusable workflow version instead of `inputs.workflows_ref || 'main'`, keeping the helper/registry schema aligned with the workflow release.Source: Linked repositories
🤖 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 @.github/workflows/maint-80-langsmith-metrics-dashboard.yml:
- Around line 181-194: The artifact selection logic in the workflow’s repository
scan is too permissive because it accepts any newest non-expired artifact whose
name merely ends with the requested suffix. Update the jq filtering in the
artifact resolution step to restrict suffix matches to artifacts from trusted
producers only, using a provenance signal from the registry or workflow/run
metadata before accepting them; keep the exact-name path but require verified
source for the fallback match in the artifact lookup block.
In @.github/workflows/maint-81-langsmith-fleet-conformance.yml:
- Around line 68-78: The artifact selection logic currently trusts any repo-wide
suffix match in the candidate filtering and sort path, which can let spoofed
artifacts win over the intended fleet upload. Update the artifact lookup in the
workflow’s conformance download step so the match from
candidates/exactCandidates is also validated against trusted provenance metadata
before selecting artifact, and only fall back to the broader suffix match if
that provenance check passes. Keep the logic centered around the
candidatePool/artifact selection block so the reusable-CI artifact is chosen
deterministically and safely.
---
Outside diff comments:
In @.github/workflows/reusable-10-ci-python.yml:
- Around line 2457-2475: The helper checkout in the reusable workflow is still
pinned to a moving `main` ref, which breaks versioned consumers of
`reusable-10-ci-python.yml`. Update the `Checkout Workflows LangSmith fleet
helper` step and the `ensure_langsmith_fleet_artifact.py` execution path so they
use the same immutable ref as the reusable workflow version instead of
`inputs.workflows_ref || 'main'`, keeping the helper/registry schema aligned
with the workflow release.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 52b04a76-c7ea-4618-a057-3febe4cebd96
📒 Files selected for processing (6)
.github/workflows/maint-80-langsmith-metrics-dashboard.yml.github/workflows/maint-81-langsmith-fleet-conformance.yml.github/workflows/reusable-10-ci-python.ymltests/workflows/test_langsmith_fleet_conformance_workflow.pytests/workflows/test_langsmith_metrics_dashboard.pytests/workflows/test_reusable_ci_workflow.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
stranske/Template(auto-detected)stranske/Fine-Art-Archive(auto-detected)stranske/Ready(auto-detected)stranske/Workflows-Integration-Tests(auto-detected)
GitHub Actions
Inactive
|
Closer review fix 2026-06-24T13:36Z: addressed the unresolved Codex thread by uploading the fallback fleet telemetry artifact under the exact registry artifact name |
Squash-merged by closer after green checks and zero unresolved review threads. No source issue; no verifier label expected.
3 participants
Summary
artifacts/langsmith/langsmith-fleet.ndjsonreusable-10-ci-python.ymlbefore the existing check/upload steps, using the shared registry from WorkflowsWhy
The Orchestrator backlog identified that several expected producer repos had recent CI runs but no accepted
langsmith-fleetartifact alias. This makes the artifact-distribution path observable even when repo-local telemetry fails or CI exits partially, without pretending domain telemetry succeeded.Validation
uv run pytest tests/scripts/test_ensure_langsmith_fleet_artifact.py tests/workflows/test_reusable_ci_workflow.pyuv run ruff check scripts/ensure_langsmith_fleet_artifact.py tests/scripts/test_ensure_langsmith_fleet_artifact.py tests/workflows/test_reusable_ci_workflow.pyuv run black --target-version py312 --check scripts/ensure_langsmith_fleet_artifact.py tests/scripts/test_ensure_langsmith_fleet_artifact.py tests/workflows/test_reusable_ci_workflow.pygit diff --checkSummary by CodeRabbit
New Features
langsmith-fleet.ndjsonand to fetch required helper/config files with safer checkout settings.Bug Fixes
endsWithmatching, selecting the newest eligible candidate.Tests