Skip to content

fix: use actual Yunxiao status names & move config to vars#2164

Closed
shinetata wants to merge 7 commits into
MemTensor:mainfrom
shinetata:feat/yunxiao-github-sync-preflight
Closed

fix: use actual Yunxiao status names & move config to vars#2164
shinetata wants to merge 7 commits into
MemTensor:mainfrom
shinetata:feat/yunxiao-github-sync-preflight

Conversation

@shinetata

Copy link
Copy Markdown
Collaborator

Summary

  • Fix status names to match actual Yunxiao workflow (待处理/设计中/开发中/已完成/已取消/测试中)
  • Move project config (ID, name, assignee) to GitHub Variables instead of hardcoding
  • Remove priority_name from preflight (hardcoded since priority is always 中)
  • All tests pass, ruff clean

Made with Proma · GitHub

@Memtensor-AI Memtensor-AI added area:core MOS 编排层 / 框架底座 / 跨模块问题 status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Jul 25, 2026
@Memtensor-AI
Memtensor-AI requested a review from WeiminLee July 25, 2026 08:17
@Memtensor-AI

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2164
Task: 818be61b8a03f617
Base: main
Head: feat/yunxiao-github-sync-preflight

🔍 OpenCodeReview found 11 issue(s) in this PR.

⚠️ 1 warning(s) occurred during review.


1. tests/test_yunxiao_github_sync.py (L10-L13)

Using assert to guard against None before SPEC.loader.exec_module(MODULE) is unsafe. Under python -O (optimized mode), all assert statements are stripped, so this becomes a no-op. If spec_from_file_location returns None or produces a spec with a None loader (e.g., the script path is wrong in CI), the subsequent exec_module call will raise an AttributeError: 'NoneType' object has no attribute 'exec_module' with no helpful message.

Replace the assert with an explicit if guard that raises a clear error:

💡 Suggested Change

Before:

SPEC = importlib.util.spec_from_file_location("yunxiao_github_sync", MODULE_PATH)
assert SPEC is not None and SPEC.loader is not None
MODULE = importlib.util.module_from_spec(SPEC)
SPEC.loader.exec_module(MODULE)

After:

SPEC = importlib.util.spec_from_file_location("yunxiao_github_sync", MODULE_PATH)
if SPEC is None or SPEC.loader is None:
    raise ImportError(f"Cannot load module from {MODULE_PATH}")
MODULE = importlib.util.module_from_spec(SPEC)
SPEC.loader.exec_module(MODULE)

2. .github/workflows/yunxiao-github-sync.yml (L6-L7)

The pull_request_target trigger runs with write-level repository permissions and full access to secrets, even for PRs from forks. The Python sync script (scripts/yunxiao_github_sync.py) only reads GitHub event data and writes to Yunxiao — it never posts back to GitHub — so write permissions are not needed. Using pull_request instead eliminates the risk that a fork PR can trigger a workflow that has access to YUNXIAO_TOKEN and GH_TOKEN. If pull_request_target must be kept (e.g., to ensure the script from the default branch is always run), add a comment documenting why, and ensure the checkout ref never changes to the PR head SHA.

💡 Suggested Change

Before:

  pull_request_target:
    types: [opened, closed, reopened]

After:

  pull_request:
    types: [opened, closed, reopened]

3. scripts/yunxiao_github_sync.py (L139-L145)

The preflight() function declares 5 parameters, but every call site passes 6 arguments — an extra YUNXIAO_DEFAULT_PRIORITY_NAME string is inserted between assignee_name and client at lines 362, 399–401, and 472–474. This is a TypeError that crashes every execution path at runtime.

Either add the missing priority_name: str parameter to the signature, or remove the extra argument from all call sites.

💡 Suggested Change

Before:

def preflight(
    org: str,
    project_id: str,
    project_name: str,
    assignee_name: str,
    client: YunxiaoClient,
) -> dict[str, Any]:

After:

def preflight(
    org: str,
    project_id: str,
    project_name: str,
    assignee_name: str,
    priority_name: str,
    client: YunxiaoClient,
) -> dict[str, Any]:

4. scripts/yunxiao_github_sync.py (L189)

