Skip to content

feat: system-aware tiered parallel pytest with cross-platform hardening#2877

Closed
LahkLeKey wants to merge 36 commits into
github:mainfrom
LahkLeKey:feat/upstream-parallel-pytest-ready
Closed

feat: system-aware tiered parallel pytest with cross-platform hardening#2877
LahkLeKey wants to merge 36 commits into
github:mainfrom
LahkLeKey:feat/upstream-parallel-pytest-ready

Conversation

@LahkLeKey
Copy link
Copy Markdown
Contributor

@LahkLeKey LahkLeKey commented Jun 6, 2026

Description

See below AI Disclosure for more details

Testing

  • Tested locally with uv run specify --help
    • Ran existing tests with uv sync && uv run pytest (optionally uv run pytest --parallel --parallel-tier medium)
  • Tested with a sample project (if applicable)

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)
  • If AI posted PR comments on my behalf, each comment includes explicit "Posted on behalf of @ by (model: )" attribution

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Continuation for LahkLeKey#2

Summary

This PR introduces system-aware optional parallel test execution for Spec Kit's pytest suite and hardens associated cross-platform tests and review-process hygiene.

Main Prompt Loop (After Feat Scaffold)

address all comments and leave a resolved by for comment, keep track of all of these add on so we ensure we do not reach a feedback loop in the main pr description update copilot guideance

What changed

  • Added bounded parallel test mode via pytest flags:

    • --parallel

    • --parallel-tier (low|medium|high)

    • --parallel-max-workers

  • Implemented worker sizing based on system constraints:

    • effective CPU count

    • available/total memory

    • cgroup v2 CPU quota (/sys/fs/cgroup/cpu.max)

    • cgroup v1 CPU quota (cpu, cpu,cpuacct, cpuacct,cpu layouts)

    • platform caps by tier

  • Wired xdist integration safely in tests/conftest.py:

    • respects explicit -n/--numprocesses (including -n auto)

    • skips injection/reinvoke when xdist is disabled (-p no:xdist / -pno:xdist)

    • handles -- sentinel correctly by injecting xdist args before positional boundary

    • reports effective worker selection in pytest header

  • Added/updated regression coverage for:

    • cgroup quota parsing behavior

    • zero-memory boundaries

    • xdist disable and -- sentinel edge cases

    • cross-platform path handling in bash-based tests

    • CWD-safe relative installer path tests (monkeypatch.chdir)

Validation

Fresh full-suite verification on this branch:

  • uv run pytest -q --parallel --parallel-tier high -rs

  • Result: 3669 passed

Review quality controls / loop prevention evidence

Source hardening was completed first in fork PR LahkLeKey/spec-kit#2 and then squashed for upstream submission.

Evidence used before opening this upstream PR:

  • All active review findings in the fork PR were answered with on-behalf inline follow-ups including explicit Resolved by <sha> references.

  • A persistent "Review Fix Ledger" was backfilled into the fork PR description to track additive review fixes across rounds and reduce recurrence loops.

  • The final outstanding unresolved thread in the fork PR has an explicit Resolved by reply from the PR owner and no pending unaddressed action item.

Scope

  • Test/workflow changes only; no production runtime behavior changes outside test harness/support scripts.

Review Fix Ledger (On Behalf Of @LahkLeKey)

This section tracks additive review-driven fixes to avoid recurring feedback loops across rounds.

