Repository navigation
Layer A: errors, presets, connection, diagnose (ADRs 0003/0004/0012) — non-breaking 0.1.x - #19
Merged
Merged
Conversation
…/0012) Additive, non-breaking 0.1.x change: four new modules, no v0 code touched. This is ADR-0007 §5 step 0 — s3dol.diagnose() ships BEFORE any resolution behaviour changes, so dependents can record what their environment resolves to (and see the v0-vs-v1 divergence table) ahead of the v1 release. - errors.py: the ADR-0004 taxonomy (KeyError family with unmangled __str__, auth errors never KeyError), pure (operation, code, status) classification incl. the body-less HEAD ambiguity moto cannot produce, per-provider override rows, tested redaction, the one translate_s3_errors seam, and the ADR-0012 ConfigurationError + S3DolWarning trees. - presets.py: Preset/Capabilities rows per ADR-0003 amended by ADR-0012 D4 (host_patterns/params/requires_endpoint/soft/config_kwargs, derived pinned gated on verified, mandatory addressing_style with 'auto' illegal, structural no-credential rule, endpoint hygiene). 15 doc-sourced rows. - connection.py: frozen picklable S3Connection (no live objects, per-instance build lock, mandatory __getstate__/__setstate__, redacting repr); pure resolve(spec, environ, aws_config) with per-field provenance, the two-rung endpoint ladder + C1/C2 guards, two-pass preset selection, omit-not-None Config semantics; credential-shape normalisation with per-shape rejections; explicit SigV4 always (the #10 root cause, fixed at this layer). - _diagnose.py: never-raising diagnose() with provenance table and the ADR-0012 D7 v0-vs-v1 differential (v0_mangle run through v1's own resolver twice). Module named _diagnose so s3dol.diagnose is unambiguously the function (a diagnose.py submodule would shadow it after first import). 134 new tier-1/2 tests + 12 doctests; full suite green. Refs #11, refs #10 (closed when the S3Store path switches to S3Connection in the v1 PR). Claude-Session: https://claude.ai/code/session_01KqivR5Be21Wh7yRdEG4fiY
This was referenced Aug 11, 2026
thorwhalen
added a commit
that referenced
this pull request
Aug 12, 2026
master moved when #19 squash-merged (and CI pushed the 0.1.14 version bump), which made every Layer A file an add/add conflict against this branch. Resolved by taking this branch's versions throughout: v1/core already contains master's Layer A **plus** the adversarial-review fixes that were applied here after #19 was opened — so master is strictly behind on those files. Concretely, the fixes that live only here (and would have been lost by taking master's side): the diagnose() credential-echo allowlist, endpoint-URL redaction at capture, the client_kwargs credential refusal, the signature_version=None refusal, the C2 soft-row scope, the deny_means_absent clock-skew guard, the credential-identity predicate, and the region-capture tightening. Verified after resolution: 300 tests + doctests pass, ruff clean, and both review-fix markers are present in the merged files. Claude-Session: https://claude.ai/code/session_01KqivR5Be21Wh7yRdEG4fiY
thorwhalen
added a commit
that referenced
this pull request
Aug 12, 2026
…testing [bump major] Implements the v1 surface per ADRs 0001-0012, on top of Layer A (#19). [bump major] — v1 is a breaking release and MUST publish as 1.0.0, not 0.1.15: the dependents (lacing, reelee) are pinned `s3dol<1`, and a default patch bump would sail straight past that pin and hand them the breaking release, which is precisely what the pin exists to prevent. Layer B: Bucket/Endpoint Collection/Reader/Store triads + ObjectHandle. The leaf owns a normalized prefix (pushed into ListObjectsV2; _key_of_id raises rather than slicing a non-matching id), key validity is checked before the wire, __len__ refuses with guidance, __delitem__ is idempotent, __contains__ resolves the body-less HEAD ambiguity with one cached HeadBucket, on_missing_bucket='assume' never probes and 'create' recovers rather than probes, and setdefault/pop re-raise ObjectArchived instead of overwriting it. ADR-0011: no key-taking public methods. Keyed capabilities are sibling stores (handles/urls/info), non-keyed operations are free functions (sub/prefixes/delete_many/delete_bucket), and a reflective conformance test enforces the rule. ADR-0011 D3b was AMENDED during implementation: the guarded url_for shim cannot serve both its call paths (dol.content_url pre-resolves the key; a delegated call does not; applying _id_of_key is right for one and double-transforms the other), so it now refuses once the leaf has been wrapped. That finding is the P1 evidence posted upstream on dol#86. Also: values/writes/reads (the closed write domain, str rejected on decidability), recipes (composition only), the store.py compat shim with a corrected de-chunker and the D7 migration warning, and s3dol.testing shipping an in-memory fake plus the exported conformance suite. A 4-lens adversarial review produced 39 findings; every reproducible one is fixed with a regression test — including two secret leaks in the migration tool itself (diagnose echoed v1 credential kwargs; env-endpoint userinfo leaked through C2 messages, warnings and reprs) and a divergence table that both cried wolf and missed the real credential-identity move. dol floor >= 0.3.63. 300 tests + doctests green on 3.10 and 3.12. Closes #10. Refs #11.
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.
What
The v1 redesign's Layer A, shipped as an additive, non-breaking 0.1.x change — no v0 module is touched. This is ADR-0007 §5 step 0:
s3dol.diagnose()ships before any resolution behaviour changes, so every dependent can run it in their real environment and record what resolves — and whether the upcoming v1 moves it.Refs #11 (tracking). Refs #10 — the SigV2 root cause is fixed at this layer (
signature_versionalways explicit, asserted structurally on the URL, never onclient.meta.configwhich lies); #10 closes when theS3Storepath switches ontoS3Connectionin the v1 PR.Modules
errors.py__str__; auth errors neverKeyError), pure(operation, code, status)classification — including the body-less HEAD ambiguity moto cannot produce — per-provider override rows, tested redaction, the singletranslate_s3_errorsseam, and ADR-0012'sConfigurationError+S3DolWarningtreespresets.pyPreset/Capabilitiesrows per ADR-0003 amended by ADR-0012 D4:host_patterns/params/requires_endpoint/soft/config_kwargs, derivedpinnedgated onverified, mandatoryaddressing_style('auto'illegal), structural no-credential rule + endpoint hygiene. 15 doc-sourced rowsconnection.pyS3Connection(no live objects; per-instance build lock; mandatory__getstate__/__setstate__; redacting repr) and the pureresolve(spec, environ, aws_config)with per-field provenance, the two-rung endpoint ladder + C1/C2 guards, two-pass preset selection, and omit-not-NoneConfigsemantics. Credential shapes normalised with per-shape rejection messages_diagnose.pydiagnose()printing the provenance table and the ADR-0012 D7 v0-vs-v1 differential (_v0_manglerun through v1's own resolver twice, so resolver imprecision cancels)Interpretive choices (flagged for review)
presetis a field onS3Connection. ADR-0012 D1's dataclass listing omits it, but D3 rung 2 ("the bound preset's endpoint") and D4 ("named > detected") require it to be nameable; the pre-0012 architecture sketch has it. Treated the omission as an oversight.softrows (aws, generic-s3) are interpreted as: never win detection, never trigger C2, and theiraddressing_styleis reported but omitted fromConfig— reconciling "every row must state it" with "omit preserves the profile channel / byte-identical for endpoint-passing callers"._diagnose.py(architecture.md saysdiagnose.py): a publicdiagnosesubmodule shadows thes3dol.diagnosefunction after first import (and in ≥3.7,import s3dol.diagnose as xbinds the package attribute). Private module removes the trap;s3dol.diagnoseis unambiguously the function.MissingEndpointeven whenrequires_endpoint=False(e.g.preset='r2'withoutaccount_id) — otherwise it would silently fall through to AWS, the exact failure C1 exists to prevent.Tests
134 new tier-1/2 tests + 12 doctests; full suite green (
137 passed, 5 deselected— the deselected 5 are the pre-existing endpoint-gated v0 tests). Includes: the full ladder against fake environ dicts (purity asserted), all credential-shape rejections, pickle round-trips incl. cached-client dropping, an 8-thread build race (one client), C1/C2, detection/fallbacks, secret-leak assertions, and a moto proof that presigning is SigV4 in us-east-1 with an injected-credential identity check.After merge
Dependents should run, in their deployment environment:
https://claude.ai/code/session_01KqivR5Be21Wh7yRdEG4fiY