Tests: test_time.py clears the job's TIME_* environment before importing time.py #54

Merged
teahubxyz merged 1 commits from time/card-51 into main 2026-10-14 21:15:49 +00:00
Collaborator

At the top of .github/time/test_time.py, before time.py is imported, the suite now removes every TIME_* variable and the seat keys (ANTHROPIC_API_KEY, VENICE_API_KEY, CURSOR_API_KEY) from os.environ. TIME_TOKEN is covered by the TIME_* prefix. Each check still sets only what it needs. Nothing else changed.

One deliberate exception: TIME_TEST_VENTURE is kept. It's the suite's own opt-in switch for the venture tests, not job configuration. Clearing it would silently disable that opt-in, which would be a behaviour change.

Checked in the worker's shell:

  • With env -i, the suite passes and prints "ALL OK" through 54, then 35, 36 and 41.
  • With TIME_WORKER=claude:sonnet TIME_MODE=live TIME_DAILY_USD=0 exported, it passes with the same output.
  • Without the fix and with those variables exported, it fails with AssertionError: unexpected POST /repos/o/r/labels.

CI has not been run on this PR yet.

Closes #51

Closes #51

worker: Claude Sonnet (Anthropic) via Time

At the top of `.github/time/test_time.py`, before `time.py` is imported, the suite now removes every `TIME_*` variable and the seat keys (`ANTHROPIC_API_KEY`, `VENICE_API_KEY`, `CURSOR_API_KEY`) from `os.environ`. `TIME_TOKEN` is covered by the `TIME_*` prefix. Each check still sets only what it needs. Nothing else changed. One deliberate exception: `TIME_TEST_VENTURE` is kept. It's the suite's own opt-in switch for the venture tests, not job configuration. Clearing it would silently disable that opt-in, which would be a behaviour change. Checked in the worker's shell: - With `env -i`, the suite passes and prints "ALL OK" through 54, then 35, 36 and 41. - With `TIME_WORKER=claude:sonnet TIME_MODE=live TIME_DAILY_USD=0` exported, it passes with the same output. - Without the fix and with those variables exported, it fails with `AssertionError: unexpected POST /repos/o/r/labels`. CI has not been run on this PR yet. Closes #51 Closes #51 <sub>worker: Claude Sonnet (Anthropic) via Time</sub>
time added 1 commit 2026-10-14 21:03:54 +00:00
card #51: kept .github/time/test_time.py (generated)
ci / test (pull_request) Successful in 35s
time/worker-run head -12 .github/time/test_time.py; git status --short
4db4344481

Object

The code change itself looks right. The blocker is that the card's finish line isn't met yet, and there is one scope question.

  • CI not green (finish line). The PR body says "CI has not been run on this PR yet." The card's DONE condition requires CI green, so this can't be marked done. Please run CI and post the result.
  • TIME_TEST_VENTURE exception (.github/time/test_time.py:7). The card says to remove every TIME_* variable. The PR keeps TIME_TEST_VENTURE on the grounds that it's the suite's own opt-in switch. The reasoning is sound, but it's a deviation from the card's wording. The finish-line check also doesn't cover it: the verified job-env run exports TIME_WORKER, TIME_MODE and TIME_DAILY_USD but not TIME_TEST_VENTURE. If a job does export it, the venture tests run inside the worker's shell, which is the same failure mode the card describes. Please either verify the suite with TIME_TEST_VENTURE=1 exported as well, or confirm that venture tests are safe to run in that shell.
  • Clean-environment check (finish line). The PR reports env -i and the job-env run passing. Those match the finish line, but the reported output is only "ALL OK through 54, then 35, 36 and 41", which doesn't show every section passing. Please confirm the exit code is 0 in both runs.
  • Minor: the PR body has "Closes #51" twice. The loop's placement before the time.py import and the seat-key list (ANTHROPIC_API_KEY, VENICE_API_KEY, CURSOR_API_KEY) match the card. No other behaviour changes are visible in the diff.

Once CI is green and the TIME_TEST_VENTURE question is answered, I'd approve.

review by haiku · claude-haiku-5-5 · via Time

**Object** The code change itself looks right. The blocker is that the card's finish line isn't met yet, and there is one scope question. - **CI not green (finish line).** The PR body says "CI has not been run on this PR yet." The card's DONE condition requires CI green, so this can't be marked done. Please run CI and post the result. - **`TIME_TEST_VENTURE` exception (.github/time/test_time.py:7).** The card says to remove every `TIME_*` variable. The PR keeps `TIME_TEST_VENTURE` on the grounds that it's the suite's own opt-in switch. The reasoning is sound, but it's a deviation from the card's wording. The finish-line check also doesn't cover it: the verified job-env run exports `TIME_WORKER`, `TIME_MODE` and `TIME_DAILY_USD` but not `TIME_TEST_VENTURE`. If a job does export it, the venture tests run inside the worker's shell, which is the same failure mode the card describes. Please either verify the suite with `TIME_TEST_VENTURE=1` exported as well, or confirm that venture tests are safe to run in that shell. - **Clean-environment check (finish line).** The PR reports `env -i` and the job-env run passing. Those match the finish line, but the reported output is only "ALL OK through 54, then 35, 36 and 41", which doesn't show every section passing. Please confirm the exit code is 0 in both runs. - **Minor:** the PR body has "Closes #51" twice. The loop's placement before the `time.py` import and the seat-key list (`ANTHROPIC_API_KEY`, `VENICE_API_KEY`, `CURSOR_API_KEY`) match the card. No other behaviour changes are visible in the diff. Once CI is green and the `TIME_TEST_VENTURE` question is answered, I'd approve. <sub>review by haiku · claude-haiku-5-5 · via Time</sub>
Owner

Evidence for the review above (no verdict from me):

  • CI. It has run and is green on this head (4db4344): ci / test (pull_request) success, alongside the worker's own time/worker-run success. "CI has not been run yet" was true when the PR was opened, seconds before CI started. The reviewer is never shown check results; that's card #52.
  • TIME_TEST_VENTURE. No workflow sets it; it's the suite's own opt-in for the venture tests, so clearing it would silently override an explicit choice by whoever runs the suite. Keeping it looks right to me, but it is a deviation from the card's wording, as the reviewer says.

This touches .github/, a protected path, so a human merges it either way.

Evidence for the review above (no verdict from me): - **CI.** It has run and is green on this head (4db4344): `ci / test (pull_request)` success, alongside the worker's own `time/worker-run` success. "CI has not been run yet" was true when the PR was opened, seconds before CI started. The reviewer is never shown check results; that's card #52. - **`TIME_TEST_VENTURE`.** No workflow sets it; it's the suite's own opt-in for the venture tests, so clearing it would silently override an explicit choice by whoever runs the suite. Keeping it looks right to me, but it is a deviation from the card's wording, as the reviewer says. This touches `.github/`, a protected path, so a human merges it either way.
teahubxyz merged commit 195c65cb55 into main 2026-10-14 21:15:49 +00:00
teahubxyz deleted branch time/card-51 2026-10-14 21:15:49 +00:00
Sign in to join this conversation.