The priority_id is hardcoded as a literal string inside preflight(), making the YUNXIAO_DEFAULT_PRIORITY_NAME environment variable (passed at every call site) completely ignored. Once the missing priority_name parameter is added (see above), the priority should be resolved from the API using that name — similar to how assignee is resolved — rather than using this hardcoded value.


5. scripts/yunxiao_github_sync.py (L338-L339)

The event file is opened without any error handling. If GITHUB_EVENT_PATH points to a missing or unreadable file, an unhandled FileNotFoundError/PermissionError propagates uncontrolled. Wrap this in a try/except to produce a clean error message and a non-zero exit code, consistent with the rest of the error handling in handle_event().

💡 Suggested Change

Before:

    with open(event_path) as f:
        event = json.load(f)

After:

    try:
        with open(event_path) as f:
            event = json.load(f)
    except OSError as e:
        print(f"无法读取事件文件 {event_path}: {e}", file=sys.stderr)
        return 2

6. scripts/yunxiao_github_sync.py (L388-L389)

These two gh() calls have no error handling. An HTTP 401/403/429 from the GitHub API raises an unhandled urllib.error.HTTPError that propagates out of backfill() entirely, bypassing all per-item failure accounting and producing no summary output.

Additionally, with per_page=100 and no pagination, repositories with more than 100 open issues or PRs will silently produce an incomplete sync. Follow the Link: rel="next" header to fetch all pages.


7. scripts/yunxiao_github_sync.py (L103-L106)

Using if v: treats falsy-but-valid values (e.g., integer 0 or an empty string "") as absent, causing _item_id() to skip them and either return a wrong ID from a later key or raise PreflightError. Use if v is not None: to distinguish a legitimately missing key from a falsy value.

💡 Suggested Change

Before:

    for key in ("id", "identifier", "organizationId", "userId", "statusId", "value"):
        v = item.get(key)
        if v:
            return str(v)

After:

    for key in ("id", "identifier", "organizationId", "userId", "statusId", "value"):
        v = item.get(key)
        if v is not None:
            return str(v)

8. scripts/yunxiao_github_sync.py (L322)

The loop variable l (lowercase L) is explicitly prohibited by PEP 8 because it is visually indistinguishable from the digit 1 in many fonts. Use a more descriptive name such as lbl.

💡 Suggested Change

Before:

        github_labels = [l["name"] for l in item.get("labels", [])]

After:

        github_labels = [lbl["name"] for lbl in item.get("labels", [])]

9. scripts/yunxiao_github_sync.py (L377)

urllib.request is already imported at the module level (from urllib.request import Request, urlopen). This deferred inner import is redundant, adds a small overhead on every backfill() call, and hides the dependency from the top-level imports. Remove it and use urlopen/Request directly, or add a top-level import urllib.request if the ur.Request alias is preferred.


10. .github/workflows/yunxiao-github-sync.yml (L55)

The github.event_name context value is interpolated directly into the shell run: block. While this specific value is controlled by GitHub Actions itself and not user-supplied, mixing ${{ }} expression interpolation with shell variables inside run: is a bad pattern that can be easily copied and extended with user-controlled inputs. Use the pre-set $GITHUB_EVENT_NAME environment variable instead, which is always available in the runner environment.

💡 Suggested Change

Before:

          if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then

After:

          if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ]; then

11. .github/workflows/yunxiao-github-sync.yml (L60)

Same interpolation concern as above: replace ${{ github.event_name }} with $GITHUB_EVENT_NAME throughout the shell script.

💡 Suggested Change

Before:

          elif [ "${{ github.event_name }}" = "issues" ] || [ "${{ github.event_name }}" = "pull_request_target" ]; then

After:

          elif [ "$GITHUB_EVENT_NAME" = "issues" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_target" ]; then

🧹 Filtered 1 low-confidence OCR finding(s) before posting/fix-loop (existing_code_mismatch: 1).

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Executor error: Command failed: git merge --no-edit base/main
Branch: feat/yunxiao-github-sync-preflight

@shinetata

Copy link
Copy Markdown
Collaborator Author

分支历史与 main 冲突,改由基于最新 main 的干净分支提交。

@shinetata shinetata closed this Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core MOS 编排层 / 框架底座 / 跨模块问题 status:in-progress Someone or AI is working on it | 人工或 AI 正在处理

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants