Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .agents/skills/python-codestyle/references/code-style.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@
- **Use modern syntax**: `list[int]` not `List[int]`, `dict[str, X]` not `Dict[str, X]`,
`X | None` not `Optional[X]`, `from __future__ import annotations` only when needed for forward
references.
- **Don't hedge that syntax for an older interpreter.** `pyproject.toml` pins `target-version` /
`python_version` to 3.13 for every Python profile in this repo, and `spec/host-tools.json`
carries that as the host floor `scripts/host_gate.py` enforces, so 3.10+-only syntax (`X | None`,
`match`, etc.) needs no quoting, no `typing.Union` fallback, and no `from __future__ import
annotations` guard on that account alone. Add that import only when a real forward reference
needs it, per the bullet above. Three named exceptions carry a lower floor on purpose and say so
themselves: `scripts/skills_install.sh` and the `install-skills.*` bootstrap scripts, which must
run on whatever interpreter a host already has before this floor's toolchain exists to install
one, and `spec/resolve_description.py`, which `repo-config/configure.sh`'s own bootstrap probe
accepts down to 3.7 for the same reason, and which carries `from __future__ import annotations`
for exactly that purpose rather than out of habit. No other `spec/` code has a reason to hedge,
so that import or a quoted annotation appearing anywhere else in `spec/` is a sign this one
exception got copied rather than a pattern to follow.
Comment thread
ptr727 marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- **Don't add `# type: ignore` to silence pyright errors without a comment** explaining the
constraint. If a recurring false positive needs suppression, configure it project-wide in
`[tool.pyright]`. A new port doesn't change this, fix freshly surfaced type errors rather than
Expand Down
2 changes: 1 addition & 1 deletion .claude-plugin/fleet-skills/.source-digest
Original file line number Diff line number Diff line change
@@ -1 +1 @@
716018234e006002
598e6a55149a7fde
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@
- **Use modern syntax**: `list[int]` not `List[int]`, `dict[str, X]` not `Dict[str, X]`,
`X | None` not `Optional[X]`, `from __future__ import annotations` only when needed for forward
references.
- **Don't hedge that syntax for an older interpreter.** `pyproject.toml` pins `target-version` /
`python_version` to 3.13 for every Python profile in this repo, and `spec/host-tools.json`
carries that as the host floor `scripts/host_gate.py` enforces, so 3.10+-only syntax (`X | None`,
`match`, etc.) needs no quoting, no `typing.Union` fallback, and no `from __future__ import
annotations` guard on that account alone. Add that import only when a real forward reference
needs it, per the bullet above. Three named exceptions carry a lower floor on purpose and say so
themselves: `scripts/skills_install.sh` and the `install-skills.*` bootstrap scripts, which must
run on whatever interpreter a host already has before this floor's toolchain exists to install
one, and `spec/resolve_description.py`, which `repo-config/configure.sh`'s own bootstrap probe
accepts down to 3.7 for the same reason, and which carries `from __future__ import annotations`
for exactly that purpose rather than out of habit. No other `spec/` code has a reason to hedge,
so that import or a quoted annotation appearing anywhere else in `spec/` is a sign this one
exception got copied rather than a pattern to follow.
Comment thread
ptr727 marked this conversation as resolved.
- **Don't add `# type: ignore` to silence pyright errors without a comment** explaining the
constraint. If a recurring false positive needs suppression, configure it project-wide in
`[tool.pyright]`. A new port doesn't change this, fix freshly surfaced type errors rather than
Expand Down
6 changes: 0 additions & 6 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,6 @@ Review effort is user-controlled. The automation observes `Lite`, `Balanced`, or
- **Proved against** - the `norm` filter in `repo-config/configure.sh` on `develop` at `756a53e`.
- **Delete when** - the filter stops calling `keys_unsorted`, or nothing this check runs on carries a jq older than 1.6.

- **Splitting the fallback parse in `host-setup/agent-safety/gh-write-guard.py` a line at a time mis-reads a newline inside a quoted argument, reintroducing the false deny that path exists to remove.** Raised against the branch that made a newline end a command, on the ground that a `--body` argument holding a newline and a `git push origin develop` would have that line read as a push.
- **Disproved by** - the arm being unreachable, and then by measuring it rather than resting on that. `punctuation_chars` arrived in Python 3.6, the module uses f-strings throughout, and `install.py` refuses to install below 3.7, so an interpreter that would raise the `TypeError` fails to import the module before reaching the fallback. Simulated against a `shlex` that rejects the keyword and passes everything else through, the quoted-newline example is allowed on both paths, because splitting a line whose quoting cannot be parsed leaves the quote glued to the token and the push target reads as `develop"`, matching no branch. The shape does bite one line further out, where a three-line body whose middle line is a bare `git push origin develop` denies on the forced path, and the alternative is worse where it counts: parsing the whole command at once keeps a quoted newline intact and drops every real one, so an ordinary push followed by a `gh pr create` denies under every interpreter rather than under none.
- **Proved against** - `_git_subcommand_arglists` in `host-setup/agent-safety/gh-write-guard.py` and the interpreter floor in `host-setup/agent-safety/install.py`, on `develop` at `dbd1cdc`.
- **Delete when** - the floor drops below 3.6, or the fallback stops splitting the command a line at a time.
- **Earned anyway** - a test case rather than a change. Only `ValueError` from unbalanced quoting reaches that path in practice and nothing covered it, so a finding wrong about its own reachability was right that the path was untested.

