-
Notifications
You must be signed in to change notification settings - Fork 0
FlowSummaryImpl: Model more source/sink steps as jump steps #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: qa/agent-github-codeql/pr-14-22470/base
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,7 +208,7 @@ def get_log_content(status: GithubStatus) -> str: | |
| LOGGER.debug(f"'{status.context}': Getting logs") | ||
| if status.job_ids: | ||
| contents = [subprocess.check_output( | ||
| ["gh", "api", f"/repos/{status.nwo}/actions/jobs/{job_id}/logs"], | ||
| ["gh", "api", f"/repos/{status.nwo}/actions/jobs/{job_id}/logs", "--allow-escape-sequences"], | ||
| ).decode("utf-8") for job_id in status.job_ids] | ||
| content = "\n".join(contents) | ||
| else: | ||
|
|
@@ -275,7 +275,7 @@ def main(pr_number: Optional[int], sha_override: Optional[str] = None, force=Fal | |
|
|
||
| lang_test_failures: List[GithubStatus] = list() | ||
| for status in newest_status.values(): | ||
| if " Language Tests" in status.context or status.context in supported_internal_status_language_test_names: | ||
| if " Language Tests" in status.context and not " Language Tests Windows" in status.context or status.context in supported_internal_status_language_test_names: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shipwright · HIGH The boolean condition in accept-expected-changes-from-ci.py mixes 'and' and 'or' without parentheses. Impact: The boolean condition in accept-expected-changes-from-ci.py mixes 'and' and 'or' without parentheses. Python precedence makes this '(A and not B) or C', so any status whose context is in supported_internal_status_language_test_names is treated as a language test regardless of state or Windows exclusion. This can silently include unintended statuses in lang_test_failures and alter which expected changes are accepted. Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright. |
||
| if status.state == "failure": | ||
| lang_test_failures.append(status) | ||
| elif status.state == "pending": | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shipwright · HIGH
The new '--allow-escape-sequences' flag is added to the 'gh api' log fetch with no comment explaining why it is required or what failure mode it fixes.
Impact: The new '--allow-escape-sequences' flag is added to the 'gh api' log fetch with no comment explaining why it is required or what failure mode it fixes. A future maintainer cannot tell whether removing it is safe, and the flag's behavior may vary across gh versions.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.