Skip to content

feat(server+migrate): round-1 feedback followups#248

Merged
bokelley merged 1 commit intomainfrom
bokelley/round-1-followups
Apr 20, 2026
Merged

feat(server+migrate): round-1 feedback followups#248
bokelley merged 1 commit intomainfrom
bokelley/round-1-followups

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

Polish items deferred out of PRs #246 / #247 during expert review.

Item What Source
TokenValidator as Protocol union SyncTokenValidator + AsyncTokenValidator Protocols (their Union is TokenValidator). Fixes mypy narrowing for async validators in downstream code. constant_time_token_match gets a TypeVar so it preserves the value type. #246 code-reviewer #2
validator_from_token_map helper Takes a {raw_token: Principal} dict, returns a SyncTokenValidator that hashes at construction and does constant-time compare on every call. Plaintext tokens are NOT retained in the closure. Shrinks the auth example's load-bearing lines. #246 code-reviewer #9
--apply dirty-tree guard migrate --apply refuses to run when git status --porcelain shows uncommitted changes; --allow-dirty overrides. Fails safe when git isn't available or the path isn't in a repo (CI sandboxes, scratch envs). Prevents post-migration git diff mixing seller work with codemod rewrites. #247 python-expert #5
Versioned JSON report REPORT_SCHEMA_VERSION = 1 module constant; top-level schema_version key in JSON output. CI scripts / editors parsing the migrate output now have a stable wire contract. #247 python-expert #10 + code-reviewer #12

Tested

  • +10 tests: validator_from_token_map (match, miss, constant-time, empty map, plaintext-no-leak-from-closure); schema-version pin + JSON output assertion; --apply refuses on dirty tree / --allow-dirty overrides / proceeds when not in a git repo.
  • Full suite: 2055 passed, 22 skipped locally (2008 → 2055).
  • ruff, mypy clean across 677 source files.

Test plan

  • CI green across Python 3.10–3.13
  • Review: is SyncTokenValidator | AsyncTokenValidator Protocol union the right shape, vs. a single @overload-decorated callable? The union exports both protocols explicitly so advanced users can type their code with the specific branch they use.
  • Review: does the --allow-dirty escape hatch have the right name? Alternative: --force.
  • Review: does the JSON schema documentation sit in the right place (module docstring), or should it move to MIGRATION_v3_to_v4.md?

Related

With this merged, round-1 of salesagent feedback is fully closed. Round-2 (A2A durable-persistence hooks) is Phase 2 / future work.

🤖 Generated with Claude Code

Polish items deferred out of PRs #246/#247 during review.

auth.py: SyncTokenValidator + AsyncTokenValidator Protocols replace
the Callable[...] alias (TokenValidator = SyncTokenValidator |
AsyncTokenValidator). Fixes mypy narrowing for async validators.
constant_time_token_match uses TypeVar so return preserves value type.

auth.py: validator_from_token_map helper — takes a raw-token to
Principal map, returns a SyncTokenValidator that hashes at
construction. Plaintext tokens NOT retained in the closure.

migrate/v3_to_v4.py: --apply refuses on dirty git tree,
--allow-dirty overrides. Fails safe when git unavailable or
not-a-repo so CI sandboxes aren't blocked.

migrate/v3_to_v4.py: REPORT_SCHEMA_VERSION=1 module constant;
top-level schema_version key in JSON output. Stable wire contract
for CI scripts/editors.

+10 tests (2008 -> 2055). mypy clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit dbbc390 into main Apr 20, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant