[codex] Fix changes timestamp Z format#379
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens the direct changes check / direct changes check-campaigns timestamp contract to match the Yandex Direct API docs by accepting only YYYY-MM-DDTHH:MM:SSZ and rejecting bare timestamps without the trailing Z.
Changes:
- Updated
parse_datetime()andchangescommand help text to require...SSZand stop auto-appendingZ. - Refreshed documentation/examples (README, AGENTS) and command fixtures/scripts to use the canonical
...SSZtimestamp. - Updated and extended regression/unit/integration tests to cover acceptance/rejection and payload wiring.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
direct_cli/utils.py |
Enforces strict YYYY-MM-DDTHH:MM:SSZ parsing for Changes timestamps. |
direct_cli/commands/changes.py |
Updates --timestamp help text and relies on strict parsing for API payloads. |
README.md |
Updates documented datetime rules and all changes examples to include Z. |
AGENTS.md |
Updates agent-facing rules/examples for changes timestamps. |
tests/test_changes.py |
Verifies strict timestamp passthrough and adds direct parse_datetime() regression tests. |
tests/test_low_coverage_payloads.py |
Updates canonical payload expectations and adds rejection coverage for bare/malformed timestamps. |
tests/test_cli.py |
Ensures help output documents the strict ...SSZ format. |
tests/test_cli_contract.py |
Adjusts README contract checks: allow Z only for direct changes ... examples; reject bare changes timestamps. |
tests/test_integration.py |
Generates integration probe timestamps with a trailing Z. |
scripts/test_safe_commands.sh |
Updates safe-command probe to pass a ...SSZ timestamp. |
scripts/build_api_coverage_report.py |
Updates fixture command args to use a ...SSZ timestamp. |
|
@codex review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production. |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
changes checkandchanges check-campaignstimestamp input with Yandex Direct docs:YYYY-MM-DDTHH:MM:SSZonly.YYYY-MM-DDTHH:MM:SSfor Changes timestamps.Why
Yandex documents
Changes.check/Changes.checkCampaignsTimestampasYYYY-MM-DDThh:mm:ssZ. The CLI previously accepted a bare timestamp and appendedZinternally, which drifted from the API contract and broke the plugin-side expectation tracked inaxisrow/yandex-direct-mcp-plugin#115.Closes #235.
Validation
python3 -m pytest tests/test_low_coverage_payloads.py tests/test_changes.py tests/test_cli.py tests/test_cli_contract.py -q->122 passed, 30 subtests passedpython3 -m pytest -m "not integration" -q->1323 passed, 44 skipped, 32 deselected, 30 subtests passedgit diff --check-> clean