[ticket-062] fix(versioning): read setup() version literals - #103
Conversation
Statistics: 12 files changed, 383 insertions, 12 deletions Summary: - Dirs: project=7, goal=2, tests=2, .=1 - Exts: .md=6, .py=4, .txt=1, .json=1 - A/M/D: 6/6/0 - Added: project/ticket-062/README.md, project/ticket-062/ai-codex-logs.txt, project/ticket-062/ai-codex.md, project/ticket-062/changelog.md, project/ticket-062/intent.json, project/ticket-062/preprompt.md - Symbols: _extract_version, offset, _replace_setup_version_literal, _dotted_python_name, test_partial_local_prebump_repairs_forward Added files: - project/ticket-062/README.md (+46/-0) - project/ticket-062/ai-codex-logs.txt (+7/-0) - project/ticket-062/ai-codex.md (+43/-0) - project/ticket-062/changelog.md (+12/-0) - project/ticket-062/intent.json (+100/-0) - project/ticket-062/preprompt.md (+12/-0) Modified files: - TODO.md (+7/-0) - goal/cli/version_state.py (+117/-0) - goal/cli/version_sync.py (+1/-10) - project/TICKETS.md (+1/-0) - tests/test_version_state.py (+28/-0) - tests/test_version_sync.py (+9/-2) Implementation notes (heuristics): - Type inferred from file paths + diff keywords + add/delete ratio - Scope prefers 'goal' when goal/* is touched; otherwise based on top-level dirs - For <=6 files: generate short per-file notes from added lines (defs/classes/click options/headings) - A/M/D derived from git name-status; per-file +X/-X from git numstat Co-authored-by: Koru Agent <agent@coru.dev>
stats: lines: "+8/-1 (net +7)" files: 3 complexity: "Stable complexity" Co-authored-by: Koru Agent <agent@coru.dev>
There was a problem hiding this comment.
Deterministic Validator approval for exact head d2a17c7d2890277e94e037c488ede01d77488550.
Ticket: ticket-062
Correlation ID: goal-pr-103-ticket-062-d2a17c7d28
Model: openrouter/z-ai/glm-5.2
Reviewed diff chunks: 2
Advisory LLM verdict: REQUEST_CHANGES
Advisory summary: Reviewed all 2 diff chunk(s). The PR adds AST-based parsing for setuptools setup() version literals with static analysis (no execution of setup.py). The approach is sound, but there are correctness and security concerns in the replacement logic that need attention. | This chunk adds ticket-062 governance documentation (intent.json, ai-codex.md, changelog, preprompt) and regression tests for setup.py version parsing/writing. The tests verify safe AST-based reading/writing of setuptools setup() version literals, including alias imports, multiline calls, and unrelated version keywords. All visible test assertions confirm the expected behavior: targeted version rewriting preserves unrelated content. Test results show all checks passing (3.13, 3.12, governance).
Advisory findings: _replace_setup_version_literal uses a regex pattern with prefix group (?i:[rub]*) to preserve string prefixes, but this pattern is not anchored to reject invalid prefix combinations. While ast already validated the literal as a real string constant, the replacement only uses the captured prefix if the fullmatch succeeds — if the prefix contains unusual but valid Python like rb'' or Rb'', the regex (?i:[rub]*) would match but reorders could theoretically differ. More critically, the .* in the regex with re.DOTALL could match across string boundaries in edge cases where adjacent string literals are concatenated, though AST col_offset/end_col_offset should prevent this.; The regex pattern (?P<prefix>(?i:[rub]*))(?P<quote>"""|'''|"|').*(?P=quote) with re.DOTALL and the greedy .* before (?P=quote) is fragile. If the string literal is something like '''it\'s version''' the backslash-escaped quote inside could confuse the greedy match. Since AST already identified the exact span, a more surgical approach replacing the content between the known start and end offsets would be safer.; _setup_version_literal walks the entire AST and collects ALL setup() call version keywords, then returns the earliest one. If a file has multiple setup() calls (unusual but possible in conditional branches), only the first is updated during write, but extraction reads the first. This is consistent but could surprise users with conditional setup calls.; The _replace_setup_version_literal function uses re.fullmatch on content derived from untrusted source files. While the AST already validated the span as a string literal, the regex with re.DOTALL and greedy .* combined with (?P=quote) backreference could theoretically be exploited with pathological input to cause ReDoS-style behavior. Since this only runs on user-controlled setup.py files during version synchronization (not on untrusted external input), the risk is low but the greedy quantifier should be made non-greedy or replaced with direct substring extraction between known offsets.; No code execution risk: the implementation correctly uses ast.parse() and never imports or execs setup.py, which is the correct security posture.
The LLM output above is advisory and was not used as the approval trust root.
Merge was not requested or performed.
Decision record (recomputable)
DECISION D-062-8504
TICKET ticket-062
HEAD_SHA d2a17c7d2890277e94e037c488ede01d77488550
CORRELATION_ID goal-pr-103-ticket-062-d2a17c7d28
ACTOR agent:ifuri-validator-agent[bot]
APPLIED_RULE P-CORE-015
INPUT author_login = "tom-sapletta-com"
INPUT observed_checks = ["test (3.13)=PASS","test (3.12)=PASS","governance / remote lifecycle=PASS"]
INPUT required_checks = ["test (3.12)","test (3.13)"]
INPUT required_checks_source = "env/request"
INPUT reviewer_login = "ifuri-validator-agent[bot]"
VERDICT APPROVE AUTHORITY DETERMINISTIC
REJECTED REQUEST_CHANGES BECAUSE NO_UNSAFE_CHANGE_REASON_FOUND
ADVISORY llm_verdict = "REQUEST_CHANGES" MODEL "openrouter/z-ai/glm-5.2"
ASSERT VERDICT_AUTHORITY != "ADVISORY"
Created by governed goal -a pull-request delivery.