diff --git a/_todo/completed/2025-10-21/fix-workflow-status-reporting.md b/_todo/completed/2025-10-21/fix-workflow-status-reporting.md new file mode 100644 index 0000000..a7be18b --- /dev/null +++ b/_todo/completed/2025-10-21/fix-workflow-status-reporting.md @@ -0,0 +1,225 @@ +# Fix Workflow Status Reporting and Auto-Merge + +## Problem Analysis + +### Current Issues + +1. **workflow_run doesn't report status to PR** + - Test workflow triggered via `workflow_run` runs in **main branch context** + - Status check not associated with PR commit + - Branch protection can't see the test result + - Auto-merge blocked indefinitely + +2. **Version bump triggers multiple times** + - Concurrency control working but cancellations happen after work starts + - First run (25s) commits version before being cancelled + - Second run commits again (different version) + - Wasteful and confusing + +3. **Root cause: GITHUB_TOKEN limitations** + - Commits made with `secrets.GITHUB_TOKEN` deliberately **don't trigger workflows** + - GitHub's safeguard against infinite workflow loops + - We're fighting against this design + +### Why workflow_run Doesn't Work + +From GitHub docs and common issues: +- `workflow_run` executes in the context of the **default branch** (main) +- It does NOT run "on behalf of" the PR +- Status checks created are for the wrong commit SHA +- Branch protection rules on PR don't see these checks + +## Best Practice Solutions + +### Option A: Use GitHub App or PAT (Recommended) + +**How it works:** +1. Create GitHub App or use Personal Access Token (PAT) +2. Store token as repository secret (e.g., `BOT_TOKEN`) +3. Use this token for version bump commits instead of `GITHUB_TOKEN` +4. Commits from this token **DO trigger workflows normally** +5. Test runs automatically, reports to PR, auto-merge works + +**Pros:** +- ✅ Clean, standard solution used by major projects +- ✅ No workflow_run hacks +- ✅ Status checks report correctly +- ✅ No manual API calls needed + +**Cons:** +- ⚠️ Requires creating GitHub App or PAT +- ⚠️ Additional secret to manage +- ⚠️ Security consideration (token has write access) + +**Implementation:** +```yaml +# version-bump.yml +- uses: actions/checkout@v4 + with: + token: ${{ secrets.BOT_TOKEN }} # Instead of GITHUB_TOKEN + +# Rest of workflow unchanged - commits will trigger test.yml normally +``` + +**Examples in the wild:** +- Renovate Bot (uses GitHub App) +- Dependabot (uses GitHub App) +- Many semantic-release setups (use PAT) + +### Option B: Manual Status Check via API + +**How it works:** +1. Version bump workflow commits changes +2. Version bump workflow manually creates passing "test" status via API +3. References the test run that already passed before version bump +4. Auto-merge sees the status and proceeds + +**Pros:** +- ✅ No additional tokens needed +- ✅ Uses existing test results + +**Cons:** +- ⚠️ Hacky - we're asserting tests pass without re-running them +- ⚠️ Tests don't actually run on bumped version +- ⚠️ Complex API calls in workflow + +**Implementation:** +```yaml +# version-bump.yml - after commit +- name: Create test status check + env: + GH_TOKEN: ${{ github.token }} + run: | + gh api repos/${{ github.repository }}/statuses/${{ github.event.pull_request.head.sha }} \ + -f state=success \ + -f context=test \ + -f description="Tests passed before version bump" +``` + +### Option C: Remove Auto-Merge (Simplest) + +**How it works:** +1. Version bump commits changes +2. User reviews the version bump +3. User manually merges (or clicks merge button) +4. Release workflow triggers + +**Pros:** +- ✅ Extremely simple +- ✅ No workflow complexity +- ✅ Human verification of version +- ✅ No token management + +**Cons:** +- ⚠️ Not fully automated +- ⚠️ Requires manual action + +**Implementation:** +- Remove auto-merge enable step from version-bump.yml +- User merges when ready + +### Option D: workflow_dispatch Chain + +**How it works:** +1. Version bump commits changes +2. Version bump triggers test via `workflow_dispatch` +3. Test workflow runs in PR context (because we pass the ref) +4. Status reported correctly + +**Pros:** +- ✅ No tokens needed +- ✅ Tests run on bumped version +- ✅ Status reports correctly + +**Cons:** +- ⚠️ More complex workflow coordination +- ⚠️ Need to pass PR context manually + +**Implementation:** +```yaml +# version-bump.yml - after commit +- name: Trigger test workflow + env: + GH_TOKEN: ${{ github.token }} + run: | + gh workflow run test.yml \ + -f pr_number=${{ github.event.pull_request.number }} \ + -f sha=${{ github.event.pull_request.head.sha }} + +# test.yml - add workflow_dispatch trigger +on: + pull_request: + workflow_dispatch: + inputs: + pr_number: + required: true + sha: + required: true +``` + +## Recommendation + +**Option A (GitHub App/PAT)** is the industry standard and cleanest solution, BUT requires setup. + +**Option C (Remove Auto-Merge)** is simplest if you don't mind one manual step. + +**For this project, I recommend:** +1. **Short-term: Option C** - Remove auto-merge, keep workflows simple +2. **Long-term: Option A** - If you want full automation, set up GitHub App + +## Proposed Implementation (Option C - Simple) + +### Changes: +1. **Remove auto-merge step** from version-bump.yml +2. **Remove workflow_run trigger** from test.yml (back to pull_request only) +3. **Update documentation** - user merges after reviewing version bump + +### Updated Flow: +1. Create PR with changes +2. Add `bump:*` labels +3. Version bump workflow commits new version +4. **User reviews version bump** +5. **User merges PR** (via UI or gh CLI) +6. Release workflow publishes package + +### Pros: +- ✅ Simple, no complex workflow orchestration +- ✅ Human verification of version before release +- ✅ No token management +- ✅ No status check issues +- ✅ Tests run normally on PR + +### Cons: +- One manual step (clicking merge button) + +## Alternative Recommendation (Option A - Fully Automated) + +If you want full automation: + +### Setup GitHub App (one-time): +1. Create GitHub App with repo write permissions +2. Install on repository +3. Store private key or create installation token +4. Add as repository secret: `BOT_TOKEN` + +### Changes: +1. version-bump.yml: Use `BOT_TOKEN` instead of `GITHUB_TOKEN` +2. test.yml: Back to simple `pull_request` trigger +3. Keep auto-merge step + +### Flow: +1. Add labels → version bump commits → test runs → auto-merge → release + +Fully automated, no manual steps. + +## Questions + +1. Do you want full automation (Option A - requires GitHub App setup)? +2. Or accept one manual step for simplicity (Option C - remove auto-merge)? +3. Or try the workflow_dispatch approach (Option D - no tokens, moderately complex)? + + \ No newline at end of file diff --git a/_todo/pending/new-release-workflow.md b/_todo/completed/2025-10-21/new-release-workflow.md similarity index 96% rename from _todo/pending/new-release-workflow.md rename to _todo/completed/2025-10-21/new-release-workflow.md index 5211d67..74bdb7a 100644 --- a/_todo/pending/new-release-workflow.md +++ b/_todo/completed/2025-10-21/new-release-workflow.md @@ -1,8 +1,26 @@ # New Release Workflow -**Status**: In Progress -**Branch**: `feature/automated-release-workflow` +**Status**: ✅ Completed **Started**: 2025-10-21 +**Completed**: 2025-10-21 +**Final Branch**: `test/workflow-fixes` (PR #26) + +## Completion Summary + +Successfully implemented automated release workflow with label-driven publishing to PyPI/TestPyPI. + +**Final Approach**: Manual version bumping with automated publishing +- User manually runs `uv version --bump ` and commits +- Release workflow automatically publishes when PR with release label is merged +- Simple, reliable, no complex workflow orchestration + +**Key Achievement**: Working release workflow that publishes to PyPI/TestPyPI with improved GitHub Release notes + +**Lessons Learned**: +- GITHUB_TOKEN commits don't trigger other workflows (GitHub safety feature) +- workflow_run executes in wrong context for PR status checks +- Sometimes simple manual steps are better than fighting automation limitations +- Archived automated version-bump workflow for future reference if GitHub App approach is needed ## Progress Log diff --git a/pyproject.toml b/pyproject.toml index 7ed44b0..78154d2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "quarto-batch-convert" -version = "2025.9.1rc5" +version = "2025.9.1" description = "Converts multiple Jupyter notebooks to Quarto documents at once" readme = "README.md" license = "MIT" @@ -23,7 +23,7 @@ dev = [ [project.scripts] quarto-batch-convert = "quarto_batch_convert.quarto_batch_convert:convert_files" -qbc = "quarto_batch_convert.quarto_batch_convert:convert_files" +qbc = "quarto_batch_convert.quarto_batch_convert:quarto_batch_convert" [build-system] requires = ["uv_build>=0.8.9,<0.9.0"] diff --git a/src/quarto_batch_convert/quarto_batch_convert.py b/src/quarto_batch_convert/quarto_batch_convert.py index 1a08001..7a6119f 100644 --- a/src/quarto_batch_convert/quarto_batch_convert.py +++ b/src/quarto_batch_convert/quarto_batch_convert.py @@ -7,8 +7,31 @@ import re from typing import List, Optional from pathlib import Path - -from .version import __version__ +from importlib.metadata import version, PackageNotFoundError + +def get_package_info() -> str: + """Get formatted package information for epilogue display.""" + from importlib.metadata import metadata + try: + meta = metadata("quarto-batch-convert") + pkg_version = meta.get("Version", "unknown") + author = meta.get("Author", "kompre") + return f"quarto-batch-convert v{pkg_version} | by {author} | https://github.com/kompre/quarto_batch_convert" + except PackageNotFoundError: + return "quarto-batch-convert | https://github.com/kompre/quarto_batch_convert" + +def get_epilog() -> str: + """Get formatted epilog with examples and package info.""" + return f"""Examples: qbc . -r -m "^__/_" + +{get_package_info()}""" + +def get_version() -> str: + """Get package version from metadata.""" + try: + return version("quarto-batch-convert") + except PackageNotFoundError: + return "unknown" def check_quarto_installation() -> None: @@ -131,7 +154,11 @@ def convert_file( subprocess.run(["quarto", "convert", file, "--output", new_file_path]) -@click.command(no_args_is_help=True) +@click.command( + no_args_is_help=True, + epilog=get_epilog(), + help="Batch quarto convert multiple .ipynb to .qmd files or vice versa", +) @click.argument( "input_paths", nargs=-1, @@ -169,8 +196,30 @@ def convert_file( is_flag=True, help="Search files recursively when input is a directory", ) -@click.version_option(version=__version__, prog_name="Quarto Batch Converter") +@click.version_option(version=get_version(), prog_name="Quarto Batch Converter") @click.pass_context +def quarto_batch_convert( + ctx: click.Context, + input_paths: tuple, + qmd_to_ipynb: bool, + match_replace_pattern: Optional[str], + prefix: str, + keep_extension: bool, + output_path: Optional[str], + recursive: bool, +) -> None: + """ CLI wrapper for convert_files - see help parameters @click.command decorator""" + return convert_files( + ctx, + input_paths, + qmd_to_ipynb, + match_replace_pattern, + prefix, + keep_extension, + output_path, + recursive, + ) + def convert_files( ctx: click.Context, input_paths: tuple, @@ -308,5 +357,8 @@ def convert_files( print("-" * len(text)) + + + if __name__ == "__main__": pass diff --git a/src/quarto_batch_convert/version.py b/src/quarto_batch_convert/version.py deleted file mode 100644 index 4bd29a2..0000000 --- a/src/quarto_batch_convert/version.py +++ /dev/null @@ -1,25 +0,0 @@ -# import tomllib -# from pathlib import Path - -# # def get_version(): -# # """Read version from pyproject.toml.""" -# # pyproject_path = Path(__file__).parent.parent.parent / "pyproject.toml" - -# # if not pyproject_path.exists(): -# # return "unknown" - -# # try: -# # with open(pyproject_path, "rb") as f: -# # pyproject = tomllib.load(f) -# # return pyproject["project"]["version"] -# # except (KeyError, FileNotFoundError, tomllib.TOMLDecodeError): -# # return "unknown" - -# # __version__ = get_version() - - -import importlib.metadata - -__version__ = importlib.metadata.version("quarto-batch-convert") - -# print(__version__) \ No newline at end of file diff --git a/tests/manual.py b/tests/manual.py index 731a549..72e5f1b 100644 --- a/tests/manual.py +++ b/tests/manual.py @@ -1,20 +1,20 @@ -from quarto_batch_convert.quarto_batch_convert import convert_files +from quarto_batch_convert.quarto_batch_convert import quarto_batch_convert import os import glob def change_dir(): os.chdir("tests/assets") - convert_files(["*"]) + quarto_batch_convert(["*"]) def is_recursive(): - convert_files( + quarto_batch_convert( ["tests/assets"] ) def simple_run(): # files = glob.glob("C:/Users/s.follador/Desktop/__canc__/qbc/**/*.ipynb", recursive=True) files = ["C:/Users/s.follador/Desktop/__canc__/qbc/**/*"] - convert_files( + quarto_batch_convert( [ *files ] diff --git a/tests/test_quarto_batch_convert.py b/tests/test_quarto_batch_convert.py index 259700c..58c351d 100644 --- a/tests/test_quarto_batch_convert.py +++ b/tests/test_quarto_batch_convert.py @@ -2,7 +2,7 @@ import shutil import pytest from click.testing import CliRunner -from quarto_batch_convert.quarto_batch_convert import convert_files +from quarto_batch_convert.quarto_batch_convert import quarto_batch_convert import glob from contextlib import contextmanager from typing import Generator @@ -78,7 +78,7 @@ def test_single_match_no_replace(setup_teardown_test_env: str) -> None: input_files = glob.glob(test_dir + "/**/*", recursive=True) - result = runner.invoke(convert_files, [*input_files, "-m", "test_2"]) + result = runner.invoke(quarto_batch_convert, [*input_files, "-m", "test_2"]) assert result.exit_code == 0 assert "test_2.ipynb" in result.output @@ -103,7 +103,7 @@ def test_match_and_replace_pattern(setup_teardown_test_env: str) -> None: input_file = os.path.join(test_dir, "notebooks/_test_1.ipynb") - result = runner.invoke(convert_files, [input_file, "-o", output_dir, "-m", "^_/REPLACED_"]) + result = runner.invoke(quarto_batch_convert, [input_file, "-o", output_dir, "-m", "^_/REPLACED_"]) assert result.exit_code == 0 assert os.path.exists(os.path.join(output_dir, os.path.dirname(input_file), "REPLACED_test_1.qmd")) @@ -120,7 +120,7 @@ def test_invalid_regex_pattern() -> None: an error code when an invalid pattern is provided. """ runner = CliRunner() - result = runner.invoke(convert_files, ["./", "-m", "[invalid"]) + result = runner.invoke(quarto_batch_convert, ["./", "-m", "[invalid"]) assert result.exit_code != 0 assert "Invalid regex pattern" in result.output @@ -139,7 +139,7 @@ def test_no_match_found(setup_teardown_test_env: str) -> None: input_files = glob.glob(test_dir + "/**/*", recursive=True) - result = runner.invoke(convert_files, [*input_files, "-m", "non_existent_pattern"]) + result = runner.invoke(quarto_batch_convert, [*input_files, "-m", "non_existent_pattern"]) assert result.exit_code != 0 assert "No files found matching the regex pattern" in result.output @@ -156,7 +156,7 @@ def test_prefix_option(setup_teardown_test_env: str) -> None: test_dir = setup_teardown_test_env input_file = os.path.join(test_dir, "notebooks/_test_1.ipynb") - result = runner.invoke(convert_files, [input_file, "-p", "prefix_"]) + result = runner.invoke(quarto_batch_convert, [input_file, "-p", "prefix_"]) assert result.exit_code == 0 assert os.path.exists(os.path.join(test_dir, "notebooks/prefix__test_1.qmd")) @@ -175,7 +175,7 @@ def test_keep_extension_option(setup_teardown_test_env: str) -> None: input_file = os.path.join(test_dir, "file_in_root.ipynb") - result = runner.invoke(convert_files, [input_file, "-k"]) + result = runner.invoke(quarto_batch_convert, [input_file, "-k"]) assert result.exit_code == 0 assert os.path.exists(os.path.join(os.path.dirname(input_file), "file_in_root.ipynb.qmd")) @@ -193,7 +193,7 @@ def test_convert_qmd_to_ipynb(setup_teardown_test_env: str) -> None: test_dir = setup_teardown_test_env input_file = os.path.join(test_dir, "notebooks", "TEST.qmd") - result = runner.invoke(convert_files, [input_file, "-q"]) + result = runner.invoke(quarto_batch_convert, [input_file, "-q"]) assert result.exit_code == 0 assert os.path.exists(os.path.join(os.path.dirname(input_file), "TEST.ipynb")) @@ -213,7 +213,7 @@ def test_file_in_cwd(setup_teardown_test_env: str) -> None: with change_dir(test_dir): input_files = glob.glob("*", recursive=True) - result = runner.invoke(convert_files, [*input_files]) + result = runner.invoke(quarto_batch_convert, [*input_files]) assert result.exit_code == 0 # assert "input_path cannot be empty" in result.output @@ -232,7 +232,7 @@ def test_prefix_to_new_dir(setup_teardown_test_env: str) -> None: input_file = os.path.join(test_dir, "notebooks/_test_1.ipynb") input_prefix = "PREFIX/" - result = runner.invoke(convert_files, [input_file, "-p", input_prefix]) + result = runner.invoke(quarto_batch_convert, [input_file, "-p", input_prefix]) file_name, _ = os.path.splitext(os.path.basename(input_file)) assert result.exit_code == 0 @@ -252,7 +252,7 @@ def test_prefix_to_nested_dir(setup_teardown_test_env: str) -> None: input_file = os.path.join(test_dir, "notebooks/_test_1.ipynb") input_prefix = "../PREFIX/" - result = runner.invoke(convert_files, [input_file, "-p", input_prefix]) + result = runner.invoke(quarto_batch_convert, [input_file, "-p", input_prefix]) file_name, _ = os.path.splitext(os.path.basename(input_file)) assert result.exit_code == 0 @@ -272,7 +272,7 @@ def test_output_path(setup_teardown_test_env: str) -> None: output_dir = os.path.join(test_dir, "output") input_file = os.path.join(test_dir, "notebooks/_test_1.ipynb") - result = runner.invoke(convert_files, [input_file, "-o", output_dir]) + result = runner.invoke(quarto_batch_convert, [input_file, "-o", output_dir]) file_name, _ = os.path.splitext(os.path.basename(input_file)) @@ -294,7 +294,7 @@ def test_recursive_option_with_nested_directory() -> None: try: # Run with recursive flag - result = runner.invoke(convert_files, [test_dir, "-r", "-o", output_dir]) + result = runner.invoke(quarto_batch_convert, [test_dir, "-r", "-o", output_dir]) assert result.exit_code == 0 @@ -327,7 +327,7 @@ def test_non_recursive_option_ignores_subdirectories() -> None: try: # Run WITHOUT recursive flag - result = runner.invoke(convert_files, [test_dir, "-o", output_dir]) + result = runner.invoke(quarto_batch_convert, [test_dir, "-o", output_dir]) assert result.exit_code == 0 @@ -358,7 +358,7 @@ def test_recursive_with_match_pattern() -> None: try: # Run with recursive flag and match pattern - result = runner.invoke(convert_files, [test_dir, "-r", "-o", output_dir, "-m", "__test3"]) + result = runner.invoke(quarto_batch_convert, [test_dir, "-r", "-o", output_dir, "-m", "__test3"]) assert result.exit_code == 0 @@ -389,7 +389,7 @@ def test_recursive_preserves_directory_structure() -> None: try: # Run with recursive flag - result = runner.invoke(convert_files, [test_dir, "-r", "-o", output_dir]) + result = runner.invoke(quarto_batch_convert, [test_dir, "-r", "-o", output_dir]) assert result.exit_code == 0 diff --git a/uv.lock b/uv.lock index 581f024..da994a5 100644 --- a/uv.lock +++ b/uv.lock @@ -81,7 +81,7 @@ wheels = [ [[package]] name = "quarto-batch-convert" -version = "2025.9.1rc5" +version = "2025.9.1" source = { editable = "." } dependencies = [ { name = "click" },