Commit Trigger / Review focus Code changes Validation
127ac67 Initial upstream hardening sweep after fork-review carryover Consolidated cross-platform path assertions, tightened Windows test behavior, and aligned integration assertions Focused touched-suite run (green)
a227754 Upstream comment batch: self-upgrade Windows semantics, xdist arg routing, path-helper permissiveness, interpreter fallback docs Added Windows os.access patching in self-upgrade tests, shared conftest helpers for argument parsing/injection, tightened path helper behavior, interpreter-generic resolver comments Focused suite run before regression and follow-up triage
42571cc Regression closure from prior tightening + open-thread closure pass Fixed malformed duplicated-drive Windows path strings in shared helper, preserved explicit --dist intent while defaulting to worksteal only when unset uv run pytest tests/test_self_upgrade_execution.py tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_timestamp_branches.py tests/test_setup_plan_no_overwrite.py tests/integrations/test_cli.py -q (239 passed)
b12061f Latest upstream comment batch: POSIX page-size portability, Windows PATH delimiter, parallel-hook complexity, and timestamp long-path behavior Added SC_PAGESIZE fallback for sysconf page-size detection, used os.pathsep in Windows PATH shim setup, removed subprocess pytest reinvocation pathway from conftest, replaced silent Windows early-return with explicit pytest.xfail for long-path limitation uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_timestamp_branches.py -q (133 passed, 1 xfailed)
b87e7c6 Follow-up upstream comment batch: autoload+explicit xdist, Bash PATH semantics, chmod assertion brittleness, runtime-exception realism, UNC slash handling Enabled --parallel injection under plugin autoload disable when xdist is explicitly enabled, restored Bash PATH separator usage in Bash subprocess test, loosened Windows chmod assertion to stable safety guarantees, skipped unreachable InvalidMetadataError simulation on runtimes lacking that class, preserved UNC leading // in path normalization and added path-utils unit tests uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/integrations/test_cli.py tests/test_self_upgrade_detection.py tests/test_path_utils.py -q (176 passed, 1 skipped)
8bb0ca4 Current upstream comment batch: UNC over-prefix edge, xdist plugin-name precision, and stale interpreter fallback wording Fixed UNC normalization for over-prefixed slash forms (////server/...), replaced substring-based xdist enable detection with exact plugin-name matching (xdist / xdist.*), and updated bash resolver fallback comment to reflect generic python interpreter availability uv run pytest tests/test_parallel_workers.py tests/test_path_utils.py tests/test_setup_tasks.py -q (55 passed)
55819c7 Latest upstream comment batch: Bash PATH portability nuance and non-POSIX installer executability semantics Clarified Windows/Git Bash PATH handling in setup-tasks shim test by preserving inherited PATH bytes while prepending shim; removed chmod-dependent assumptions from non-executable installer tests where executability is already controlled via mocked os.access uv run pytest tests/test_setup_tasks.py tests/test_self_upgrade_execution.py -q (46 passed) plus targeted checks (2 passed)
4c2a128 Latest upstream comment batch: POSIX-leading-slash normalization, empty-PATH separator safety, and chmod kwargs robustness Restricted UNC preservation to native Windows UNC input forms (avoids POSIX ///... mis-normalization), avoided trailing : when inherited PATH is empty in Git Bash shim setup, and made Windows chmod assertion resilient to positional/keyword mode invocation uv run pytest tests/test_path_utils.py tests/test_setup_tasks.py tests/integrations/test_cli.py -q (105 passed)
1ce7a90 Latest upstream comment batch: slash-normalized UNC handling, sentinel-safe --parallel, and chmod target precision Preserved slash-normalized UNC inputs while still collapsing POSIX triple-leading slashes, limited --parallel detection to args before --, and tightened Windows chmod assertions to match the plan-template target (final or expected temp-write form) uv run pytest tests/test_path_utils.py tests/test_parallel_workers.py tests/integrations/test_cli.py tests/test_setup_tasks.py -q (135 passed)
1c95074 Latest upstream comment batch: unknown-platform worker caps, setup-tasks path assertion duplication, path-from-bash trimming robustness, and Windows template-write verification Defaulted unknown platform OS caps to the most permissive known cap, added path_from_bash_output trimming for quotes/whitespace/CRLF, extracted shared TASKS_TEMPLATE assertion helper in setup-tasks tests, and strengthened Windows integration assertion with explicit output-file existence check uv run pytest tests/test_parallel_workers.py tests/test_path_utils.py tests/test_setup_tasks.py tests/integrations/test_cli.py -q (137 passed)
ba72907 Latest upstream comment batch: non-Windows path equivalence robustness, resolver fallback branch coverage, and early-hook mutation coverage Updated setup-tasks path helper to compare resolved equivalent paths on non-Windows, added bash-backed tests for python interpreter fallback/CRLF preset-id trimming/deterministic parse-failure scan, and added focused tests for pytest_load_initial_conftests sentinel-safe injection behavior uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py -q (142 passed)
d7ba3f6 Latest upstream comment batch: PATH separator portability concern, strict python3→python fallback proof, /tmp mapping robustness, and early max-workers validation alignment Switched setup-tasks shim prepend to os.pathsep in env composition, made python3-missing test unambiguous by constraining PATH to shim-only for fallback proof, made /tmp mapping fall back to raw path when mapped temp candidate is absent, and aligned early --parallel-max-workers parsing with UsageError validation semantics plus focused tests uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py -q (144 passed)
f78887f Latest upstream comment batch: over-prefixed slash UNC normalization, deterministic tmp mapping policy, and duplicate settings-compute concerns Preserved over-prefixed slash UNC-like inputs while still keeping POSIX triple-slash behavior, made /tmp conversion deterministic/configurable via SPECKIT_BASH_TMPDIR, cached early parallel settings to avoid duplicate compute passes, and added focused regression tests for early-hook validation and compute-once behavior uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py -q (147 passed)
0bb271f Latest upstream comment batch: xdist worker early-hook guard, Windows memory helper deduplication, Git Bash helper consolidation, and docs table rendering Added xdist worker no-op guard in early injection path, deduplicated MEMORYSTATUSEX handling, moved host-to-bash path conversion into shared path utils, and fixed the CONTRIBUTING markdown table rendering uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py -q (150 passed)
8e07002 Latest upstream comment batch: bash PATH separator portability, shared Windows memory probe reuse, and clearer UNC branch intent Switched the Git Bash shim PATH composition to : with shared bash-path conversion, reused _read_windows_memory_status() from both Windows memory detection paths, and rewrote UNC normalization checks into explicit named booleans for readability uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_path_utils.py -q (73 passed)
822e9f8 PR scratch-file hygiene add-on: ignore .tmp_* artifacts so ad hoc review/PR files are not accidentally committed Added .tmp_* to the root .gitignore alongside the existing *.tmp rule so temporary PR/body/thread snapshots stay out of version control git diff --check -- .gitignore
e00e647 Upstream comment batch: hidden preset-dir scan semantics and PR template checklist nesting Excluded dot-directories from the find-based preset fallback scan to preserve prior hidden-directory behavior, and restored PR template checklist nesting so the rendered hierarchy remains stable uv run pytest tests/test_setup_tasks.py -q (27 passed)
71e9e04 Upstream comment batch: PR template tab indentation, PATH override helper resilience, python-fallback shim contract coverage, and single-worker --parallel no-op behavior Replaced tab indentation with spaces in PR template checklist nesting, changed _run_bash_resolve_template PATH override to prepend instead of replace, strengthened python-fallback shim test to validate -c and SPECKIT_REGISTRY, and made --parallel skip xdist injection/requirement when computed workers resolve to 1 (with focused regression tests) uv run pytest tests/test_setup_tasks.py tests/test_parallel_workers.py -q (66 passed)
100b994 Upstream comment batch: process-substitution robustness for deterministic preset fallback scans Guarded the `find ... sortprocess-substitution pipelines in preset fallback scans with
9d3ddbc Upstream comment batch: Python 3 probe for bash resolver selection and Windows chmod spy isolation Added a resolve_template_python_cmd() helper that only accepts python when it reports Python 3 before registry parsing, added a regression test that skips a non-Python3 python shim and falls back deterministically, and made the Windows chmod test patch Path.chmod as a no-op spy instead of executing real chmod uv run pytest tests/test_setup_tasks.py tests/integrations/test_cli.py -q (105 passed)
1c10388 Upstream comment batch: exact xdist disable matching, BusyBox-friendly preset scan predicates, and scoped auth stat spying Tightened xdist disable parsing to exact no:xdist / no:xdist.* matches, replaced find ... -not -name with portable ! -name predicates in preset scans, and scoped the auth Path.stat patch so it delegates to the real implementation for non-target paths while keeping the warning-path assertion focused uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_authentication.py -q (149 passed)
afd927d Upstream comment batch: deterministic interpreter-path branch coverage in setup-task tests and python command probing consistency Added Python 3 runnability/version checks for both python3 and python interpreter selection, added deterministic PATH-replacement support in setup-task resolver test helper, and hardened fallback/interpreter shim tests to guarantee intended branch coverage across host environments uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_authentication.py -q (150 passed)
005a5da Upstream comment batch: drive-letter false-positive risk, interpreter-probe repeated overhead, and brittle auth stat target matching Made drive stripping in normalized_parts() Windows-only to preserve non-Windows drive distinctions, memoized resolve_template_python_cmd() interpreter selection per shell session to avoid repeated process probes, and changed auth test stat-target matching to normalized absolute string-path comparison for robust derived-path handling uv run pytest tests/test_path_utils.py tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_authentication.py -q (159 passed)
8e922e0 Upstream comment batch: subshell cache persistence, xdist autoload guidance specificity, and Windows drive-letter mismatch masking Refactored bash resolver callers to avoid command-substitution subshells so interpreter-cache state persists, split xdist --parallel error branching to report install guidance when xdist is absent and autoload guidance only when installed-but-not-loaded, and preserved drive components in normalized path comparisons to prevent cross-drive false positives on Windows uv run pytest tests/test_setup_tasks.py tests/test_parallel_workers.py tests/test_path_utils.py tests/test_authentication.py -q (163 passed)
7af875d Upstream comment batch: preset-order drift risk, global os.name mutation in auth test, and missing direct resolve_template_content edge-case coverage Extracted shared preset-order iteration helper consumed by both resolve_template and resolve_template_content, pre-warmed interpreter selection in parent shell to preserve cache across process-substitution calls, removed direct os.name monkeypatching in auth warning test via scoped patch.object, and added direct resolve_template_content CRLF/fallback-determinism regressions uv run pytest tests/test_setup_tasks.py tests/test_authentication.py tests/test_parallel_workers.py tests/test_path_utils.py -q (165 passed)
0b201ac Upstream comment batch: cached-interpreter mismatch in manifest parsing, registry tie-order determinism, and auth-test fixture hygiene Updated preset registry sorting to tie-break by (priority, preset_id) for deterministic ordering, made resolve_template_content manifest parsing use cached selected Python command instead of hard-coded python3, removed unused monkeypatch fixture parameter, and added regressions for tie-order selection and manifest parsing through Python fallback path uv run pytest tests/test_setup_tasks.py tests/test_authentication.py tests/test_parallel_workers.py tests/test_path_utils.py -q (167 passed)
7727673 Upstream comment batch: UNC-vs-non-UNC false positives, preset-id path traversal hardening, ineffective find-pipeline sed stage, and Windows chmod behavior preservation Added UNC sentinel handling in normalized-part comparisons so UNC and non-UNC absolute paths do not compare equal, hardened preset-id handling by skipping unsafe registry IDs before path construction, removed ineffective trailing-slash sed stage from preset scan pipeline, restored Windows chmod test patching with wraps=Path.chmod, and added regressions for UNC mismatch and unsafe preset-id skipping in both path/content resolvers uv run pytest tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py tests/test_authentication.py tests/test_parallel_workers.py -q (247 passed)
1858a93 Upstream comment batch: per-preset state leakage risk in composition loop Made parse-failure reset of strategy/manifest_file explicit per iteration in resolve_template_content and added regression coverage ensuring manifest state does not leak between presets uv run pytest tests/test_setup_tasks.py tests/test_path_utils.py tests/test_authentication.py tests/integrations/test_cli.py tests/test_parallel_workers.py -q (248 passed)
45d0fbd Upstream comment batch: mixed-type priority sort resilience, over-broad .. preset-id rejection, non-Windows // UNC marker scope, and PR checklist nesting Coerced registry priority values to safe integer sort keys (with default fallback) to keep deterministic ordering on mixed types, narrowed preset-id safety rule to traversal-relevant cases while allowing benign IDs like v1..0, scoped UNC marker behavior to Windows in path-part comparison logic, restored nested checklist indentation in PR template, and added regressions for mixed-type priority ordering and benign double-dot preset IDs uv run pytest tests/test_setup_tasks.py tests/test_path_utils.py tests/test_authentication.py tests/integrations/test_cli.py tests/test_parallel_workers.py -q (250 passed)
2b5fa8e Upstream comment batch: duplicate bash resolver test helper logic Consolidated duplicated _run_bash_resolve_template* helper implementations into a single shared _run_bash_template_resolver(...) with thin wrappers for resolve_template and resolve_template_content to reduce drift risk while preserving call-site clarity uv run pytest tests/test_setup_tasks.py tests/test_self_upgrade_execution.py -q (61 passed)
015b976 Upstream comment batch: deterministic no-sentinel parallel arg placement and redundant template resolver probe Changed early xdist arg injection to place injected args adjacent to --parallel when no -- sentinel is present, removed redundant resolve_template_python_cmd invocation inside _iter_preset_ids_ordered by reusing the pre-selected cached command, and added regression coverage for deterministic adjacent injection behavior uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py -q (81 passed)
093b04b Upstream comment batch: manifest file: traversal hardening and import duplication cleanup Added _is_safe_manifest_relative_path to reject absolute/drive-prefixed paths and any .. segment after slash/backslash normalization, switched manifest candidate gating in resolve_template_content to that helper, added direct bash regression coverage for unsafe/safe manifest path inputs, and consolidated tests._path_utils imports into one statement uv run pytest tests/test_setup_tasks.py tests/test_path_utils.py -q (63 passed)
78d7988 Upstream comment batch: manifest parser CRLF handling, Windows device-path normalization, and missing early-option values Trimmed trailing \r from manifest parser strategy/manifest_file fields before safety checks and candidate lookup, excluded \\?\\ and \\.\\ prefixed paths from UNC normalization/sentinel handling, raised explicit pytest.UsageError when --parallel-tier or --parallel-max-workers is provided without a value, and added focused regressions across setup-tasks/path-utils/parallel workers uv run pytest tests/test_setup_tasks.py tests/test_parallel_workers.py tests/test_path_utils.py -q (111 passed)
da4953a Upstream comment batch: manifest drive-prefix false positive and slash-form device-path normalization invariance Narrowed manifest drive-path rejection to drive-rooted forms (X:/... and X:\\...) so benign relative names like a:overlay.md are allowed, extended device-path detection to recognize slash-form prefixes (//?/, //./) before UNC classification, and added regressions for both guard behaviors uv run pytest tests/test_setup_tasks.py tests/test_path_utils.py -q (68 passed)

Open Review Thread Status

  • All currently open upstream review threads have explicit inline replies: Resolved by 42571cc.
  • Latest comment batch also has explicit inline replies: Resolved by b12061f.
  • Current comment batch also has explicit inline replies: Resolved by b87e7c6.
  • Latest comment batch also has explicit inline replies: Resolved by 8bb0ca4.
  • Latest comment batch also has explicit inline replies: Resolved by 55819c7.
  • Latest comment batch also has explicit inline replies: Resolved by 4c2a128.
  • Latest comment batch also has explicit inline replies: Resolved by 1ce7a90.
  • Latest comment batch also has explicit inline replies: Resolved by 1c95074.
  • Latest comment batch also has explicit inline replies: Resolved by ba72907.
  • Latest comment batch also has explicit inline replies: Resolved by d7ba3f6.
  • Latest comment batch also has explicit inline replies: Resolved by f78887f.
  • Latest comment batch also has explicit inline replies: Resolved by 0bb271f.
  • Latest comment batch also has explicit inline replies: Resolved by 8e07002.
  • Latest comment batch also has explicit inline replies: Resolved by 822e9f8.
  • Latest comment batch also has explicit inline replies: Resolved by e00e647.
  • Latest comment batch also has explicit inline replies: Resolved by 71e9e04.
  • Latest comment batch also has explicit inline replies: Resolved by 100b994.
  • Latest comment batch also has explicit inline replies: Resolved by 9d3ddbc.
  • Latest comment batch also has explicit inline replies: Resolved by 1c10388.
  • Latest comment batch also has explicit inline replies: Resolved by afd927d.
  • Latest comment batch also has explicit inline replies: Resolved by 005a5da.
  • Latest comment batch also has explicit inline replies: Resolved by 8e922e0.
  • Latest comment batch also has explicit inline replies: Resolved by 7af875d.
  • Latest comment batch also has explicit inline replies: Resolved by 0b201ac.
  • Latest comment batch also has explicit inline replies: Resolved by 7727673.
  • Latest comment batch also has explicit inline replies: Resolved by 1858a93.
  • Latest comment batch also has explicit inline replies: Resolved by 45d0fbd.
  • Latest comment batch also has explicit inline replies: Resolved by 2b5fa8e.
  • Latest comment batch also has explicit inline replies: Resolved by 015b976.
  • Latest comment batch also has explicit inline replies: Resolved by 093b04b.
  • Latest comment batch also has explicit inline replies: Resolved by 78d7988.
  • Latest comment batch also has explicit inline replies: Resolved by da4953a.
  • Remaining unresolved state is reviewer-owned conversation resolution status, not missing engineering action.

POSIX vs Git Bash Guard

  • When tests run Bash subprocesses on Windows, preserve inherited PATH bytes and only prepend required shim paths; do not normalize delimiters globally.
  • Treat UNC normalization as a Windows-specific input behavior (native \\server\share forms), not a generic rule for any leading // POSIX path.
  • Prefer assertions against behavioral guarantees (mode, directory, error contract) over implementation-detail assumptions that vary across POSIX vs Git Bash runtimes.

Scope Creep Guard (Copilot Review Guidance)

  • Primary objective: deliver this feature to completion with high-confidence correctness and full test coverage for the touched parallel-workflow/path-normalization surface.
  • In scope: defects that directly affect --parallel behavior, worker sizing, xdist argument flow, cross-platform path correctness, and failing/fragile tests in touched files.
  • Out of scope for this PR: unrelated refactors, style-only churn, speculative architecture changes, and broad feature additions not required by failing tests or stated requirements.
  • Review acceptance rule: each requested change must map to one of: a reproducible failure, a cross-platform correctness bug, a regression risk, or explicit feature requirement.
  • Copilot anti-nitpick rule: do not request or apply style/readability-only tweaks unless they are directly required to fix a failing test, prevent a concrete regression, or satisfy an explicit acceptance criterion for this feature.
  • Evidence rule: every non-trivial review request should include a failing command/test case, a concrete platform incompatibility, or a precise behavioral risk statement.
  • POSIX/Git Bash tie-breaker: when a suggestion is primarily stylistic but risks destabilizing Windows Git Bash path/process behavior, defer it unless there is failing evidence.
  • Duplicate-feedback suppression: if a concern is already covered by the Review Fix Ledger or Scope/Guard rules, do not reopen it unless new failing evidence is provided.
  • Reproducibility rule: for POSIX-vs-Git-Bash concerns, include an explicit repro command and expected/actual output delta before requesting additional churn.
  • Re-proof rule: repeated claims previously addressed in this PR must cite a new failing command/result pair against current HEAD before requesting additional edits.
  • Low-churn preference: when a reviewer comment can be satisfied by consolidating an existing helper or tightening an existing assertion, prefer that over introducing new bespoke code paths.
  • Ledger lock rule: if a later comment only rephrases a fix already captured in the Review Fix Ledger, treat the ledger entry and resolved-by reply as sufficient unless the new review includes a failing command against current HEAD.
  • Deferral rule: useful but non-blocking improvements should be logged as follow-up issues to avoid rabbit-hole expansion in this PR.
  • Shell subshell cache rule: when introducing bash-level memoization, do not consume caching helpers through command substitution ($(...)) if cache state must persist; prefer in-shell function calls that set/read global state directly.
  • Test isolation rule: do not mutate process-global platform markers like os.name directly in tests; use narrowly scoped module-level patch contexts or dedicated predicates to avoid cross-test side effects.
  • Resolver parity rule: keep resolve_template and resolve_template_content on the same interpreter-selection and preset-ordering helpers; avoid hard-coded interpreter calls in one path that bypass fallback/caching in the other.
  • Loop state rule: when iterating over preset entries, initialize and explicitly reset per-entry parsing state (strategy, manifest_file, similar locals) so parse failures cannot leak values into subsequent entries.
  • Priority coercion rule: when sorting registry metadata from user-editable JSON, coerce priority-like values to safe numeric keys with deterministic tie-breaks so mixed types do not trigger fallback-path drift.
  • Test helper dedupe rule: when two test helpers differ only by invoked function name, use a single shared helper with thin wrappers to keep env/path/quoting behavior in sync.
  • Arg placement rule: when injecting early pytest args, place injected flags adjacent to their triggering option (or before --) instead of appending at the end to keep ordering deterministic.
  • Manifest path rule: treat preset-manifest file: values as strict relative paths by normalizing separators and rejecting any .. segment or absolute/drive-prefixed form before candidate resolution.
  • Manifest parse rule: normalize parser fields for cross-platform line endings (trim trailing \r) before path validation and filesystem checks.
  • Windows device-path rule: do not classify \\?\\... or \\.\\... paths as UNC-equivalent in normalization/assertion helpers.
  • Early option rule: when parsing --parallel-tier or --parallel-max-workers, raise an immediate usage error if the option is present without a value.
  • Drive-prefix rule: in manifest relative-path guards, reject drive-rooted forms (X:/..., X:\\...) without over-matching benign colon-bearing relative names.
  • Device-prefix invariance rule: normalize Windows device paths consistently for both backslash and slash representations (\\?\\... and //?/..., \\.\\... and //./...) before UNC checks.
  • Main prompt loop: "address all comments and leave a resolved by for comment, keep track of all of these add on so we ensure we do not reach a feedback loop in the main pr description update copilot guideance"

2026-06-06 Add-on (commit 42571cc)

  • Fixed Windows malformed shell path handling in tests/_path_utils.py by trimming nested drive-root prefixes and removing permissive tail-only fallback to keep path assertions strict.
  • Ensured pytest parallel mode only overrides dist strategy when --dist was not explicitly provided, preserving user intent while keeping worksteal defaulting behavior.
  • Stabilized Windows self-upgrade execution tests by patching os.access in relative/absolute installer path scenarios under mocked execute bits.
  • Updated scripts/bash/common.sh resolver comments to stay interpreter-generic now that python3/python fallback is supported.
  • Validation: uv run pytest tests/test_self_upgrade_execution.py tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_timestamp_branches.py tests/test_setup_plan_no_overwrite.py tests/integrations/test_cli.py -q (239 passed).

2026-06-06 Add-on (commit b12061f)

  • Addressed sysconf portability in tests/_parallel.py by trying both SC_PAGE_SIZE and SC_PAGESIZE before computing total memory.
  • Updated Windows PATH handling in tests/test_setup_tasks.py to prepend shim paths using os.pathsep instead of hardcoded :.
  • Reduced parallel-mode complexity in tests/conftest.py by removing subprocess-based pytest reinvocation and relying on existing injection/configure hooks.
  • Converted Windows long-path early return to explicit expected-failure signaling (pytest.xfail) in tests/test_timestamp_branches.py.
  • Validation: uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_timestamp_branches.py -q (133 passed, 1 xfailed).

2026-06-06 Add-on (commit b87e7c6)

  • Addressed explicit xdist enablement under PYTEST_DISABLE_PLUGIN_AUTOLOAD in tests/conftest.py so --parallel still injects workers when xdist is intentionally loaded via -p xdist.
  • Restored Bash PATH separator semantics (:) for the shim-based Bash subprocess test in tests/test_setup_tasks.py.
  • Relaxed brittle Windows chmod assertion in tests/integrations/test_cli.py to assert safe mode and target directory without coupling to temp filename implementation details.
  • Updated editable-metadata test realism in tests/test_self_upgrade_detection.py by skipping on runtimes without InvalidMetadataError instead of fabricating an exception class.
  • Preserved UNC leading double-slash semantics in tests/_path_utils.py and added direct helper coverage in tests/test_path_utils.py.
  • Validation: uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/integrations/test_cli.py tests/test_self_upgrade_detection.py tests/test_path_utils.py -q (176 passed, 1 skipped).

2026-06-06 Add-on (commit 8bb0ca4)

  • Fixed UNC normalization in tests/_path_utils.py for over-prefixed slash inputs so replacement output stays canonical (//server/share/...), not triple-slash.
  • Tightened explicit xdist enable detection in tests/conftest.py to exact plugin names (xdist, xdist.*) and avoid substring false positives.
  • Added regression coverage for xdist matching precision in tests/test_parallel_workers.py and UNC over-prefix normalization in tests/test_path_utils.py.
  • Updated fallback wording in scripts/bash/common.sh to reflect missing generic python interpreter availability, not just python3.
  • Validation: uv run pytest tests/test_parallel_workers.py tests/test_path_utils.py tests/test_setup_tasks.py -q (55 passed).

2026-06-06 Add-on (commit 55819c7)

  • Updated tests/test_setup_tasks.py Windows shim PATH handling to preserve inherited PATH bytes while prepending the shim path for Bash subprocess execution, with an explicit comment documenting the drive-letter delimiter caveat.
  • Removed chmod-dependent setup from path-specific non-executable installer tests in tests/test_self_upgrade_execution.py, relying on mocked os.access(..., X_OK) as the cross-platform executability control.
  • Validation: uv run pytest tests/test_setup_tasks.py tests/test_self_upgrade_execution.py -q (46 passed) and targeted regression checks (2 passed).

2026-06-06 Add-on (commit 4c2a128)

  • Refined tests/_path_utils.py so UNC-preserving behavior applies only to native Windows UNC-style inputs, preventing POSIX leading-slash paths (for example ///usr/bin) from being pinned to a double-slash prefix.
  • Updated tests/test_setup_tasks.py to avoid appending a trailing PATH separator when inherited PATH is empty.
  • Hardened tests/integrations/test_cli.py Windows chmod assertion to accept positional or keyword mode invocation.
  • Added/updated path normalization regression coverage in tests/test_path_utils.py.
  • Validation: uv run pytest tests/test_path_utils.py tests/test_setup_tasks.py tests/integrations/test_cli.py -q (105 passed).

2026-06-06 Add-on (commit 1ce7a90)

  • Updated tests/_path_utils.py to preserve slash-normalized UNC inputs (//server/share/...) while continuing to normalize POSIX triple-leading-slash paths correctly.
  • Updated tests/conftest.py so pytest_load_initial_conftests only honors --parallel before the -- sentinel.
  • Tightened Windows chmod assertion targeting in tests/integrations/test_cli.py to require the expected plan-template target path shape.
  • Added regression coverage in tests/test_parallel_workers.py and tests/test_path_utils.py.
  • Validation: uv run pytest tests/test_path_utils.py tests/test_parallel_workers.py tests/integrations/test_cli.py tests/test_setup_tasks.py -q (135 passed).

2026-06-06 Add-on (commit 1c95074)

  • Updated tests/_parallel.py to use the most permissive known OS cap when platform_name is unrecognized, avoiding unnecessary throttling on non-Windows unknown platforms.
  • Updated tests/_path_utils.py so path_from_bash_output trims shell artifacts (quotes, whitespace, CRLF) before path conversion.
  • Extracted _assert_tasks_template_matches(...) in tests/test_setup_tasks.py to centralize repeated POSIX-vs-Windows shell path assertions.
  • Added explicit written.is_file() verification in tests/integrations/test_cli.py for Windows template-write safety checks.
  • Added regression coverage in tests/test_parallel_workers.py and tests/test_path_utils.py.
  • Validation: uv run pytest tests/test_parallel_workers.py tests/test_path_utils.py tests/test_setup_tasks.py tests/integrations/test_cli.py -q (137 passed).

2026-06-06 Add-on (commit ba72907)

  • Updated _assert_tasks_template_matches(...) in tests/test_setup_tasks.py to compare resolved-equivalent paths on non-Windows, preventing false negatives when shell output contains equivalent non-canonical segments.
  • Added bash-backed resolver coverage in tests/test_setup_tasks.py for: python fallback when python3 is unavailable, CRLF trimming for preset IDs, and deterministic alphabetical directory fallback when registry parsing fails.
  • Added focused mutation tests in tests/test_parallel_workers.py that exercise pytest_load_initial_conftests around -- sentinel boundaries and injection placement.
  • Validation: uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py -q (142 passed).

2026-06-07 Add-on (commit d7ba3f6)

  • Updated tests/test_setup_tasks.py to use os.pathsep when prepending shim paths in the invoke-separator test and adjusted fallback-proof tests to avoid accidental python3 discovery from host paths.
  • Updated tests/_path_utils.py so /tmp/... mapping on Windows falls back to raw path when the mapped tempdir candidate does not exist.
  • Updated tests/conftest.py early parallel settings parsing to raise pytest.UsageError for invalid or < 1 --parallel-max-workers values, aligning with pytest_configure behavior.
  • Added focused validation tests in tests/test_parallel_workers.py for early hook UsageError behavior.
  • Validation: uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py -q (144 passed).

2026-06-07 Add-on (commit f78887f)

  • Updated tests/_path_utils.py to preserve over-prefixed slash UNC-like inputs (////server/...) and made /tmp conversion deterministic with optional override via SPECKIT_BASH_TMPDIR.
  • Updated tests/conftest.py to cache early-computed parallel settings and reuse them in pytest_configure, avoiding duplicate compute passes in the same invocation path.
  • Added focused tests in tests/test_parallel_workers.py for compute-once behavior and early-hook max-worker validation consistency.
  • Added path-helper regression coverage in tests/test_path_utils.py for over-prefixed slash UNC-like normalization and deterministic tmp mapping behavior.
  • Validation: uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py -q (147 passed).

2026-06-07 Add-on (commit 0bb271f)

  • Added an xdist worker-process guard in tests/conftest.py so the early injection hook no-ops inside worker processes.
  • Deduplicated Windows MEMORYSTATUSEX handling in tests/_parallel.py through a shared helper.
  • Moved host-to-Git-Bash path conversion into tests/_path_utils.py and updated setup-tasks tests to reuse the shared helper.
  • Fixed the markdown table syntax in CONTRIBUTING.md so it renders correctly on GitHub.
  • Added focused regression tests for the new worker-process guard and shared bash-path helper behavior.
  • Validation: uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py -q (150 passed).

…and on-behalf review standards (#2)

* feat: add system-aware parallel pytest tiers

* fix: honor cgroup v1 cpu quota for parallel worker sizing

* docs: add on-behalf AI review disclosure standard

* fix: support cpuacct,cpu cgroup v1 quota mount layout

* test: make --parallel activate xdist workers

* test: harden bash path assertions on Windows

* test: handle zero available memory in parallel sizing

* test: make symlink skip capability-based on Windows

* test: replace platform skips with capability checks

* test: address open review findings for path and xdist checks

* test: restore strict setup task assertions

* test: honor explicit -n auto and clean InvalidMetadata fallback

* test: floor cgroup quota workers and remove dead helper

* test: mock os.access for relative installer path case

* test: tighten path checks and report effective workers

* test: skip parallel xdist hooks when plugin is disabled

* test: simplify bash path normalization helper

* test: harden parallel args and cwd-safe upgrade tests
Copilot AI review requested due to automatic review settings June 6, 2026 06:07
@LahkLeKey LahkLeKey requested a review from mnriem as a code owner June 6, 2026 06:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR improves cross-platform test robustness (especially on Windows/Git Bash path translation) and adds an opt-in pytest --parallel mode that chooses a conservative worker count based on CPU/memory/OS caps (via pytest-xdist).

Changes:

  • Added system-aware parallel worker sizing utilities and --parallel/tier/max-workers pytest options, plus new unit tests.
  • Relaxed or normalized several path-based assertions to handle Windows/Git Bash path formats and filesystem limits.
  • Updated contributor docs and PR template to mention the new parallel test option and AI attribution guidance.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_timestamp_branches.py Adds Windows-tolerant path assertion helper + relaxes long-path failure expectation on Windows.
tests/test_setup_tasks.py Adjusts TASKS_TEMPLATE assertions to be Windows/Git Bash tolerant.
tests/test_setup_plan_no_overwrite.py Normalizes bash-emitted paths to Windows paths for assertions.
tests/test_self_upgrade_execution.py Makes self-upgrade tests less POSIX-only via mocking/path changes.
tests/test_self_upgrade_detection.py Makes InvalidMetadataError test runnable on more Python versions by patching conditionally.
tests/test_parallel_workers.py New tests covering worker sizing, cgroup parsing, and CLI arg parsing helpers.
tests/test_authentication.py Makes permission-warning test runnable on Windows via patching.
tests/integrations/test_integration_subcommand.py Adds a capability check for Windows directory symlink creation and skips accordingly.
tests/integrations/test_cli.py Makes chmod-mode assertion Windows-tolerant by spying on chmod calls.
tests/conftest.py Adds --parallel pytest option + early xdist arg injection, configuration, and report header.
tests/_parallel.py New module implementing CPU/memory detection and recommended worker computation.
pyproject.toml Adds pytest-xdist to test extras.
CONTRIBUTING.md Documents pytest --parallel usage and tuning options.
.github/PULL_REQUEST_TEMPLATE.md Updates checklist to mention parallel tests + AI comment attribution checkbox.
Comments suppressed due to low confidence (1)

tests/test_self_upgrade_execution.py:1

  • These tests no longer appear gated to POSIX, but they still rely on POSIX executable-bit semantics (chmod(0o644) and (likely) os.access(..., X_OK)). On Windows, os.access(..., X_OK) can behave like an existence check, so this test can fail (or become meaningless) when run under --parallel/xdist on Windows. Suggested fix: reintroduce a POSIX-only marker for this specific test, or explicitly patch specify_cli._version.os.access to return False for the installer path in this test so the behavior is deterministic cross-platform.
"""Installer execution, verification, and error-path tests for `specify self upgrade`."""

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_setup_tasks.py Outdated
Comment thread tests/test_setup_tasks.py Outdated
Comment thread tests/conftest.py
Comment thread tests/test_timestamp_branches.py Outdated
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex). Review round update for 127ac67: addressed all currently open review comments and posted inline 'Resolved by 127ac67' replies on each thread. Also refreshed the PR description Review Fix Ledger to track additive fixes and reduce feedback-loop churn. Validation: uv run pytest tests/test_setup_tasks.py tests/test_timestamp_branches.py tests/test_setup_plan_no_overwrite.py tests/test_self_upgrade_execution.py tests/test_parallel_workers.py -q (160 passed).

@LahkLeKey LahkLeKey requested a review from Copilot June 6, 2026 06:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

Comment thread tests/conftest.py Outdated
Comment thread tests/conftest.py Outdated
Comment thread tests/test_setup_tasks.py Outdated
Comment thread tests/test_setup_tasks.py Outdated
Comment thread tests/test_timestamp_branches.py Outdated
Comment thread tests/integrations/test_cli.py Outdated
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex). Review round update for a227754: addressed all currently open review comments and posted inline 'Resolved by a227754' replies on each thread. Refreshed the PR description Review Fix Ledger so additive follow-up fixes remain tracked and feedback-loop churn is reduced. Validation: uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_timestamp_branches.py tests/test_setup_plan_no_overwrite.py tests/test_self_upgrade_execution.py tests/integrations/test_cli.py -q (239 passed).

@LahkLeKey LahkLeKey requested a review from Copilot June 6, 2026 06:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Comment thread tests/test_self_upgrade_execution.py
Comment thread tests/conftest.py
Comment thread tests/_path_utils.py Outdated
Comment thread scripts/bash/common.sh Outdated
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

LahkLeKey commented Jun 6, 2026

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.4 mini). Addressed the latest review round in commit 42571cc (Windows self-upgrade test stability, xdist dist-default handling, tightened Windows shell-path matching, and interpreter-generic comments in common.sh). Focused validation: uv run pytest tests/test_self_upgrade_execution.py tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_timestamp_branches.py tests/test_setup_plan_no_overwrite.py tests/integrations/test_cli.py -q (239 passed).

@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.4 mini). Backfilled the PR review ledger with commit-level drilled-down details (trigger, code-surface, validation), added a scope-creep guard section to keep review changes bounded to this feature and coverage outcomes, and re-verified open threads. Current open-thread count is reviewer-owned conversation state; all open threads contain explicit inline Resolved by 42571cc. notes.

@LahkLeKey LahkLeKey requested a review from Copilot June 6, 2026 06:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Comment thread tests/_parallel.py Outdated
Comment thread tests/test_setup_tasks.py Outdated
Comment thread tests/conftest.py Outdated
Comment thread tests/test_timestamp_branches.py Outdated
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex). Addressed the latest four review comments in commit b12061f: sysconf page-size fallback portability, Windows PATH separator handling in shim-based setup-tasks test, simplification of parallel pytest flow by removing subprocess reinvocation, and explicit xfail for Windows long-path truncation edge case. Validation: uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/test_timestamp_branches.py -q (133 passed, 1 xfailed). PR description was backfilled with drilled-down add-on ledger details and updated open-thread status notes to reduce feedback-loop churn.

@LahkLeKey LahkLeKey requested a review from Copilot June 6, 2026 06:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

Comment thread tests/conftest.py
Comment thread tests/test_setup_tasks.py Outdated
Comment thread tests/integrations/test_cli.py Outdated
Comment thread tests/test_self_upgrade_detection.py Outdated
Comment thread tests/_path_utils.py
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex). Addressed the latest five review comments in commit b87e7c6: explicit xdist enablement when plugin autoload is disabled, Bash PATH separator handling for Bash subprocess shims, less brittle Windows chmod assertion semantics, realistic InvalidMetadataError coverage behavior, and UNC-preserving slash normalization with direct helper tests. Validation: uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py tests/integrations/test_cli.py tests/test_self_upgrade_detection.py tests/test_path_utils.py -q (176 passed, 1 skipped). PR description was backfilled with this add-on in the review ledger and open-thread status notes were updated.

@LahkLeKey LahkLeKey requested a review from Copilot June 6, 2026 06:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Comment thread tests/_path_utils.py
Comment thread tests/conftest.py
Comment thread scripts/bash/common.sh Outdated
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex). Addressed the latest three review comments in commit 8bb0ca4: UNC over-prefix normalization in shared path helpers, precise explicit-xdist plugin matching to avoid substring false positives, and interpreter-fallback wording alignment in bash resolver comments. Validation: uv run pytest tests/test_parallel_workers.py tests/test_path_utils.py tests/test_setup_tasks.py -q (55 passed). PR description review ledger has been backfilled with this add-on and open-thread resolved-by status updated.

@LahkLeKey LahkLeKey requested a review from Copilot June 6, 2026 07:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Comment thread tests/test_setup_tasks.py
Comment thread tests/test_self_upgrade_execution.py
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Review round update for commits 7727673 and 1858a93.

Addressed latest actionable comments:

  • tests/_path_utils.py: prevented UNC-vs-non-UNC false positives by preserving UNC-ness in normalized-part comparisons.
  • scripts/bash/common.sh: hardened preset-id handling to skip unsafe IDs from .registry; removed ineffective trailing-slash sed stage from fallback scan pipeline.
  • tests/integrations/test_cli.py: restored Path.chmod spy to preserve real behavior by using wraps=Path.chmod on Windows too.
  • scripts/bash/common.sh: made per-preset parse-state resets explicit so strategy/manifest_file cannot leak between loop iterations.
  • tests/test_setup_tasks.py and tests/test_path_utils.py: added regressions for unsafe preset IDs, UNC mismatch, and cross-preset state-leak prevention.

Validation runs:

  • uv run pytest tests/test_setup_tasks.py tests/test_path_utils.py tests/integrations/test_cli.py tests/test_authentication.py tests/test_parallel_workers.py -q
  • Result: 247 passed then 248 passed after the final loop-state leak fix.

Inline replies have been posted on all current open review threads as Resolved by 7727673. and Resolved by 1858a93.

@LahkLeKey LahkLeKey requested a review from Copilot June 7, 2026 23:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.

Comment thread scripts/bash/common.sh
Comment thread scripts/bash/common.sh
Comment thread tests/_path_utils.py
Comment thread .github/PULL_REQUEST_TEMPLATE.md Outdated
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Review round update for commit 45d0fbd.

Addressed latest actionable comments:

  • scripts/bash/common.sh: made registry priority sort robust to mixed priority value types via safe integer coercion + deterministic tie-break.
  • scripts/bash/common.sh: narrowed preset-id safety checks to traversal-relevant cases so benign IDs containing .. (for example v1..0) are accepted.
  • tests/_path_utils.py: scoped UNC marker behavior to Windows so non-Windows //... paths are not forced into UNC semantics.
  • .github/PULL_REQUEST_TEMPLATE.md: restored checklist nesting indentation for the optional parallel test item under the local test step.
  • Added focused regressions in tests/test_setup_tasks.py and adjusted path-utils coverage in tests/test_path_utils.py.

Validation run:

  • uv run pytest tests/test_setup_tasks.py tests/test_path_utils.py tests/test_authentication.py tests/integrations/test_cli.py tests/test_parallel_workers.py -q
  • Result: 250 passed.

Inline replies have been posted on all current open review threads as Resolved by 45d0fbd.

@LahkLeKey LahkLeKey requested a review from Copilot June 7, 2026 23:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

tests/test_self_upgrade_execution.py:1

  • This test is intended to cover the non-executable-installer error path, but it no longer explicitly forces the installer to be non-executable (the prior chmod was removed) and it also doesn’t patch os.access(..., X_OK) like other tests do. That makes the test outcome dependent on platform/filesystem defaults (e.g., umask and Windows os.access semantics). To make it deterministic, either restore an explicit non-executable setup on platforms where it’s meaningful (e.g., chmod to 0o644 on POSIX) or patch specify_cli._version.os.access to return False within this test.
"""Installer execution, verification, and error-path tests for `specify self upgrade`."""

Comment thread tests/test_setup_tasks.py Outdated
Comment thread tests/test_setup_tasks.py Outdated
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Review round update for commit 2b5fa8e.

Addressed latest actionable comments:

  • Consolidated duplicated bash test helpers in tests/test_setup_tasks.py into _run_bash_template_resolver(...) with thin wrappers for resolve_template and resolve_template_content.
  • This keeps PATH override/env/quoting behavior centralized and avoids helper drift.

Validation run:

  • uv run pytest tests/test_setup_tasks.py tests/test_self_upgrade_execution.py -q
  • Result: 61 passed.

Inline replies have been posted on all current open review threads as Resolved by 2b5fa8e.

@LahkLeKey LahkLeKey requested a review from Copilot June 7, 2026 23:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.

Comment thread tests/conftest.py Outdated
Comment thread scripts/bash/common.sh
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Review round update for commit 015b976.

Addressed latest actionable comments:

  • tests/conftest.py: changed no-sentinel --parallel injection to insert xdist args adjacent to --parallel (deterministic ordering), rather than appending to the end.
  • scripts/bash/common.sh: removed redundant resolve_template_python_cmd probing inside _iter_preset_ids_ordered and reused the cached command selected by parent callers.
  • tests/test_parallel_workers.py: added regression coverage for adjacent no-sentinel injection placement.

Validation run:

  • uv run pytest tests/test_parallel_workers.py tests/test_setup_tasks.py -q
  • Result: 81 passed.

Inline replies have been posted on all current open review threads as Resolved by 015b976.

@LahkLeKey LahkLeKey requested a review from Copilot June 8, 2026 00:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.

Comment thread scripts/bash/common.sh
Comment thread tests/test_path_utils.py Outdated
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Review round update for commit 093b04b.

Addressed latest actionable comments:

  • scripts/bash/common.sh: replaced manifest file: gating with _is_safe_manifest_relative_path, which rejects absolute/drive-prefixed paths and any .. segment after normalizing slash/backslash separators.
  • tests/test_setup_tasks.py: added focused bash-level regression coverage for unsafe and safe manifest relative path inputs.
  • tests/test_path_utils.py: consolidated duplicated imports into one grouped import statement.

Validation run:

  • uv run pytest tests/test_setup_tasks.py tests/test_path_utils.py -q
  • Result: 63 passed.

Inline replies have been posted on all current open review threads as Resolved by 093b04b.

@LahkLeKey LahkLeKey requested a review from Copilot June 8, 2026 00:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.

Comment thread scripts/bash/common.sh
Comment thread tests/_path_utils.py
Comment thread tests/conftest.py Outdated
Comment thread tests/test_setup_tasks.py
@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Review round update for commit 78d7988.

Addressed latest comments:

  • scripts/bash/common.sh: trim trailing \r from manifest parser strategy and manifest_file values before safety checks and -f lookup.
  • tests/_path_utils.py: stop UNC classification for Windows device/extended-length prefixes (\\?\\, \\.\\) during normalization/sentinel handling.
  • tests/conftest.py: raise explicit usage errors when --parallel-tier or --parallel-max-workers is provided without a value.
  • tests/test_setup_tasks.py, tests/test_parallel_workers.py, tests/test_path_utils.py: added focused regressions for each behavior above.
  • tests/test_setup_tasks.py performance suggestion was reviewed as optional; kept subprocess-per-scenario structure for current PR scope (isolation/readability) and documented inline.

Validation run:

  • uv run pytest tests/test_setup_tasks.py tests/test_parallel_workers.py tests/test_path_utils.py -q
  • Result: 111 passed.

Inline replies have been posted on all current open review threads as Resolved by 78d7988.

@LahkLeKey LahkLeKey requested a review from Copilot June 8, 2026 01:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.

Comment thread scripts/bash/common.sh
Comment on lines +494 to +501
_is_safe_manifest_relative_path() {
local manifest_file="$1"
[ -n "$manifest_file" ] || return 1

# Reject absolute and drive-prefixed paths.
case "$manifest_file" in
/*|\\*|[A-Za-z]:*) return 1 ;;
esac
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Resolved by da4953a.

Comment thread tests/_path_utils.py
Comment on lines +11 to +21
def normalize_path_text(path_value: str) -> str:
"""Normalize slashes and repeated separators for string checks."""
normalized = path_value.replace("\\", "/")
if path_value.startswith("\\\\?\\") or path_value.startswith("\\\\.\\"):
return re.sub(r"/{2,}", "/", normalized)
is_unc_backslash = path_value.startswith("\\\\")
is_unc_slash_double = normalized.startswith("//") and not normalized.startswith("///")
is_unc_slash_overprefixed = normalized.startswith("////")
if is_unc_backslash or is_unc_slash_double or is_unc_slash_overprefixed:
unc_tail = normalized.lstrip("/")
return "//" + re.sub(r"/{2,}", "/", unc_tail)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Resolved by da4953a.

@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).

Review round update for commit da4953a.

Addressed latest actionable comments:

  • scripts/bash/common.sh: narrowed manifest drive-path rejection to drive-rooted forms (X:/..., X:\\...) to avoid rejecting benign relative names like a:overlay.md.
  • tests/_path_utils.py: made device-path normalization representation-invariant by handling slash-form prefixes (//?/, //./) before UNC classification.
  • tests/test_setup_tasks.py and tests/test_path_utils.py: added regressions for both behaviors.

Validation run:

  • uv run pytest tests/test_setup_tasks.py tests/test_path_utils.py -q
  • Result: 68 passed.

Inline replies have been posted on all current open review threads as Resolved by da4953a.

@LahkLeKey
Copy link
Copy Markdown
Contributor Author

Posted on behalf of @LahkLeKey by GitHub Copilot (model: GPT-5.3-Codex).\n\nClosing this upstream PR to continue iteration in LahkLeKey/spec-kit after merging the PR branch work into the fork's main branch.

@LahkLeKey LahkLeKey closed this Jun 8, 2026
@LahkLeKey LahkLeKey deleted the feat/upstream-parallel-pytest-ready branch June 8, 2026 01:40
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.

2 participants