docs: promote review fix quickstart update to main#53
Merged
Merged
Conversation
Implement /flow-watch as a read-only GitHub PR checks watcher with status normalization, summaries, bounded session state, docs, and tests.\n\nValidation:\n- npm run check\n- npm run typecheck\n- npm test\n- npm run check:json\n- npm run check:text\n- npm run check:docs\n- git diff --check\n\nRefs #13
Context: Codex flagged that gh pr checks --required can return a non-zero no required checks reported message that was not recognized as an empty checks result. Changes: - Recognize GitHub CLI no required checks reported/found and no required check runs messages as no_checks. - Added regression coverage for the required-only no required checks path. - Updated PR and ADR docs for the normalized no_checks behavior. Verification: - npm run check - npm run typecheck - npm test - npm run check:json - npm run check:text - npm run check:docs - git diff --check
Context: Codex flagged that /flow-watch could start a new poll after the configured deadline and accept a terminal sample as non-timeout. Changes: - Check the watch deadline before starting each new GitHub checks poll. - Reuse the latest pre-deadline sample when the deadline has elapsed. - Added regression coverage to ensure a post-deadline terminal sample is not polled or accepted. - Updated PR and ADR docs. Verification: - npm run check - npm run typecheck - npm test - npm run check:json - npm run check:text - npm run check:docs - git diff --check
Context: Codex flagged that mixed passed and skipped GitHub checks were summarized with skipped rows in the passed section and again in the skipped section. Changes: - Render only passed/neutral checks in the passed summary section. - Keep skipped checks in the skipped section. - Added regression coverage for mixed passed and skipped summaries. - Updated PR docs. Verification: - npm run check - npm run typecheck - npm test - npm run check:json - npm run check:text - npm run check:docs - git diff --check
Context: Codex flagged that per-check GitHub details URLs could be stored unbounded in session state. Changes: - Truncate stored per-check strings in GitHub checks state, including details URLs. - Added regression coverage for long check names, workflows, and details URLs. - Updated PR, security, and ADR docs for bounded stored per-check strings. Verification: - npm run check - npm run typecheck - npm test - npm run check:json - npm run check:text - npm run check:docs - git diff --check
Context: Issue #14 was still marked needs-design after the GitHub checks watcher foundation. Review-thread automation needs a clear safety and data model before implementation because it may eventually reply to and resolve reviewer conversations. Changes: - Documented GitHub review comment and review thread concepts. - Defined the review thread data model and triage classification schema. - Split the implementation plan into read-only triage and mutating fix/resolve phases. - Documented safety rules for replies, resolution, human decisions, and checks-before-resolve. - Updated workflow, state machine, configuration, prompts, templates, and implementation plan for the review-comments loop. - Addressed Codex review feedback on unsafe invalid auto-resolution, human-required resolution, and tentative read-only reply drafts. Verification: - npm run check - npm run typecheck - npm test - npm run check:json - npm run check:text - npm run check:docs - git diff --check Risk: Low. Documentation, schema, prompt, template, and planning changes only. Refs: #14
Stop /flow-watch polling when GitHub CLI reports a terminal no-selected-checks result, while preserving polling for transient empty check samples. Verification: - npm run check - npm run typecheck - npm test - npm run check:json - npm run check:text - npm run check:docs - git diff --check Refs: #13
Keep generic no-checks GitHub CLI responses retryable in watch mode and only stop early for no-selected-required-checks responses. Verification: - npm test -- tests/github/pr-checks-client.test.ts - npm run check - npm run typecheck - npm test - npm run check:json - npm run check:text - npm run check:docs - git diff --check Refs: #13
Keep no-checks and no-required-checks GitHub CLI responses retryable in watch mode so checks that appear shortly after PR creation are not missed. Verification: - npm test -- tests/github/pr-checks-client.test.ts - npm run check - npm run typecheck - npm test - npm run check:json - npm run check:text - npm run check:docs - git diff --check Refs: #13
Context: Codex review on the dev-to-main promotion found that incomplete scans still returned found when selected threads were present. Changes: - Treat any incomplete review-thread scan as failed/blocked, even when selected threads were found. - Add regression coverage for incomplete scans after selected threads are present. Verification: - npm run check - git diff --check Refs: #14
Context: Codex review on the dev-to-main promotion found that valid triage payloads stayed in review_triage despite requiring fixes. Changes: - Move /flow-comments results with valid triage classifications to fixing_review_findings. - Add regression coverage for the lifecycle transition. - Align workflow and state-machine docs. Verification: - npm run check - git diff --check Refs: #14
Context: Codex review on the dev-to-main promotion found that partial triage payloads could advance selected threads to fixing while leaving other selected threads unclassified. Changes: - Add complete selected thread ID validation for triage payloads. - Require /flow-comments triage payloads to include every selected filtered thread. - Update command and validation tests. - Document that selected threads cannot be omitted from triage payloads. Verification: - npm run check - git diff --check Refs: #14
Context: Codex review on the dev-to-main promotion found duplicate triage thread IDs could inflate counts or conflict classifications. Changes: - Reject duplicate triage thread IDs during validation. - Add regression coverage for duplicate IDs. - Document duplicate ID rejection. Verification: - npm run check - git diff --check Refs: #14
Context: Codex review on the dev-to-main promotion found that /flow-comments could combine the current directory repository with a PR from a different upstream/base repository. Changes: - Derive GraphQL owner/repo from the PR URL returned by gh pr view. - Stop calling gh repo view as a separate repository source for review thread reads. - Update tests to assert the PR URL repository drives GraphQL queries. Verification: - npm run check - git diff --check Refs: #14
Context: The package now has implementation code, tests, schemas, docs checks, and validation scripts. CI should validate every PR before further release-readiness work. Changes: - Added GitHub Actions validation workflow. - Configured validation for pull requests into `dev` and `main`. - Configured validation for pushes to `dev` and `main`. - Added manual workflow dispatch. - Used read-only permissions and no secrets. - Updated documentation for CI behavior. Verification: - npm run check - npm run typecheck - npm test - npm run check:json - npm run check:text - npm run check:docs - git diff --check Risk: Low. This adds CI validation only. It does not publish packages, deploy, require secrets, or mutate repository state. Refs: #17
Resolve the dev-to-main promotion conflict by keeping the newer dev implementation-plan status while recording main as merged. Verification: - npm run check
Context: pi-codeflow now has the implemented workflow commands, review-comments loop, CI validation, and reviewed installation/usage docs. Changes: - Added installation, usage, examples, and troubleshooting docs. - Updated README navigation and project status. - Updated safety, release, configuration, architecture, and agent docs. - Addressed Codex review feedback on staging, check mutation wording, and config snippet schemas. Verification: - npm run check - npm run check:docs - npm run check:text - git diff --check - GitHub Validate package passed Risk: Low. Documentation-only change. Refs #15
Context: Follow-up documentation fix after Codex review found the quickstart review-comment loop skipped required commit and remote-check evidence before /flow-fix-comments. Changes: - Added review-fix /flow-check, staging, fresh commit payload, /flow-commit, /flow-pr, and /flow-watch steps before /flow-fix-comments. Verification: - npm run check - npm run check:docs - npm run check:text - git diff --check - PR #52 Validate package passed - PR #52 review threads resolved Risk: Low. Documentation-only fix. Refs #15
![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: e4ac928cc3
ℹ️ 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".
Context: Codex review on PR #53 found the README lifecycle skipped /flow-pr after the review-fix /flow-commit. Changes: - Added /flow-pr before the second /flow-watch in the README lifecycle. - Kept the no-content sync merge with main so PR #53 can become mergeable. Verification: - npm run check - npm run check:docs - npm run check:text - git diff --check Risk: Low. Documentation-only fix plus no-content history sync. Refs #15
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ 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". |
Context: Repeated dev-to-main promotion PRs have hit avoidable conflicts when promotion PRs are squash-merged. Changes: - Document that dev-to-main promotion PRs should use merge commits. - Add a release-process check for ensuring main is already in dev before opening the next promotion PR. - Note the fallback sync step when a previous promotion was squashed. Verification: - npm run check:docs - npm run check:text - git diff --check Risk: Low. Documentation-only policy update. Refs #15
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f0698bafa
ℹ️ 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".
Context: Codex review on PR #53 raised the risk of promotion branches that copy dev content onto main without preserving dev history. Changes: - Clarified that promotion PRs should come from dev itself or preserve dev ancestry. - Documented that tree-copy single-parent promotion commits should not be used. Verification: - npm run check:docs - npm run check:text - git diff --check Risk: Low. Documentation-only clarification. Refs #15
Owner
Author
|
@codex review |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Promote the review-fix quickstart documentation update from
devtomain.Context
PR #52 fixed valid Codex review feedback by clarifying that review-fix changes must be checked, staged, committed with a fresh payload, pushed through the PR flow, and watched before
/flow-fix-commentsreplies or resolution.Changes
docs/USAGE.mdreview-comment quickstart steps./flow-fix-commentsafter refreshed local and remote verification.Verification
Validate package: passeddocs/USAGE.mdSelf-review
devtomainonly.Risk
Low. Documentation-only promotion.
Rollback
Revert this PR from
mainif needed.Reviewer notes
Please focus on promotion correctness and whether
docs/USAGE.mdnow reflects the implemented review-fix policy.Linked issues
Refs #15