From 847a9c36b6020359a989c978a1558497b973605b Mon Sep 17 00:00:00 2001 From: Ade Oshineye Date: Fri, 9 Oct 2026 15:49:35 +0100 Subject: [PATCH] Document Ruff rollout lessons and shared engineering guidance --- LESSONS_LEARNED.md | 85 ++++++++++++ README.md | 8 ++ docs/engineering-guidance.md | 68 ++++++++++ .../retrospectives/2026-10-09-ruff-rollout.md | 121 ++++++++++++++++++ 4 files changed, 282 insertions(+) create mode 100644 LESSONS_LEARNED.md create mode 100644 docs/engineering-guidance.md create mode 100644 docs/retrospectives/2026-10-09-ruff-rollout.md diff --git a/LESSONS_LEARNED.md b/LESSONS_LEARNED.md new file mode 100644 index 0000000..aae80f0 --- /dev/null +++ b/LESSONS_LEARNED.md @@ -0,0 +1,85 @@ +# Lessons learned + +## 2026-10-09 — Ruff rollout exposed Workers runtime and test-harness failures + +The Ruff upgrade was not sufficient to explain the red integration checks: +the failures also reproduced without the Ruff changes. Treat a lint upgrade +and a runtime repair as separate hypotheses, even when they share a PR. + +### Evidence and resolution + +[PR #2](https://github.com/adewale/python-workers-examples/pull/2) contains the +fixes. The merged revision is +[`a3192a8`](https://github.com/adewale/python-workers-examples/commit/a3192a8bdae07b8b699a5dfb9eba24d7854e94e5). +Both the [PR CI run](https://github.com/adewale/python-workers-examples/actions/runs/37918860898) +and the [post-merge CI run](https://github.com/adewale/python-workers-examples/actions/runs/37919180915) +passed the integration suite and Ruff. + +- **Runtime dependencies are not typing dependencies.** Remove typing-only + `webtypy` from the runtime dependency lists. The previous toolchain's legacy + Pyodide package-installation path failed with newer pip/urllib3. Update the + Workers CLI as part of the runtime repair rather than merely increasing a + server-startup timeout. +- **Worker import time is not request time.** FastAPI's telemetry initialization + needed entropy unavailable during Worker startup. Keep the application and + routes in `src/app.py`, and import that module inside `Default.fetch` so the + existing app is initialized in request context. +- **Unused bindings can have side effects.** D1's unused AI binding made the + tested Wrangler version request Cloudflare credentials even for local + development. Remove the binding that the example does not use; do not add + production credentials to make an otherwise local test pass. +- **Compatibility flags are part of the runtime contract.** Add + `python_workflows` alongside `python_workers`. Keep awaiting the workflow + binding calls, and test that the DAG reaches `complete`, not just that its + status endpoint returns JSON. +- **State and processes must be isolated.** A repeated Durable Objects run + inherited a message left by an earlier run. Give each test temporary storage, + use that same path when initializing its D1 database, and terminate the + entire dev-server process group rather than only the parent `uv` process. +- **Cold startup differs from a cached run.** Allow dependency downloads for + every example in CI, and notice when the startup process exits. Timeout and + diagnostic handling should not hide an immediate startup failure. + +### Tested environment and commands + +These are the versions used for the repair, not a promise that every layer is +locked indefinitely: + +| Layer | Version / policy | +| --- | --- | +| Ruff / pre-commit hook | 0.16.0, aligned with `required-version` | +| uv | 0.12.3, pinned in CI | +| workers-py | 1.17.7, pinned in the example development groups | +| Python | 3.13 selected in CI; local Docker used 3.13.14 and PR CI used 3.13.15 | +| Wrangler | 4.149.0 observed during local verification; not pinned by this repair | +| Runner | Linux Docker locally; GitHub's clean `ubuntu-latest` runner for CI | + +```sh +uv sync --dev +UV_PYTHON=3.13 CI=true uv run pytest -vv +uv run ruff check . +``` + +The PR CI result was **8 passed, 1 xfailed, 1 xpassed**; Ruff passed. +No production deployment was performed. + +### What green CI does not establish + +- `test_05_langchain` is a placeholder containing `pass`. Its XPASS is not + evidence that LangChain works. +- The dev-server fixture does not start Workers for tests marked `xfail`. + Consequently, the Workers AI expected failure does not verify the AI path. +- The suite exercises selected local examples, not every directory, remote + deployment, or platform capability. The Cron test checks the fetch response, + not a scheduled invocation. +- Floating Wrangler/runtime versions and the runner image can still change. + Future toolchain upgrades need their own recorded compatibility run. + +Follow-ups are real LangChain and Workers AI coverage, explicit toolchain +version policy, and tests for capabilities outside the current local suite. +They are not unfinished Ruff merges. + +See the [shared engineering guidance](docs/engineering-guidance.md), +[cross-project retrospective](docs/retrospectives/2026-10-09-ruff-rollout.md), +and [Workers issues lessons](https://github.com/adewale/python-workers-issues/blob/main/LESSONS_LEARNED.md) +for the reusable rules and companion findings. diff --git a/README.md b/README.md index 363643f..fc73d82 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,14 @@ The examples keep development tooling in the `dev` dependency group. SDK supplies the Worker types. Bundling `webtypy` needlessly invokes the legacy Pyodide package installer, which fails with newer pip/urllib3 versions. +## Engineering notes + +- [Lessons learned](LESSONS_LEARNED.md): dated runtime and test-harness findings. +- [Shared engineering guidance](docs/engineering-guidance.md): reusable diagnosis, + testing, toolchain, and merge practices. +- [Ruff rollout retrospective](docs/retrospectives/2026-10-09-ruff-rollout.md): + cross-project evidence, completed scope, and remaining coverage gaps. + ## Examples - [**`01-hello/`**](01-hello) — the most basic Python Worker diff --git a/docs/engineering-guidance.md b/docs/engineering-guidance.md new file mode 100644 index 0000000..0cb64d3 --- /dev/null +++ b/docs/engineering-guidance.md @@ -0,0 +1,68 @@ +# Shared engineering guidance + +Reusable rules from the [2026 Ruff rollout](retrospectives/2026-10-09-ruff-rollout.md). +This document is hosted here and linked from both Workers repositories; the +rules apply beyond Cloudflare Workers. Incident-specific versions and evidence +belong in dated retrospectives and repository lessons, not in timeless rules. + +## Diagnose before attributing + +1. Record the failing command, check category, commit SHA, and tool/runtime + versions. Separate lint, types, dependency audits, startup, and behavior. +2. Run the same check on the base revision and the proposed revision under the + same environment. Do not assume that a failure observed on a PR was caused + by its diff. +3. If unchanged source now fails, investigate resolved dependencies, generated + artifacts, runtime/runner changes, external services, and audit database + updates. Compare one suspected factor at a time before claiming causality. +4. Keep source-backed hypotheses separate from behavior actually reproduced. + A plausible explanation is not proof. + +## Make validation meaningful and repeatable + +- Test the observable outcome: bytes, headers, state transitions, or completed + work. A status code or JSON shape alone is often insufficient. +- A regression test should fail when the relevant fix is removed. Verify that + negative control when practical; otherwise state precisely what was and was + not checked. Never imply a deliberate revert test happened if it did not. +- Control external dependencies while preserving the real protocol. A local + HTTP server can test real client requests without depending on a public + service's uptime. Assert that requests actually reached it. +- Give tests independent storage, isolate process lifecycles, and run them + again to expose leaked state. Keep cold-start evidence as well as cached runs. +- Use expected failures only for identified defects reached by the test. + Assert setup and control paths before marking the known-bug path as xfail; + an unrelated connection or startup failure must not be treated as confirmation. +- Report skips, expected failures, XPASSes, placeholders, and untested paths + separately from successful behavior. A green badge is bounded evidence. + +## Control changes without weakening gates + +- Align the linter version across development dependencies, required-version + policy, pre-commit, and CI. Pin the versions meant to be reproducible, and + explicitly identify any intentionally floating toolchain layers. +- Classify runtime, development, and typing dependencies correctly. Do not + bundle a typing-only package just to satisfy development tooling. +- Fix the vulnerable dependency or incorrect call that a gate identifies. + Do not disable an audit or type check simply to make a rollout green. +- Keep changes scoped. Separate follow-up coverage and runtime work from the + claim that a tooling rollout has been completed. +- Before merging, check the exact tested head, all relevant checks, draft state, + and mergeability. Rebase only when necessary; revalidate changed heads. + Verify the resulting main-branch checks after merging. + +## Leave durable evidence + +For each incident, record the date, affected revisions, reproduction command, +tested versions, observed failure, resolution, immutable source/CI links, +validation result, and remaining limitations. Put repository-specific findings +in that repository's lessons document and link to shared rules rather than +copying an incident narrative into every project. + +Check repository visibility before publishing a cross-project inventory in a +public repository. Keep private project identities and evidence in an approved +private location; use anonymized counts in public summaries when appropriate. + +Do not turn a historical green run into a claim that every project remains +healthy forever. New dependency, service, and runtime changes require fresh +evidence. diff --git a/docs/retrospectives/2026-10-09-ruff-rollout.md b/docs/retrospectives/2026-10-09-ruff-rollout.md new file mode 100644 index 0000000..7d1edbe --- /dev/null +++ b/docs/retrospectives/2026-10-09-ruff-rollout.md @@ -0,0 +1,121 @@ +# Ruff 0.16.0 rollout retrospective + +Recorded: 2026-10-09. Rollout merges span 2026-07-26 to 2026-10-09. + +## Outcome and scope + +All **29** PRs on the rollout branch `agent/upgrade-ruff-0.16` were merged. +Autowiki was explicitly excluded. GitHub's PR inventory was checked on the +recorded date; the public links below preserve the shareable scope independently +of this chat. One private-repository PR is included only as an anonymized count. +This is a completed tooling rollout, not a claim that every repository's +current application behavior has been exhaustively tested. + +The last two merges were +[python-workers-issues #2](https://github.com/adewale/python-workers-issues/pull/2) +and [python-workers-examples #2](https://github.com/adewale/python-workers-examples/pull/2). +Their PR and post-merge checks passed. No production Workers deployment was +performed during those repairs. + +## What blocked progress + +Red checks were not all Ruff failures. The rollout also encountered dependency +audits, type-checker errors, and integration failures: + +| Project / evidence | Failing check encountered | +| --- | --- | +| [pythonbyexample #12](https://github.com/adewale/pythonbyexample/pull/12) | npm audit: high-severity sharp advisories through the Miniflare/Wrangler dependency path | +| [claude-history-explorer #12](https://github.com/adewale/claude-history-explorer/pull/12) | Production dependency audit: high-severity Hono advisories | +| [planet_cf #16](https://github.com/adewale/planet_cf/pull/16) | ty: invalid argument types | +| [xampler #1](https://github.com/adewale/xampler/pull/1) | ty: redundant cast reported as a failure | +| [olsen #9](https://github.com/adewale/olsen/pull/9) | govulncheck: reachable TIFF vulnerabilities in golang.org/x/image | +| [Workers examples #2](https://github.com/adewale/python-workers-examples/pull/2) | Legacy package installation, startup/runtime contracts, and persistent test state | +| [Workers issues #2](https://github.com/adewale/python-workers-issues/pull/2) | External header-echo availability and startup/runtime contracts | + +The Workers failures also reproduced without the Ruff changes. Comparing the +base and PR behavior prevented us from treating the linter upgrade as the +automatic explanation. The eventual repairs changed the toolchain and runtime +configuration as well as test infrastructure; see the repository-specific +evidence rather than attributing every symptom to one dependency. + +## Lessons we will reuse + +- Compare base and head in a controlled environment before assigning blame. +- Treat lint, types, audits, startup, and behavioral failures as distinct gates. +- Align tool versions, and document which other layers still float. +- Exercise real protocols against controlled services: local echoing removed + public-service uptime from CI without mocking away HTTP behavior. +- Validate work actually completed: the workflow test now waits for `complete`. +- Isolate storage and terminate complete server process groups between tests. +- Read the test and fixture before interpreting a green badge, xfail, or XPASS. +- Keep evidence and limitations beside the code, not solely in chat history. + +The operational checklist is [shared engineering guidance](../engineering-guidance.md). +Detailed findings are in the [examples lessons](../../LESSONS_LEARNED.md) and +[issues lessons](https://github.com/adewale/python-workers-issues/blob/main/LESSONS_LEARNED.md). + +## Final Workers verification + +The repair pinned Ruff 0.16.0, uv 0.12.3 in CI, and workers-py 1.17.7 in the +example development groups. CI selected Python 3.13. Local Docker verification +observed Wrangler 4.149.0; Wrangler was not pinned by this repair. + +| Repository | PR CI | Post-merge CI | Integration result in PR CI | +| --- | --- | --- | --- | +| python-workers-examples | [successful run](https://github.com/adewale/python-workers-examples/actions/runs/37918860898) | [successful run](https://github.com/adewale/python-workers-examples/actions/runs/37919180915) | 8 passed, 1 xfailed, 1 xpassed; Ruff passed | +| python-workers-issues | [successful run](https://github.com/adewale/python-workers-issues/actions/runs/37918858835) | [successful run](https://github.com/adewale/python-workers-issues/actions/runs/37919064552) | 2 passed, 3 skipped, 1 xfailed; Ruff passed | + +Merged revisions: +[examples `a3192a8`](https://github.com/adewale/python-workers-examples/commit/a3192a8bdae07b8b699a5dfb9eba24d7854e94e5), +[issues `17e2a28`](https://github.com/adewale/python-workers-issues/commit/17e2a28d4ac2d5439aaf8470866c5e7a94ededf0). +These links are dated receipts, not continuously updated health indicators. + +## Remaining work, separate from the rollout + +- Replace the LangChain placeholder test; its XPASS proves nothing about a + running LangChain Worker. +- Exercise Workers AI in a supported environment. The current xfail fixture + prevents its server from starting, so the expected failure is not validation. +- Run the three deployment-only R2 tests with explicit authorization for a + suitable deployment; they were skipped in this verification. +- Continue tracking the reproduced httpx User-Agent stripping bug. In the + issues suite, real-request assertions now run before its expected failure. +- Decide the remaining floating toolchain/version policy and validate future + upgrades, including runner-image changes. + +These are coverage or platform follow-ups, not outstanding Ruff rebases. + +## Rollout inventory + +Each of the 28 public PRs below was merged when this retrospective was recorded. +One additional private-repository PR was also merged; its identity and link are +omitted from this public document. Together they account for all 29 merges. + +- [agentic-mermaid #229](https://github.com/adewale/agentic-mermaid/pull/229) +- [aha #20](https://github.com/adewale/aha/pull/20) +- [anti-slop-writing #16](https://github.com/adewale/anti-slop-writing/pull/16) +- [atlas #35](https://github.com/adewale/atlas/pull/35) +- [audit-skill #9](https://github.com/adewale/audit-skill/pull/9) +- [cfboundary #1](https://github.com/adewale/cfboundary/pull/1) +- [cfdoctor #18](https://github.com/adewale/cfdoctor/pull/18) +- [claude-history-explorer #12](https://github.com/adewale/claude-history-explorer/pull/12) +- [geist_fabrik #82](https://github.com/adewale/geist_fabrik/pull/82) +- [good-pr #13](https://github.com/adewale/good-pr/pull/13) +- [good-readme #9](https://github.com/adewale/good-readme/pull/9) +- [good-repo #10](https://github.com/adewale/good-repo/pull/10) +- [guardrails-skill #12](https://github.com/adewale/guardrails-skill/pull/12) +- [keyboardia #72](https://github.com/adewale/keyboardia/pull/72) +- [olsen #9](https://github.com/adewale/olsen/pull/9) +- [oshineye-dev #2](https://github.com/adewale/oshineye-dev/pull/2) +- [planet_cf #16](https://github.com/adewale/planet_cf/pull/16) +- [python-workers-examples #2](https://github.com/adewale/python-workers-examples/pull/2) +- [python-workers-issues #2](https://github.com/adewale/python-workers-issues/pull/2) +- [python-workers-skill #9](https://github.com/adewale/python-workers-skill/pull/9) +- [pythonbyexample #12](https://github.com/adewale/pythonbyexample/pull/12) +- [skill-eval-harness #50](https://github.com/adewale/skill-eval-harness/pull/50) +- [skill_scanner #11](https://github.com/adewale/skill_scanner/pull/11) +- [slide-maker #13](https://github.com/adewale/slide-maker/pull/13) +- [swiss-poster-skill #17](https://github.com/adewale/swiss-poster-skill/pull/17) +- [tasche #14](https://github.com/adewale/tasche/pull/14) +- [testing-best-practices #23](https://github.com/adewale/testing-best-practices/pull/23) +- [xampler #1](https://github.com/adewale/xampler/pull/1)