You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
uvx ruff format --check src/specify_cli/artifacts/_operation_lookup.py src/specify_cli/artifacts/command_lookup.py tests/specify_cli/artifacts/test_operation_lookup.py tests/specify_cli/artifacts/test_command_lookup.py — passed
git diff --check — passed
AI disclosure
Implemented with GitHub Copilot using GPT-5.6 Sol in autonomous mode; architecture analysis, code generation, test generation, validation, and PR drafting.
This bypasses preflight_project_policy, so the refactor changes established CLI error text for structural project failures. For example, a symlinked .specify used to produce Refusing to use symlinked .specify path: ..., while calling project_files directly emits the lower-level Cannot read init-options.json: ... message; a non-directory .specify similarly loses its dedicated diagnostic. That contradicts the stated error-message compatibility goal. Reuse the existing preflight helper here and map its ProjectResolutionError into ArtifactLookupProjectPolicyError.
Sanitize unexpected adapter exceptions into JSON errors
src/specify_cli/artifacts/command_lookup.py:69
The adapter only normalizes OSError; any other unexpected operation exception (for example RuntimeError from a catalog dependency) escapes without the command's JSON error envelope. design/shared.md:273-274 requires adapters to sanitize unexpected exceptions, so catch Exception here after the expected ArtifactError branch and add a non-filesystem unexpected-failure case.
The remaining MCP inventory finding was already addressed in 0dabb649: artifacts/_mcp.py imports ARTIFACT_LOOKUP_OPERATION, derives the lookup operation ID, contract version, capabilities, and network access from the descriptor, keeps the tool unavailable, and states that only the first-class MCP adapter remains pending. The inventory regressions in test_mcp_info.py and test_mcp_list.py verify descriptor parity and non-registration.
Reverified on current HEAD 171a3e8a: 6 targeted MCP inventory/registration tests passed. The branch is clean and matches the pushed remote, so no additional code change or empty commit was created.
AI disclosure: Posted on behalf of @mnriem by GitHub Copilot using GPT-5.6 Sol in autonomous mode; Copilot inspected the review thread, verified the implementation and tests, and authored this review-round summary.
Addressed the current review findings in 907b5046. CLI project-context OSError and RuntimeError failures again emit the established not a Spec Kit project: no .specify/ directory found JSON error. Unexpected operation exceptions now record exception details through the adapter logger while the command protocol remains one sanitized JSON error with empty stdout and no traceback or secret detail.
Validation: 430 artifact tests passed; 101 focused lookup tests passed with 100% statement and branch coverage for the lookup operation and CLI adapter; Ruff, formatting, and git diff --check passed. CI and Copilot review have been retriggered for the new commit.
AI disclosure: Posted on behalf of @mnriem by GitHub Copilot using GPT-5.6 Sol in autonomous mode; Copilot analyzed the review findings, generated the code and regression tests, ran validation, committed and pushed the changes, and authored this review-round summary.
An invalid ID containing an unpaired surrogate (the new operation test uses \ud800) is copied unchanged into both the message and details. The CLI then calls json.dumps(..., ensure_ascii=False), producing text that cannot be UTF-8 encoded, so this expected validation failure can itself raise UnicodeEncodeError instead of returning the JSON error contract. Sanitize or backslash-escape the invalid value before constructing this error, and cover UTF-8 serialization of the resulting error.
Addressed the current review findings in 25f32490. Cwd acquisition failures again retain the established artifact resolution failed response, while SPECIFY_INIT_DIR canonicalization failures remain mapped to the legacy project error. Invalid lookup IDs now backslash-escape unpaired surrogates before constructing the operation error, keeping both its message and details UTF-8/JSON-safe without changing valid identifier text.
Validation: 432 artifact tests passed; 103 focused lookup tests passed with 100% statement and branch coverage for the lookup operation and CLI adapter; Ruff, formatting, and git diff --check passed. CI and Copilot review have been retriggered for the new commit.
AI disclosure: Posted on behalf of @mnriem by GitHub Copilot using GPT-5.6 Sol in autonomous mode; Copilot analyzed the review findings, generated the code and regression tests, ran validation, committed and pushed the changes, and authored this review-round summary.
The sole open finding in the latest review is already addressed in 0dabb649 and remains present at current HEAD 25f32490. I replied inline with the exact implementation and regression evidence. Reverification: 6 targeted MCP inventory/registration tests passed; the branch is clean and matches the pushed remote. No code change or empty commit was created.
AI disclosure: Posted on behalf of @mnriem by GitHub Copilot using GPT-5.6 Sol in autonomous mode; Copilot inspected the review thread, verified the implementation and tests, posted the inline response, and authored this review-round summary.
Addressed the current expected-error normalization finding in bd7fdd05. ArtifactLookupError now normalizes every message through the UTF-8-safe error-text boundary, covering filesystem- and integration-derived diagnostics as well as identifier validation. Added operation and CLI regressions using a surrogate-bearing integration failure to prove the message remains diagnostic, the JSON envelope is UTF-8 encodable, stdout stays empty, and no traceback escapes.
Validation: 434 artifact tests passed; 105 focused lookup tests passed with 100% statement and branch coverage for the lookup operation and CLI adapter; Ruff, formatting, and git diff --check passed. CI and Copilot review have been retriggered for the new commit.
AI disclosure: Posted on behalf of @mnriem by GitHub Copilot using GPT-5.6 Sol in autonomous mode; Copilot analyzed the review finding, generated the code and regression tests, ran validation, committed and pushed the changes, and authored this review-round summary.
Only the error message is made UTF-8-safe; details keeps raw strings. A POSIX project path may contain surrogate-escaped bytes, and registry/integration failures capture lookup_id before its Unicode validation, leaving a transport-neutral error whose structured details may fail JSON/UTF-8 serialization. Normalize string detail values at this boundary as well and cover a surrogate path or pre-validation identifier.
These project probes can raise OSError (for example, a permission-denied stat on supported Python 3.13), but they currently run outside the operation's filesystem-error conversion. That leaks a raw exception from lookup_artifact, so direct adapters do not receive the descriptor's stable filesystem_error contract and the CLI treats an expected filesystem failure as unexpected. Include both probes in the conversion boundary.
Addressed both previously missed transport-contract findings in f9252830. The shared error boundary now normalizes every top-level string detail value as UTF-8-safe text, including pre-validation lookup IDs and surrogate-bearing project paths. Both project directory probes now run inside the operation filesystem conversion boundary, so an OSError from either probe becomes the descriptor-declared filesystem_error before integration loading or catalog access.
Validation: 437 artifact tests passed; 108 focused lookup tests passed with 100% statement and branch coverage for the lookup operation and CLI adapter; Ruff, formatting, and git diff --check passed. CI and Copilot review have been retriggered for the new commit.
AI disclosure: Posted on behalf of @mnriem by GitHub Copilot using GPT-5.6 Sol in autonomous mode; Copilot analyzed all open and previously missed findings, generated the code and regression tests, ran validation, committed and pushed the changes, and authored this review-round summary.
Catch ArtifactLookupError to prevent sibling error detail leakage
src/specify_cli/artifacts/command_lookup.py:76
ArtifactError is the package-wide base (artifacts/models.py:130-166), not this operation's expected-error contract. If lookup_artifact unexpectedly leaks a sibling such as ArtifactResolutionError("catalog detail"), this branch renders that detail verbatim and bypasses the logging/sanitization below. That conflicts with design/shared.md:254-274; catch ArtifactLookupError here instead, let all other exceptions reach the sanitizer, and add a sibling-ArtifactError regression case.
Narrowed both CLI expected-failure catches from package-wide ArtifactError to operation-specific ArtifactLookupError.
Unexpected sibling errors now reach the existing logged, sanitized internal-failure path.
Added a regression using ArtifactResolutionError("unexpected catalog detail") that verifies the detail is not rendered and the generic JSON error is emitted.
Validation: 109 focused tests passed; 438 artifact tests passed; focused statement/branch coverage remains 100%; Ruff check/format and git diff --check passed. The previously listed MCP descriptor finding remains implemented and covered by the existing inventory/non-registration tests.
AI disclosure: Authored by GitHub Copilot using GPT-5.6 Sol in autonomous mode. Copilot identified and implemented the catch narrowing, regression coverage, validation, and this review-round summary.
Addressed all findings from review 5519960051. Its sole open finding was already implemented in 0dabb649 and remains present at current HEAD da6aedfc: the MCP inventory derives lookup metadata from ARTIFACT_LOOKUP_OPERATION, retains the explicit adapter-pending unavailable reason, and keeps the lookup tool unregistered.
Reverified the MCP info/list and registration suites: 109 passed. The previously stale conversation is now resolved as explicitly requested. No code change or additional commit was necessary.
AI disclosure: Authored by GitHub Copilot using GPT-5.6 Sol in autonomous mode. Copilot inspected the review, reverified the implementation and tests, resolved the already-addressed conversation, and authored this review-round summary.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
triage-can-waitVerdict: valid and in-scope but deprioritized; held behind the evidence gate
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
artifact.lookuplogical operation beneath the existing CLIValidation
LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/artifacts/test_operation_lookup.py tests/specify_cli/artifacts/test_command_lookup.py -q— 94 passedLC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/artifacts/test_operation_lookup.py tests/specify_cli/artifacts/test_command_lookup.py --cov=specify_cli.artifacts._operation_lookup --cov=specify_cli.artifacts.command_lookup --cov-branch --cov-report=term-missing -q— 94 passed; 100% statements and branches for both changed source modulesLC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/artifacts -q— 423 passedLC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/mcp_server tests/specify_cli/test_operation_version.py tests/specify_cli/test_command_version.py tests/specify_cli/test_mcp_version.py -q— 53 passedLC_ALL=en_US.UTF-8 .venv/bin/python -m pytest -q— 12,338 passed, 98 skipped; collection increased from 12,362 to 12,436uvx ruff check src/specify_cli/artifacts/_operation_lookup.py src/specify_cli/artifacts/command_lookup.py tests/specify_cli/artifacts/test_operation_lookup.py tests/specify_cli/artifacts/test_command_lookup.py— passeduvx ruff format --check src/specify_cli/artifacts/_operation_lookup.py src/specify_cli/artifacts/command_lookup.py tests/specify_cli/artifacts/test_operation_lookup.py tests/specify_cli/artifacts/test_command_lookup.py— passedgit diff --check— passedAI disclosure
Implemented with GitHub Copilot using GPT-5.6 Sol in autonomous mode; architecture analysis, code generation, test generation, validation, and PR drafting.