- **A description's stale commit claims are found by extracting the bare SHAs it quotes.** Not a reviewer's finding but the method this repository's own backlog specified for the `claims` check in `scripts/pr_review.py`, recorded here because a rejected method costs the same to re-propose as a declined finding costs to re-derive, and because a backlog has a place for a claim the tree contradicts and none for a method a measurement rejects.
- **Disproved by** - running it over the 25 most recent merged pull requests, where it raised four references and all four were correct prose: a `develop` commit named as history, a SHA inside a pasted digest, and two commits in another repository written without a URL. Nothing in the shape of a bare SHA separates those from a claim, and separating them by meaning is the similarity heuristic [GOVERNANCE.md "Documentation Style Conventions"](../GOVERNANCE.md#documentation-style-conventions) rules out. A path arm measured on the same corpus is worse, flagging 54 of 215 backticked candidates, nearly all of them bare basenames and other repositories.
- **Proved against** - the 25 most recent merged pull requests as of `develop` at `756a53e`, the corpus on which the anchored verb form that ships instead raises one reference, and that one true.
Expand Down
13 changes: 13 additions & 0 deletions .github/skills/python-codestyle/references/code-style.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@
- **Use modern syntax**: `list[int]` not `List[int]`, `dict[str, X]` not `Dict[str, X]`,
`X | None` not `Optional[X]`, `from __future__ import annotations` only when needed for forward
references.
- **Don't hedge that syntax for an older interpreter.** `pyproject.toml` pins `target-version` /
`python_version` to 3.13 for every Python profile in this repo, and `spec/host-tools.json`
carries that as the host floor `scripts/host_gate.py` enforces, so 3.10+-only syntax (`X | None`,
`match`, etc.) needs no quoting, no `typing.Union` fallback, and no `from __future__ import
annotations` guard on that account alone. Add that import only when a real forward reference
needs it, per the bullet above. Three named exceptions carry a lower floor on purpose and say so
themselves: `scripts/skills_install.sh` and the `install-skills.*` bootstrap scripts, which must
run on whatever interpreter a host already has before this floor's toolchain exists to install
one, and `spec/resolve_description.py`, which `repo-config/configure.sh`'s own bootstrap probe
accepts down to 3.7 for the same reason, and which carries `from __future__ import annotations`
for exactly that purpose rather than out of habit. No other `spec/` code has a reason to hedge,
so that import or a quoted annotation appearing anywhere else in `spec/` is a sign this one
exception got copied rather than a pattern to follow.
Comment thread
ptr727 marked this conversation as resolved.
- **Don't add `# type: ignore` to silence pyright errors without a comment** explaining the
constraint. If a recurring false positive needs suppression, configure it project-wide in
`[tool.pyright]`. A new port doesn't change this, fix freshly surfaced type errors rather than
Expand Down
6 changes: 4 additions & 2 deletions repo-config/configure.sh
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,15 @@ description=""
if [ -f "$registry" ]; then
# Resolved here, not near the top, so a run with no registry (an explicit model, no hub checkout) never needs Python at all.
# The name python3 is not universal: native Windows can register a Microsoft Store stub under that name that resolves on PATH but fails when actually run, so this runs it rather than just checking PATH (docs/host-setup.md).
# The probe itself is spec/resolve_description.py's actual floor (PEP 563, Python 3.7+) rather than an arbitrary version number, so a too-old interpreter fails here with a clear message instead of a bare SyntaxError from the script.
# The probe checks 3.7+ (PEP 563, from __future__ import annotations) because that is the oldest interpreter resolve_description.py happens to parse and run on, not because 3.7 is this repo's supported floor.
# The repository floor is 3.13 (spec/host-tools.json's python3 target, enforced on the host by scripts/host_gate.py); resolve_description.py's own from __future__ import annotations is what lets it stay parseable below that, not a license for any other spec/ code to hedge for anything older.
# A too-old interpreter fails here with a clear message instead of a bare traceback from the script.
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
if python3 -c "from __future__ import annotations" >/dev/null 2>&1; then
py_cmd=(python3)
elif py -3 -c "from __future__ import annotations" >/dev/null 2>&1; then
py_cmd=(py -3)
else
Comment thread
ptr727 marked this conversation as resolved.
echo "No Python 3.7+ interpreter found (python3 or py -3). See docs/host-setup.md." >&2
echo "No Python interpreter found (python3 or py -3) able to run resolve_description.py. This repo targets Python 3.13; see docs/host-setup.md." >&2
exit 1
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
fi
# Delegates to spec/resolve_description.py rather than a third hand-rolled copy of description_errors().
Expand Down
2 changes: 1 addition & 1 deletion spec/host-tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
"probes": [["python3", "--version"], ["py", "-3", "--version"]],
"pattern": "Python (\\d+(?:\\.\\d+)*)",
"minimum": "3.13",
"why": "Every script here is standard library only, so a bare interpreter is enough and no package floor exists. The floor is the toolchain target rather than a measured breakage one version below it, which is the one entry here that reads that way and says so rather than implying a defect nobody found. pyproject.toml sets ruff target-version to py313 and mypy python_version to 3.13, so what those tools report describes 3.13 and describes no other interpreter, and a run below the floor is unverified rather than known broken. Both tools run in CI through uvx at their latest releases, beside the doc linters, the registry and spec validation, the script self-tests under coverage, and the repo and prose gates. That run judges the code rather than any host, so this floor is still a configuration choice rather than an enforced result, and a host failing it has no CI failure to point at. Two hard requirements are measured, both sit lower, and they fail differently. str.removeprefix and str.removesuffix need 3.9, and each is called where the tree actually calls it: removeprefix in scripts/prose_lint.py and spec/audit.py, removesuffix in spec/audit.py alone. An older interpreter starts, runs, and raises AttributeError when it reaches one. datetime.UTC needs 3.11 and arrives through a module-level from datetime import UTC in spec/audit.py, scripts/pr_review.py and its tests, so an older interpreter raises ImportError before any of those modules run at all. Which mode a host sees is decided by the script it runs rather than by the interpreter alone: spec/audit.py carries both and fails at import, scripts/pr_review.py carries only the import and fails the same way, and scripts/prose_lint.py carries only the call and therefore starts, runs, and fails partway through. The name rather than the version is what differs per platform, which the second probe covers.",
"why": "Every script here is standard library only, so a bare interpreter is enough and no package floor exists. The floor is the toolchain target rather than a measured breakage one version below it, which is the one entry here that reads that way and says so rather than implying a defect nobody found. pyproject.toml sets ruff target-version to py313 and mypy python_version to 3.13, so what those tools report describes 3.13 and describes no other interpreter, and a run below the floor is unverified rather than known broken. Both tools run in CI through uvx at their latest releases, beside the doc linters, the registry and spec validation, the script self-tests under coverage, and the repo and prose gates. That run judges the code rather than any host, so this floor is still a configuration choice rather than an enforced result, and a host failing it has no CI failure to point at. Two hard requirements are measured, both sit lower, and they fail differently. str.removeprefix and str.removesuffix need 3.9, and each is called where the tree actually calls it: removeprefix in scripts/prose_lint.py and spec/audit.py, removesuffix in spec/audit.py alone. An older interpreter starts, runs, and raises AttributeError when it reaches one. datetime.UTC needs 3.11 and arrives through a module-level from datetime import UTC in spec/audit.py, scripts/pr_review.py and its tests, so an older interpreter raises ImportError before any of those modules run at all. Which mode a host sees is decided by the script it runs rather than by the interpreter alone: spec/audit.py carries both and fails at import, scripts/pr_review.py carries only the import and fails the same way, and scripts/prose_lint.py carries only the call and therefore starts, runs, and fails partway through. The name rather than the version is what differs per platform, which the second probe covers. Two scripts outside this gate probe a lower number on purpose rather than by drift: repo-config/configure.sh accepts 3.7+ because that is the oldest interpreter resolve_description.py happens to run on (PEP 563), not a second floor for this repo, and scripts/skills_install.sh / host-setup/*/install-skills.* accept 3.7+ because they must run on whatever a host already has before this floor's own toolchain exists to install one; neither is evidence that spec/ or scripts/ code should hedge for anything older than 3.13.",
"source": {
"linux": "Whatever the platform provides at or above the floor, since the scripts need an interpreter and no packages, so no distribution or build is pinned here.",
"macos": "Whatever the platform provides at or above the floor, on the same reasoning as Linux.",
Expand Down