fix(hygiene): untrack the per-run langsmith worker-attempt artifact - #72
Merged
Conversation
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 73 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
stranske
force-pushed
the
claude/agitated-almeida-13a122
branch
from
August 23, 2026 19:27
c8a2d60 to
b98e9d0
Compare
stranske
temporarily deployed
to
agent-standard
August 23, 2026 19:27 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
stranske
force-pushed
the
claude/agitated-almeida-13a122
branch
from
August 23, 2026 19:28
b98e9d0 to
20b83e2
Compare
stranske
temporarily deployed
to
agent-standard
August 23, 2026 19:28 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
stranske
temporarily deployed
to
agent-standard
August 23, 2026 19:28 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Workflow source detectedPR #72 now has valid workflow source context (origin=local_request). No linked GitHub issue is required for this PR. |
Contributor
Automated Status SummaryHead SHA: 6299481
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
|
Owner
Author
|
Runner dispatch state for codex on PR #72. Do not edit. |
Contributor
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
stranske
force-pushed
the
claude/agitated-almeida-13a122
branch
from
August 23, 2026 19:31
20b83e2 to
976bf44
Compare
Contributor
|
Workflow state fingerprint for Keepalive Loop Reporter. Do not edit. |
stranske
temporarily deployed
to
agent-standard
August 23, 2026 19:32 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
stranske
temporarily deployed
to
agent-standard
August 23, 2026 19:33 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
`langsmith-fleet-worker-attempt.json` was TRACKED at the repo root, holding one run's execution telemetry (`operation_role: worker`, a resolved `selected_model`, and the `pr_number` of whichever PR last merged). `stranske/Workflows` `reusable-codex-run.yml` rewrites it into the checkout root on every agent round, purely to stage the `actions/upload-artifact` step on the next line — so the autofix bot re-emitted it on the next PR and #66 hit an add/add conflict on a path neither side had a reason to author. That was dropped there; the cause was not. It recurred during this branch's own review: #68's merge re-added the file, turning the rebase into a modify/delete conflict, which is the defect demonstrating itself. Wrong in this repo's own terms, not merely noisy. CLAUDE.md §1's tool-vs-evidence split puts one run's telemetry outside the tree. §2 requires execution provenance to reach the learner through `feedback.py`'s tables, and a git-churned `worker` record with a resolved model is a second, unmanaged store of exactly that evidence. Nothing here reads the path — this tree's emitters are `langsmith_*.py` and they read `langsmith-fleet.ndjson` from `$ORCH_STATE_DIR`. Both steps are needed, and the order is not interchangeable. Verified against a scratch repository: `git add -A` SKIPS an ignored UNTRACKED path but STAGES an ignored TRACKED one. So ignoring alone would have been inert while the path stayed in the index, and untracking alone would have let the next round re-add it. It also means the runner's `git reset HEAD --` denylist is only load-bearing for paths that are already tracked. A PATTERN, not the literal name: the runner names the file after the ROLE it recorded, so a verifier or evaluator attempt arrives as a sibling and escapes a literal. ROOT-ANCHORED, and the leading slash is load-bearing. The first draft bounded the patterns by extension and treated that as sufficient; it is not. Unanchored, a gitignore pattern matches at EVERY depth, so `langsmith-fleet*.json` also swallowed a contract schema at `docs/contracts/schemas/langsmith-fleet-v1.schema.json` — a path `stranske/Workflows` really tracks, whose sibling `artifact-manifest-v1.schema.json` this tree already keeps. A schema IS a `.json`, so extension-bounding cannot save it. Found by a concurrent session reviewing the upstream half of this fix. The nested CI artifact directory is named separately as `artifacts/langsmith/` for the same reason: anchoring it to the one directory that holds it cannot reach a schema elsewhere. Same near-miss the vendored-`node_modules` exception records, same conclusion. Placed ABOVE the managed `BEGIN/END WORKFLOWS STATUS FILES` block on purpose: a Template-Version bump rewrites that block and would silently drop the line. `scripts/sync_status_file_ignores.py --check` still reports all canonical patterns present. Guarded by `test_repo_artifact_hygiene.py` (11 tests), which puts every question to git rather than reimplementing gitignore precedence — `.gitignore`'s own header forbids the alternative, recording that trailing comments once made every pattern inert and staged 795 files instead of 141. The new prerequisite is `env_prereq.git_repo_absent()`, gated on whether git can answer here and never on `.gitignore` itself, since gating on the measured file would leave the check unable to fail once the file went missing. Three break->revert directions are held, each demonstrated and reverted byte-identical: too narrow (the bare literal fails the four sibling cases), too broad by KIND (`*langsmith-fleet*` fails on the contract doc), too broad by DEPTH (dropping the slash fails on the schema). Floor 410 -> 421, re-measured on each merge result as main moved twice underneath (402+10, then 407+10, then 410+11). No ceiling moved — the tests run on any checkout and skip only in the exec-mirror layout, which is a file copy, not a `.git`. Upstream, handled separately: Workflows PR #2856 (2026-07-31) diagnosed this verbatim but fixed only its own repo, never `templates/consumer-repo/.gitignore`, so consumers stayed exposed and this repo repeated the defect 23 days later. Co-authored-by: Cursor <cursoragent@cursor.com>
stranske
force-pushed
the
claude/agitated-almeida-13a122
branch
from
August 23, 2026 21:47
976bf44 to
dc38c43
Compare
stranske
temporarily deployed
to
agent-standard
August 23, 2026 21:47 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
stranske
temporarily deployed
to
agent-standard
August 23, 2026 21:47 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Owner
Author
Closer rebase evidence (cursor closer lane)Rebased
CI is running on the new head. Merge will wait for required checks + the 7-minute post-push review window. |
…rop a claim #3210 falsifies This branch ended up with two independent explanations of the same rule, because a concurrent session added `4b5764a` while this branch already carried its own block. Both were correct; having both in a file whose header is specifically about gitignore-comment discipline is not. Kept from the second block, because the first did not have it: - the artifact was tracked from 2118f57, not merely "on main until today"; - ten commits across PRs #59/#61/#62/#63 rewrote it, one add/add conflict per concurrent PR, silently widening unrelated diffs; - untracking loses no provenance — the producer already uploads the same bytes as Actions artifact `langsmith-fleet-v1-worker-attempt-<pr>` with 30-day retention, which is the only reason the file is written at all. Dropped from it, because it is about to become false: "that block mirrors Workflows' templates/consumer-repo/.gitignore, which does NOT carry this pattern". Workflows PR #3210 adds exactly these two patterns to that template at Template-Version 6. The placement above the managed block is still right, but the reason is that a version bump rewrites the block wholesale — not that the template lacks the pattern. Also drops that block's bare literal `langsmith-fleet-worker-attempt.json`, which was already subsumed by the root-anchored `/langsmith-fleet*.json` above it. Comments only. Verified identical behaviour with `git check-ignore`: the root artifact and artifacts/langsmith/ stay ignored, docs/contracts/schemas/langsmith-fleet-v1.schema.json and langsmith_*.py stay committable, and no tracked file is shadowed by any pattern. 427 collected = floor 427; black, ruff and the 11 guard tests unchanged.
stranske
temporarily deployed
to
agent-standard
August 23, 2026 22:26 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
stranske
temporarily deployed
to
agent-standard
August 23, 2026 22:26 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
This was referenced Aug 24, 2026
stranske
added a commit
that referenced
this pull request
Aug 25, 2026
… tracked binary (#119) `.coverage`, a 90 KB coverage.py SQLite database, has been TRACKED on main since #109 — a typing PR whose every other changed file is about mypy. Nobody chose to commit it; it was in the working tree when the branch was staged, the same accident that put src/UNKNOWN.egg-info/ on #113. It is build output, and the churn is mechanical rather than occasional: verify.py's coverage_reset() UNLINKS ROOT/.coverage and ROOT/.coverage.* before every instrumented run and coverage_combine_and_report() writes them again. So while tracked, the repo's own verdict command showed up as a deletion of a tracked binary followed by a re-add of different bytes, and an uncommitted copy blocked a branch switch. BOTH patterns, because two different steps write them: `--parallel-mode` writes one `.coverage.<host>.<pid>.<random>` per instrumented child (~90 subprocesses per run) and `coverage combine` merges them into `.coverage`. Ignoring only the combined file leaves the per-child data files unignored. ROOT-ANCHORED per the rule #72 established for every debris pattern in .gitignore, and true of the producer here: verify.py globs and unlinks ROOT, so the checkout root is the only place these land. `/.coverage.*` and NOT `/.coverage*` — the second also swallows `.coveragerc`, coverage.py's own config file, which is source. And UNTRACKING is what makes the patterns bite: `git add -A` skips an ignored UNTRACKED path but stages an ignored TRACKED one, so the pattern alone would have changed nothing. test_repo_artifact_hygiene.py had NO case for this file — `grep -n coverage` on it returned nothing — so nothing would have caught a recurrence. Five added, matching the suite's three halves: two parametrized ignore cases, one untracked case, and two must-stay-committable cases. Break -> revert performed in all three directions the patterns can fail (too narrow, too broad by kind, ignored-but-re-tracked); each broke exactly one case, and the revert was byte-identical. Recorded in the file. The depth direction the langsmith patterns record has no in-tree near-miss here and is not claimed to; the anchoring is kept for the producer's reason instead. Verified: 458 passed, 0 failed, 0 skipped, 85/85 selftests, 5/5 gates. Floor 453 -> 458, measured on the merge result after fast-forwarding onto origin/main 5c769e0 and re-fetching to confirm main had not moved again. Co-authored-by: Tim Stranske <tim@stranskemo.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
langsmith-fleet-worker-attempt.jsonwas tracked at the repo root, holding one run's executiontelemetry (
operation_role: worker, a resolvedselected_model, and thepr_numberof whichever PRlast merged — #61 when this branch was cut).
stranske/Workflowsreusable-codex-run.ymlrewrites itinto the checkout root on every agent round, purely to stage the
actions/upload-artifactstep on thenext line — so the autofix bot re-emitted it on PR #66 against main's copy, an add/add conflict on a
path neither side had a reason to author.
The defect recurred during this branch's own review: #68's merge re-added the file, turning the
rebase into a modify/delete conflict. That is the mechanism demonstrating itself, not a coincidence.
Why this is wrong in this repo's terms, not merely noisy
evidence is not. One run's telemetry is evidence, and runtime state belongs at
$ORCH_STATE_DIR.operation_role: workerand aresolved
selected_model— exactly the causal execution provenance the learner consumes. That issupposed to reach it through
feedback.py's tables; a git-churned copy is a second, unmanagedstore of the same evidence.
Nothing in the tree reads the path. This repo's emitters are
langsmith_*.py(underscores), they readlangsmith-fleet.ndjson, andlangsmith_fetchextracts to~/.codex/orchestrator/langsmith-artifacts,outside the tree.
Both steps are required, and the order is not interchangeable
Verified against a scratch repository:
git add -AresultSo ignoring alone would have been inert while the path stayed in the index, and untracking alone would
have let the next round re-add it. It also means the runner's
git reset HEAD --denylist is onlyload-bearing for paths that are already tracked.
A pattern, and ROOT-ANCHORED
/langsmith-fleet*.json+/langsmith-fleet*.ndjson+artifacts/langsmith/.A pattern because the runner names the file after the role it recorded, so a verifier or
evaluator attempt arrives as a sibling and escapes a literal.
Root-anchored because bounding by extension is not enough — the first draft of this PR got that
wrong. Unanchored, a gitignore pattern matches at every depth, so
langsmith-fleet*.jsonalsoswallows a contract schema at
docs/contracts/schemas/langsmith-fleet-v1.schema.json— a pathstranske/Workflowsgenuinely tracks, and whose siblingartifact-manifest-v1.schema.jsonthis treealready keeps. A schema is a
.json, so the extension bound does not save it. Caught by aconcurrent session reviewing the upstream half of this fix. The nested CI artifact directory is named
separately for the same reason: anchoring it to the one directory that holds it cannot reach a schema
elsewhere. Same near-miss the vendored-
node_modulesexception records, same conclusion — theanchored pattern is the safe one.
Placed above the managed
BEGIN/END WORKFLOWS STATUS FILESblock:--checkis presence-only soextra patterns pass, but a
Template-Versionbump rewrites that block and would silently drop theline.
scripts/sync_status_file_ignores.py --checkstill reports all canonical patterns present, andno tracked file is shadowed by any pattern (checked with
git check-ignoreovergit ls-files).Guard
test_repo_artifact_hygiene.py(11 tests) puts every question to git (check-ignore,ls-files)rather than reimplementing gitignore precedence —
.gitignore's own header forbids the alternative,recording that trailing comments once made every pattern inert and staged 795 files instead of 141.
New prerequisite
env_prereq.git_repo_absent()gates on whether git can answer here, never on.gitignoreitself: gating on the measured file would leave the check unable to fail once the filewent missing.
Three break → revert directions, each demonstrated and reverted byte-identical:
*langsmith-fleet*docs/langsmith-fleet-contract.mddocs/contracts/schemas/langsmith-fleet-v1.schema.jsonVerification
Full
verify.pygreen on the identical code content: 421 passed, 0 failed, 0/26 skipped, 84/84selftests, 5/5 gates. Rebased since onto
4122a07; the only change was.verify-floor.json's twointegers, re-measured at 422 collected = floor 422, with the guard tests,
env_prereq --selftestand the sync check re-run directly. CI runs the authoritative full
verify.pyon a clean machine.Floor re-measured on each merge result as main moved underneath four times (402+10, 407+10, 410+11,
411+11) — which is exactly what the collection equality exists to force. No ceiling moved: the
tests run on any checkout and skip only in the exec-mirror layout, which is a file copy, not a
.git.Upstream — the actual root cause, fixed separately
Workflows/docs/WORKFLOW_ARTIFACT_CHECKLIST.mdis the owning doc, and its "Recovery from ArtifactPollution" procedure is exactly the two steps taken here. The recurrence cause is add-scope:
reusable-codex-run.ymlandreusable-18-autofix.ymlrungit add -Aand then subtract ahand-curated exclusion list of ~25 names, which a newly-added emitter escapes by default.
Workflows had already diagnosed this verbatim in PR #2856 (2026-07-31) — "while tracked, every run
that rewrote it produced a diff that codex-autofix then committed onto whatever PR happened to be
open" — but fixed only its own repo, never
templates/consumer-repo/.gitignore, so consumers stayedexposed and this repo repeated the defect 23 days later. Six repos carried a tracked copy.
stranske/Workflows#3210— template +FALLBACK_PATTERNSat Template-Version 6, the emitter movedto
$RUNNER_TEMPso the file never enters the checkout, and a guard test that turns the checklistinto a gate (nothing referenced it before; a grep for callers found zero).
trip-planner#1759.
🤖 Generated with Claude Code