Conversation
Add installed-project deployment with an explicit validated first-party application wheel while preserving source-compatible deployments. Use role-aware staging and dirty-input provenance protection, isolate source and package launches, extend artifact-aware runtime rollback and fast-path state, and support deterministic package-mode releases.
Compare role-approved deployment inputs from both HEAD and the working tree so deleted and unstaged-renamed runtime inputs cannot escape dirty-source protection. Bound wrapper return inference to owned returns and unique callable names, preserving conservative unknown classifications for ambiguous code.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2aa4b0b38b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| if tracked is None: | ||
| return selected | ||
| selected.intersection_update(tracked) |
There was a problem hiding this comment.
When an external staging directory is used for a Git repository that initially lacks uv.lock, --prepare-lock creates the lockfile after the first staging pass, but the second pass intersects selected inputs with git ls-files, removing the newly created and necessarily untracked lockfile. _staging_files then raises Required deployment input is missing: uv.lock, so the documented prepare-and-generate workflow fails after modifying the source repository. The explicitly authorized lockfile needs to bypass this tracked-only filter (or be staged directly after creation).
Useful? React with 👍 / 👎.
| if application_wheel is not None: | ||
| paths.append(f"deployment/application/{application_wheel.name}") |
There was a problem hiding this comment.
When regenerating an existing package-mode kit with an application wheel whose filename changed, only the new filename is planned, while the old wheel recorded in the previous generated-file index is neither removed nor reported as a collision. The new index consequently drops the old path but leaves the file on disk, causing the freshly generated kit to fail NO_UNINDEXED_STAGED_FILES and preventing packaging. Reconciliation should delete unchanged previously owned paths that are no longer planned, or stop before writing.
Useful? React with 👍 / 👎.
| if str(filename_version) != expected_version: | ||
| raise PreparationError( | ||
| f"Application wheel version mismatch: expected {expected_version}, received " | ||
| f"{filename_version}." | ||
| ) |
There was a problem hiding this comment.
For projects using a valid but non-canonical PEP 440 version such as 1.0-rc1 or 1.0-1, parse_wheel_filename returns the normalized version (1.0rc1 or 1.0.post1), while expected_version retains the spelling from project metadata. This raw string comparison rejects a correctly built wheel for the same version before generation can proceed; parse the project version and compare Version objects (and apply the same normalization to the METADATA check below).
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b57ae648a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if plan.deployment_mode != "package" and application_wheel is not None: | ||
| raise PreparationError("--application-wheel is accepted only for package deployment mode.") | ||
| application_artifact = ( | ||
| validate_application_wheel(application_wheel.resolve(), assessment, plan) |
There was a problem hiding this comment.
When --application-wheel contains an unexpanded home-directory path (for example, a quoted ~/dist/app.whl or a programmatic Path('~/dist/app.whl')), resolving it here first turns it into <cwd>/~/dist/app.whl. The subsequent expanduser() inside validate_application_wheel() can no longer recognize the home marker, so generation incorrectly reports that an existing wheel is missing. Pass the original path to the validator, which already expands and resolves it in the correct order.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07cc9b0ae0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from python_deployment_builder.generation.acquisition import PreparationError, sha256_file | ||
| from python_deployment_builder.models import ApprovedArtifact, DeploymentPlan | ||
| from python_deployment_builder.models import ( | ||
| ApplicationArtifact, |
There was a problem hiding this comment.
When an application wheel contains a sensitive file already prohibited elsewhere, such as token.json, .pypirc, pip.ini, or .env.production, this reduced name set lets it pass validation and the static kit scan cannot detect it inside the unextracted .whl. The resulting release therefore distributes credentials despite the deployment secret-file policy; apply the same filename and .env.* rules used by static validation to wheel members.
Useful? React with 👍 / 👎.
| def _selected_deployment_paths(assessment, plan) -> set[str]: | ||
| selected = {"pyproject.toml", "uv.lock"} | ||
| if plan.deployment_mode == "source": |
There was a problem hiding this comment.
For a Git repository whose tracked .gitignore is locally changed, neither the current nor HEAD selected-path set includes that ignore file, so the intersection in _dirty_tracked_deployment_paths misses the change even though ignore rules directly control which source/resources are staged and are included in the assessment fingerprint. A local ignore-rule edit can therefore add or omit runtime files while the manifest still claims the unchanged HEAD revision; include all applicable .gitignore files in the dirty deployment inputs.
Useful? React with 👍 / 👎.
| "name=os.environ['PDBUILDER_APPLICATION_DISTRIBUTION'];" | ||
| "version=os.environ['PDBUILDER_APPLICATION_VERSION'];" | ||
| "module=os.environ['PDBUILDER_APPLICATION_MODULE'];" | ||
| "sys.exit(m.version(name)!=version or importlib.util.find_spec(module) is None)" |
There was a problem hiding this comment.
For the newly supported non-canonical PEP 440 spellings (for example, project version 1.0-rc1 with wheel metadata/version 1.0rc1), application-wheel validation now deliberately accepts the wheel but stores the original project spelling in the manifest. This probe then compares importlib.metadata.version() to that spelling as raw strings, causing runtime validation—and all --runtime-validation—to fail after a successful installation; compare parsed Version values or store the canonical version.
Useful? React with 👍 / 👎.
Unify staged-kit and application-wheel security policy, guard Git ignore inputs as analysis provenance, and compare installed application versions with PEP 440 semantics.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 038ab6c4d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| item.path.rstrip("/") | ||
| for item in assessment.file_inventory | ||
| if item.role in RUNTIME_ROLES and not item.path.endswith("/") |
There was a problem hiding this comment.
When a flat-layout source-mode project declares runtime files in [tool.setuptools.package-data] but accesses them dynamically (for example, app/data/default.json selected through importlib.resources using a variable), those non-Python files remain UNKNOWN because package-data declarations do not promote inventory roles. This filter consequently omits them from the kit, while generation and static validation still succeed; the installed application then fails when it tries to read its declared data. Include files matching authoritative package-data mappings among source-mode runtime inputs.
Useful? React with 👍 / 👎.
| if ".dist-info" in member_path.parts or member_path.suffix.lower() not in TEXT_SUFFIXES: | ||
| continue | ||
| text = bundle.read(member).decode("utf-8", errors="replace") |
There was a problem hiding this comment.
Although prohibited basenames are now caught, this suffix allowlist still skips common textual package-data formats such as .yaml, .yml, .toml, .ini, and .cfg. For example, a wheel member named settings.yaml containing api_key: sk-abcdefghijklmnop bypasses text_security_findings, and the static kit scan cannot inspect it inside the .whl, so the release distributes a credential despite the secret-content policy. Scan all recognized textual resource formats, not only TEXT_SUFFIXES as currently defined.
Useful? React with 👍 / 👎.
| if application_wheel is not None: | ||
| paths.append(f"deployment/application/{application_wheel.name}") |
There was a problem hiding this comment.
When --application-wheel is a symlink whose basename differs from its target (for example, latest.whl pointing to a valid versioned wheel), validation resolves the link and _render_owned_files stages the target's ApplicationArtifact.filename, but collision planning uses the unresolved application_wheel.name here. An unowned file already present at the actual versioned destination is therefore absent from preview.collisions and is overwritten during _write_files; derive planned paths from the validated artifact filename instead.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f26c0bec41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| repository_root: Path, provenance_guarded: set[str] | ||
| ) -> list[str]: | ||
| result = subprocess.run( | ||
| ["git", "-C", str(repository_root), "diff", "--name-only", "-z", "HEAD", "--"], |
There was a problem hiding this comment.
When a tracked runtime input is staged with git mv into an excluded scope, Git's rename detection makes this command report only the destination path. For example, moving helper.py to docs/helper.py leaves the destination outside the current guarded set while only the old path is in the HEAD guarded set, so the intersection is empty; generation can then omit the helper while still recording the unchanged HEAD revision. Disable rename detection here or parse both sides of rename records.
Useful? React with 👍 / 👎.
| for pattern in patterns: | ||
| if not any(fnmatch.fnmatchcase(name, pattern) for name in package_members): |
There was a problem hiding this comment.
When one package-data glob matches multiple source files, this any(...) check accepts a wheel containing only one of them. For example, data/*.json covering both defaults.json and schema.json passes if the wheel contains only defaults.json, allowing generation and static validation to succeed even though the installed application can fail when it reads the omitted resource. Compare wheel members with every concretely resolved packaged resource rather than requiring only one match per pattern.
Useful? React with 👍 / 👎.
| metadata_distribution, metadata_version = _require_core_metadata( | ||
| metadata, label="METADATA", wheel=path | ||
| ) |
There was a problem hiding this comment.
When the supplied wheel declares a stricter Requires-Python than the assessed project, such as project metadata >=3.12 with a selected 3.12 runtime but wheel metadata >=3.13, the current metadata checks accept it because they inspect only name and version. The generated kit then fails during the end-user uv pip install step. Parse the wheel's Requires-Python and ensure the selected runtime satisfies it before staging the artifact.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82d111199c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| f"{assessment.repository.revision}: {', '.join(dirty)}. Commit or restore " | ||
| "those inputs before release-oriented generation." | ||
| ) | ||
| selected.intersection_update(tracked) |
There was a problem hiding this comment.
When a tracked source file imports a newly created untracked local module, assessment promotes that module to APPLICATION_SOURCE, but this intersection silently removes it from the staged kit without reporting a provenance error. Generation and static validation can then succeed even though the application fails at setup or launch when the tracked module imports the omitted helper; selected untracked runtime inputs should block generation rather than be discarded.
Useful? React with 👍 / 👎.
| if ".dist-info" in member_path.parts or member_path.suffix.lower() not in TEXT_SUFFIXES: | ||
| continue |
There was a problem hiding this comment.
When a project README containing a credential is embedded as the wheel's Core Metadata description, the resulting .dist-info/METADATA member has no suffix and is skipped by this allowlist, while static kit validation cannot inspect the opaque wheel. The fresh evidence after the earlier wheel-scanning comments is that the expanded suffix list still excludes this standard extensionless textual member, allowing the generated release to distribute an obvious or configured secret.
Useful? React with 👍 / 👎.
| explicit = project.package_directories.get(package) | ||
| if explicit is not None: | ||
| candidates.append(root / explicit) |
There was a problem hiding this comment.
For a valid setuptools layout such as package-dir = {app = "lib"} with package data declared for app.sub, the exact lookup for app.sub misses the parent app mapping, so the resolver searches app/sub instead of lib/sub. Consequently the concrete declared resource never reaches expected_members, and a stale or incomplete application wheel can pass validation without it. Fresh evidence beyond the earlier package-data finding is this new resolver's lack of longest-parent mapping semantics.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 368c17cbaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "be inventoried. Generation stopped rather than claiming clean provenance." | ||
| ) | ||
| try: | ||
| head_root = safe_extract_zip(archive, extracted) |
There was a problem hiding this comment.
When a Git repository's HEAD contains an unrelated tracked symlink, any tracked working-tree change reaches this extraction; git archive preserves the symlink type, while safe_extract_zip rejects every symbolic-link member. Consequently even an unrelated README edit causes generation to stop before the changed paths are intersected with deployment inputs, despite the documented policy that unrelated tracked documentation should not trigger the provenance gate.
Useful? React with 👍 / 👎.
| key = normalized.casefold() | ||
| if key in recorded: | ||
| raise PreparationError(f"Wheel RECORD contains duplicate paths: {wheel.name}") | ||
| recorded[key] = (row[1], row[2]) |
There was a problem hiding this comment.
When a wheel member is modified and the ZIP is rebuilt with a valid CRC while its RECORD retains the original digest and size, this validator merely stores those fields and checks the path inventory. The altered application or dependency wheel is therefore accepted and assigned a new authoritative whole-file SHA-256 even though its internal integrity metadata no longer matches; validate every non-RECORD member's recorded hash and size rather than accepting them unchecked or blank.
Useful? React with 👍 / 👎.
| write_assessment_reports(assessment, reports_root) | ||
| write_deployment_plan_reports(plan, reports_root) |
There was a problem hiding this comment.
Because the assessment and plan report writes were moved below generation, the normal first pdbuilder all run for a package-mode project without --application-wheel returns at the new blocker before creating any report files. The command prints the ASSESS and PLAN phases but leaves the developer without the plan needed to inspect the package-mode decision and prepare the wheel; write these reports immediately after planning, before readiness-related early returns.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdf7256448
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| parser = configparser.ConfigParser(interpolation=None) | ||
| parser.optionxform = str | ||
| parser.read_string(bundle.read(members[entry_points_name]).decode("utf-8-sig")) | ||
| entry_group = "gui_scripts" if entry_point.kind == "gui" else "console_scripts" |
There was a problem hiding this comment.
For a package-mode project declaring [project.scripts] gui-tool = "app:main" (or any console-script name/target containing gui), analysis.metadata._entry_point heuristically sets kind to gui, so this lookup requires the wheel to place the entry point under gui_scripts. A standards-compliant wheel instead records it under console_scripts, causing every valid wheel for that project to be rejected as disagreeing with the authoritative metadata. Record the source metadata group separately or derive this lookup from the declared group rather than the UI-launch heuristic.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86c7065aae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| configured_package_data = setuptools.get("package-data") | ||
| if isinstance(configured_package_data, dict): | ||
| package_data = { |
There was a problem hiding this comment.
For a supported setuptools project whose metadata and entry point come from setup.cfg, an [options.package_data] declaration is never copied into PackagingAssessment.package_data because only [tool.setuptools.package-data] is read here. Consequently, dynamically accessed data can remain UNKNOWN and be omitted from a flat-layout source-mode kit even though setuptools declares it as runtime package data. Fresh evidence beyond the earlier package-data issue is that the new resolver is fed exclusively from this pyproject-specific branch despite inspect_metadata() continuing to support setup.cfg projects.
Useful? React with 👍 / 👎.
| if tracked is None: | ||
| return selected |
There was a problem hiding this comment.
When source mode is generated from an ordinary non-Git directory, this early return bypasses the new authoritative package-data stageability check. If a concretely declared package-data file is classified as IGNORED_OR_LOCAL or MUTABLE_STATE_CANDIDATE, _selected_deployment_paths() omits it, yet generation and static validation can succeed and the deployed application can fail when reading the missing resource. Fresh evidence beyond the earlier package-data finding is that the new exclusion check only runs when Git tracking information exists; non-Git inputs should either stage the authoritative file or fail explicitly.
Useful? React with 👍 / 👎.
| if metadata_version_value != expected_version_value: | ||
| raise PreparationError("Application wheel METADATA version is wrong.") | ||
| _validate_requires_python(metadata, plan, path) |
There was a problem hiding this comment.
When a same-name/version application wheel declares an applicable Requires-Dist that is absent from or incompatible with the selected lock graph, these checks accept and stage it because only name, version, Requires-Python, and tags are examined. End-user setup then installs that wheel with --no-deps, after which the generated uv pip check fails, so a kit that passed generation and static validation cannot complete first-run setup. Validate the wheel's applicable dependency requirements against the planned locked environment before accepting it.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc682d75b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| edge.selected_extra | ||
| and canonicalize_name(edge.selected_extra) not in activated_parent_extras | ||
| ): | ||
| continue |
There was a problem hiding this comment.
When an approved package is reached beneath a selected application extra (for example, map -> pywebview -> proxy-tools), inspect_uv_lock() propagates selected_extra="map" onto proxy-tools' normal dependency edges. Here that root-extra label is compared with extras requested of proxy-tools—usually an empty set—so a valid edge such as proxy-tools -> helper is skipped and an approved wheel declaring Requires-Dist: helper>=1 is incorrectly rejected even though the selected locked sync installs helper. Compare selected_extra with the graph's selected application extras, and use activated_dependency_extra when filtering the parent's own optional-dependency edges.
Useful? React with 👍 / 👎.
| for artifact in manifest.approved_artifacts: | ||
| if relative := manifest_artifact_wheel_path("wheels", artifact.filename): | ||
| expected_wheel_identities[relative] = ( | ||
| artifact.distribution_name, | ||
| artifact.version, |
There was a problem hiding this comment.
For a consistently re-indexed kit, two distinct locked artifact records can point to the same wheel filename and SHA; this assignment silently lets the later identity replace the earlier one. If the record matching the real wheel comes last, metadata validation sees only that identity, while the independent lock-identity loop can still accept both manifest records. The runtime sync then excludes both distributions but _promote_environment installs the same wheel twice, leaving one required distribution absent. Require approved artifact filenames/paths to be unique before building this map.
Useful? React with 👍 / 👎.
| if len(node.args) != 2: | ||
| return function, [] | ||
| allowed_keywords = {"encoding", "errors"} if function.endswith("text") else set() | ||
| if any(keyword.arg not in allowed_keywords for keyword in node.keywords): |
There was a problem hiding this comment.
When supported Python 3.12 source uses importlib.resources.read_text(package="app", resource="defaults.json") (similarly read_binary, open_text, or open_binary), the new legacy resolver returns no path because it requires both arguments to be positional and then rejects package/resource keywords. The concrete file therefore remains UNKNOWN, role-aware source staging omits it, and the deployed application fails when it performs the read. Resolve the two positional-or-keyword parameters with call_argument() before permitting the optional text arguments.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40da085224
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if any( | ||
| evidence.file in {"setup.cfg", "setup.py"} | ||
| for evidence in dependency.evidence | ||
| ) | ||
| and not any(evidence.file == "pyproject.toml" for evidence in dependency.evidence) |
There was a problem hiding this comment.
[project]
When pyproject.toml explicitly declares [project].dependencies (including an empty list) but a retained setup.cfg or setup.py contains stale install_requires, PEP 621 makes the standardized field authoritative and setuptools ignores the legacy value. Metadata analysis nevertheless retains that legacy dependency, and this filter classifies it as backend-only, adding RUNTIME_SYNC_METADATA_UNSUPPORTED even though the valid uv.lock correctly omits it. Only apply this blocker when the dependency field is actually supplied by legacy metadata rather than overridden by [project].
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd5ea6081a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| project_dependencies_static_authoritative = ( | ||
| project_dependencies_present and not project_dependencies_dynamic | ||
| ) |
There was a problem hiding this comment.
When a project has a [project] table, omits dependencies, and does not list it in dynamic, PEP 621 and setuptools 79 treat the field as statically empty and ignore legacy install_requires. This predicate instead requires the field to be present, so stale requirements from setup.cfg or setup.py are retained and later trigger RUNTIME_SYNC_METADATA_UNSUPPORTED, blocking an otherwise valid project. Fresh evidence beyond the earlier explicit-list report is that the new predicate still requires project_dependencies_present rather than treating absence from dynamic as authoritative whenever [project] exists.
Useful? React with 👍 / 👎.
| static_entry_point_groups = { | ||
| legacy_group | ||
| for group, legacy_group in ( | ||
| ("scripts", "console_scripts"), ("gui-scripts", "gui_scripts") | ||
| ) | ||
| if group in project and group not in project_dynamic | ||
| } |
There was a problem hiding this comment.
When [project] omits scripts or gui-scripts without declaring that field dynamic, setuptools ignores the corresponding legacy setup.cfg/setup.py entry points. This comprehension suppresses legacy values only when the standardized group is explicitly present, so a stale launcher is treated as authoritative; planning can select it and application-wheel validation then rejects the correctly built wheel because it contains no such entry point.
Useful? React with 👍 / 👎.
| # ``anchor=`` is the Python 3.12 spelling. Deliberately leave | ||
| # deprecated ``package=`` unresolved rather than treating either | ||
| # keyword form as the zero-argument implicit caller anchor. |
There was a problem hiding this comment.
For deployments targeting Python 3.11 or 3.12, importlib.resources.files(package="app") remains a supported call (deprecated in 3.12 after the parameter was renamed to anchor). This branch deliberately returns no path for it, so a sole call such as files(package="app").joinpath("defaults.json").read_text() leaves the existing resource unresolved and role-aware source staging omits it, producing a runtime failure. Fresh evidence beyond the implicit-anchor report is the explicit rejection of this still-supported keyword spelling.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a7556358e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| trusted_wheels = trusted_artifact_wheel_paths(manifest) | ||
| secret_scanability_failures: list[str] = [] | ||
| try: | ||
| secret_values = configured_secret_values(manifest.configuration_secret_names) |
There was a problem hiding this comment.
When validating a pre-M6.1 manifest, configuration_secret_names defaults to an empty list even though configuration_presence_names may contain credentials such as DB_PASSWORD. The previous validator scanned values for every presence name, but this call now scans none of them, so an arbitrary configured credential embedded in indexed source can receive STATIC_VALID as long as it does not match the narrow obvious-secret regex. Fall back conservatively to the legacy presence list when the new field was absent from the serialized manifest.
Useful? React with 👍 / 👎.
| return _resource_package_anchor_values( | ||
| anchor, | ||
| root=root, | ||
| source_path=source_path, | ||
| source_roots=source_roots, | ||
| project=project, | ||
| assignments=assignments, | ||
| returns=returns, | ||
| ) |
There was a problem hiding this comment.
For source-mode code targeting Python 3.12+ that calls importlib.resources.files("app.config").joinpath("defaults.json"), where app.config is a module and the resource is adjacent at app/defaults.json, the explicit anchor is valid and resolves to the module's containing directory. This path delegates only to the package-directory resolver, which searches for an app/config/ directory and returns no resource; defaults.json therefore remains UNKNOWN and is omitted from role-aware staging. Resolve literal module anchors to the containing directory as well as package anchors.
Useful? React with 👍 / 👎.
|
@codex review |
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d26ebe9b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for expression, mode in _path_uses(node, directory_reads): | ||
| resource_values = _importlib_resource_path_values( |
There was a problem hiding this comment.
When source mode materializes a resource with the standard form with importlib.resources.as_file(importlib.resources.files("app") / "model.dat") as path, this dispatch finds no path use: the outer as_file() call is not recognized by _path_uses, while the inner / expression is not visited as a read operation. An unconventionally named tracked resource therefore remains UNKNOWN, is omitted by role-aware staging, and is unavailable when as_file() runs in the deployed application.
Useful? React with 👍 / 👎.
| module = target.value | ||
| if module and all(part.isidentifier() for part in module.split(".")): | ||
| modules.append((module, node.lineno)) |
There was a problem hiding this comment.
For a flat source-mode project using importlib.import_module(".examples.plugin", package="app"), the empty component created by the leading dot makes this predicate reject the otherwise literal target, and the package argument is never used to make it absolute. If that local module is under an excluded examples/docs scope, it is not promoted into the staged application files, so generation and static validation can succeed before the runtime import raises ModuleNotFoundError.
Useful? React with 👍 / 👎.
commented
Sep 12, 2026
|
@codex review |
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94b3fd52e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| automatic_root = ( | ||
| source_roots[0] | ||
| if len(source_roots) == 1 | ||
| else "src" | ||
| if (root / "src").is_dir() | ||
| else "." |
There was a problem hiding this comment.
When a setuptools project relies on automatic explicit-layout discovery with only package-dir = {app = "lib"} and no packages or py-modules, setuptools 79 discovers the installed package app from lib, but this selects the ordinary ./src root and runs flat/src discovery instead. The assessment consequently records lib (or no package) rather than app, so package-mode validation rejects a correctly built wheel for missing the incorrectly inferred installed members. Handle nonempty package_directories as setuptools's explicit-layout discovery before choosing this fallback root.
Useful? React with 👍 / 👎.
| if len(node.args) > 2: | ||
| return function, [] |
There was a problem hiding this comment.
A remaining case after the keyword-binding fix is importlib.resources.read_text("app", "defaults.json", "utf-8") (and the equivalent open_text form): Python 3.11 and 3.12 accept encoding and errors as third and fourth positional arguments, but this unconditional two-argument limit marks the resource unresolved. If the resource has a nonconventional name, role-aware source staging omits it and the deployed application fails when reading it; permit the documented positional text parameters while retaining the two-argument limit for binary helpers.
Useful? React with 👍 / 👎.
commented
Sep 12, 2026
|
@codex review |
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ce99790f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if len(node.args) > (4 if text_function else 2): | ||
| return function, [] |
There was a problem hiding this comment.
When the selected runtime is Python 3.13 or 3.14, importlib.resources.read_binary and open_binary accept multiple path components after the anchor, but this guard rejects every binary call with more than two positional arguments. A valid call such as read_binary("app", "models", "weights.bin") therefore yields no resource evidence; because a .bin file under a nonconventional models directory remains UNKNOWN, source-mode staging can omit it while static validation succeeds, leaving the deployed application to fail when it reads the resource.
Useful? React with 👍 / 👎.
commented
Sep 12, 2026
|
@codex review |
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4888ee2871
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| member.installed_member_path | ||
| for member in resolve_packaged_python_sources(source_root, assessment.project) | ||
| } if source_root is not None else set() | ||
| missing_python_members = sorted(expected_python_members - installed_names) |
There was a problem hiding this comment.
When a supplied application wheel contains every expected member plus a stale top-level module such as requests/__init__.py or a root .pth file, this subset-only comparison accepts it. The runtime at manage.py:126-175 synchronizes locked dependencies and then installs the application wheel, so the extra file can overwrite dependency code or execute during Python startup; uv pip check --help describes checking compatible dependencies, not installed-file ownership, so it does not detect this collision. Reject unexpected Python and startup-executable destinations outside the authoritative modeled wheel surface.
Useful? React with 👍 / 👎.
| if _is_resource_files_call(node, module_bindings, files_bindings): | ||
| if not isinstance(node, ast.Call): |
There was a problem hiding this comment.
After the inline as_file(files(...) / ...) fix, a remaining common form is asset = files("app") / "model.dat" followed by as_file(asset) or asset.read_bytes(). In that case this resolver receives an ast.Name, but it only recognizes a direct files() call, joinpath, or / expression and never follows the supplied assignments; the generic path fallback cannot interpret the assigned files() call either. A nonconventionally named existing resource therefore remains UNKNOWN and is omitted from source-mode staging.
Useful? React with 👍 / 👎.
commented
Sep 12, 2026
|
@codex review |
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 847198a4ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for relative_text in sorted(selected): | ||
| relative = Path(relative_text) | ||
| if _is_runtime_cache(relative): | ||
| continue | ||
| path = repository_root / relative |
There was a problem hiding this comment.
When source-mode generation runs on a case-sensitive non-Windows host, a selected path containing a Windows-reserved component such as app/CON.py, an invalid character such as data/foo:bar.json, or a case-insensitive collision is copied and indexed without the Windows materialization checks applied to wheel members. Static validation and packaging can therefore accept a ZIP that cannot be extracted reliably on the target Windows system; validate all selected kit paths and Windows-equivalent collisions before staging them.
Useful? React with 👍 / 👎.
| item.path.rstrip("/") | ||
| for item in assessment.file_inventory | ||
| if item.role in RUNTIME_ROLES and not item.path.endswith("/") |
There was a problem hiding this comment.
For source code that explicitly reads an existing file with an unlisted suffix or no suffix, such as open("weights.dat", "rb"), resource analysis discards the path because _looks_like_resource_literal() recognizes only its fixed suffix/directory list. The file consequently remains UNKNOWN, this role filter omits it from the source-mode kit, and generation/static validation can succeed before the application raises FileNotFoundError; an explicit file-read operation should promote any safely resolved repository file regardless of naming convention.
Useful? React with 👍 / 👎.
| from pathlib import PurePosixPath | ||
|
|
||
| FORBIDDEN_SHELL = ("powershell.exe", "pwsh.exe", "executionpolicy") | ||
| WINDOWS_ABSOLUTE = re.compile(r"(?i)[a-z]:\\(?:users|home)\\[^\r\n\"]+") |
There was a problem hiding this comment.
When staged source or a validated wheel contains a hard-coded path such as C:/Users/alice/project/data.json, Python and Windows accept that spelling but this pattern matches only backslash separators. text_security_findings() therefore omits developer_path, allowing NO_DEVELOPER_PATHS to pass even though the release contains a machine-specific path that will fail for end users; accept either slash direction in the path detector.
Useful? React with 👍 / 👎.
commented
Sep 16, 2026
|
@codex review |
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83dcead905
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """Validate raw POSIX components before any host Path can normalize them.""" | ||
| if not value or "\\" in value or any(p in {"", ".", ".."} for p in value.split("/")): | ||
| raise WindowsPathError(f"Windows-unsafe relative path: {value!r}") | ||
| for component in value.split("/"): |
There was a problem hiding this comment.
Reject components that exceed the Windows filesystem limit here. A supplied application or approved wheel can contain a member with a 256-character ASCII component; _safe_wheel_members() delegates to this validator, so generation and static validation accept the wheel, but uv pip install cannot materialize that member on NTFS and first-run setup rolls back. Validate each component's Windows length before declaring wheel and kit destinations safe.
Useful? React with 👍 / 👎.
1 participant
Summary - Completes PDB's installed-project/package deployment mode. - Adds explicit first-party
--application-wheelhandling. - Keeps source mode for source-compatible projects. - Makes generation staging use role-aware M6 inventory and provenance rules. - Prevents ignored, untracked, cache, and deployment-support files from leaking into standalone kits. - Adds dirty tracked deployment-input protection. - Extends validation, runtime state, packaging, reports, and theallworkflow for package mode. ## Mode contract ### Source mode - The project is not installed. - The authoritative entry point must be source-import compatible. - Declared source roots are used. - Dependency synchronization retains--no-install-project. ### Package mode - Locked third-party dependencies are synchronized without building the project. - Reviewed exceptional dependency artifacts are installed separately. - A validated first-party application wheel is installed without dependency resolution or builds. - The authoritative entry point is imported from the installed environment. - No application source roots orPYTHONPATHare configured. Typed mode/readiness conditions are: -SOURCE_COMPATIBLE-PACKAGE_PREFERRED-ENTRYPOINT_REQUIRES_PACKAGE_MODE-DEPLOYMENT_MODE_CONFLICT-INSTALLED_PROJECT_REQUIRED-APPLICATION_WHEEL_REQUIRED## First-party artifact contract--application-wheelis the first-party project artifact and is distinct from--artifact, which remains the reviewed escape hatch for exceptional dependency wheels. PDB records the exact application-wheel SHA-256 and validates its distribution, version, authoritative entry point, module, explicitly named setuptools package-data mappings, RECORD inventory, wheel tags, archive safety, pure-Python policy, and deployment security constraints. The assessed source Git revision and supplied wheel SHA are separate provenance facts. Ordinary wheel metadata is not treated as cryptographic proof that a particular source revision produced the supplied bytes. ## Staging and provenance - Role-aware M6 inventory drives staging. - Git repositories stage tracked, role-approved deployment inputs. - Relevant tracked inputs identified in either theHEADsnapshot or current inventory block release-oriented generation when modified, deleted, or unstaged-renamed. - Deployment support, documentation, examples, mutable state, ignored/untracked files, and caches are excluded. - Non-Git directories and safely materialized archives remain supported under the same role policy. - Package-mode application runtime content comes only from the validated application wheel. ## Runtime and package behavior Package setup order is: 1. Provision managed Python. 2. Perform locked third-party synchronization. 3. Install approved dependency artifacts. 4. Install the first-party application wheel. 5. Runuv pip check. 6. Check the authoritative installed entry point. 7. Write successful state. Failed setup preserves or restores the prior known-good environment. Fast launch, Diagnose, Repair, rollback, and staleness checks share artifact-aware fingerprint semantics. Package-mode releases include the first-party wheel separately from dependency artifacts and retain deterministic ZIP behavior. ## Compatibility - Older source-mode manifests remain statically valid and packageable through compatible defaults. - Package mode begins with M6.1 and requires an application artifact. - Geo Map Explanation Extractor and TN Coordinate Converter retain source mode and their existing launch/sync behavior. ## Acceptance evidence ### SimpleGeorefGUI - Source remained unchanged atf484570d89fb1f9e9170fac915475358dfc1234e. - Package mode selected. - Source-derived authoritative surface: 13 Python plus 77 package-data members; all 90 persisted members validated, with no unexpected Python/startup destinations. - Static validation:STATIC_VALID. - Automated runtime checks: pass. - Final automated state:MANUAL_GUI_VALIDATION_REQUIRED. - Authoritative target imported from managedsite-packages. - ArcGIS, GDAL, osgeo, ArcPy, and ArcGIS Pro runtime paths were absent. ### Source-mode regressions - Geo Map Explanation Extractor: source mode, static/runtime automation pass, package success. - TN Coordinate Converter: source mode, reviewedproxy_toolsretained, static/runtime automation pass, package success. The same reviewed SGG kit packaged twice with identical ZIP SHA-256:31875A58658E11AFC968FA9224258B2180A30858B81659DDB6CE1E87BEE37BD6Acceptance wheels, runtime environments, deployment kits, and ZIPs are not included in this PR. ## Quality -2031 passed, 4 skipped- Ruff passed -git diff --checkpassed - The three P2 corrections, unchanged Geo/TN acceptance, and clarified deterministic-packaging evidence are documented in current correction evidence. Detailed SGG traceability retains both historical and current hashes: regeneration changed only intentional timestamp/ID metadata and its indexed digest; the deployment fingerprint and all other ZIP members are unchanged. Review accounting remains 130 total findings, 127 unresolved inline threads, 3 review-level-only findings, and 1 NOT_APPLICABLE inline finding.