From 142b530088439bae2daed7a11ba79e0173ee7956 Mon Sep 17 00:00:00 2001 From: prakashUXtech Date: Thu, 2 Jul 2026 21:13:03 +0530 Subject: [PATCH 1/2] ci: run tests on dev PRs and stop the quality gate crashing on forks CI only triggered on pushes and PRs targeting main, but dev is the working branch. Every dev-targeted PR merged without lint or the 3.11/3.12/3.13 test matrix ever running, so green checks proved nothing. Add dev to both triggers in ci.yml. The quality gate also 403-crashed on PRs from forks: fork PRs get a read-only GITHUB_TOKEN, so the addLabels call threw and the whole check reported failure for the wrong reason. Wrap the label and comment writes so the gate reports its verdict through the run summary instead of crashing, and keep the pass/fail behavior otherwise unchanged. --- .github/workflows/ci.yml | 10 ++-- .github/workflows/pr-quality-gate.yml | 86 ++++++++++++++++++--------- 2 files changed, 65 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 89b49dee..b055cb55 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,12 +1,14 @@ -# CI — lint and test on every PR and push to main. -# Updated: 2026-03-05 — Lint errors fixed, lint job now blocks PRs. +# CI — lint and test on every PR and push to the mainline branches. +# Updated: 2026-07-02 — Run on PRs and pushes targeting dev, not just main. +# dev is the working branch, so main-only triggers meant lint and the +# 3.11/3.12/3.13 test matrix never ran on the PRs that actually merge. name: CI on: push: - branches: [main] + branches: [main, dev] pull_request: - branches: [main] + branches: [main, dev] permissions: contents: read diff --git a/.github/workflows/pr-quality-gate.yml b/.github/workflows/pr-quality-gate.yml index 69ef46fa..bc70a207 100644 --- a/.github/workflows/pr-quality-gate.yml +++ b/.github/workflows/pr-quality-gate.yml @@ -1,5 +1,7 @@ # PR quality gate — enforces contribution standards and security checks. # Added: 2026-03-05 — Quality gate + security scan for soul-protocol. +# Updated: 2026-07-02 — Wrap label/comment writes so fork PRs (read-only +# token) report the verdict via the run summary instead of 403-crashing. name: PR Quality Gate on: @@ -115,16 +117,32 @@ jobs: c => c.user.type === 'Bot' && c.body.includes(MARKER) ); + // Fork PRs run with a read-only GITHUB_TOKEN, so labeling and + // commenting return 403 no matter what the permissions block says. + // Wrap every write so the gate still reports its verdict (via the + // run summary) instead of crashing the whole check. + async function tryWrite(desc, fn) { + try { + await fn(); + return true; + } catch (e) { + core.warning(`Skipped ${desc}: ${e.message}. Expected on PRs from forks, whose token is read-only.`); + return false; + } + } + if (issues.length > 0 || warnings.length > 0) { let message = `${MARKER}\n`; if (issues.length > 0) { - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - labels: ['needs-work'] - }); + await tryWrite('adding the needs-work label', () => + github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number, + labels: ['needs-work'] + }) + ); message += `### Issues (must fix)\n\n${issues.join('\n')}\n\n`; } @@ -134,20 +152,32 @@ jobs: message += `Please update your PR to address these points.`; - if (botComment) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: botComment.id, - body: message - }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - body: message - }); + const posted = botComment + ? await tryWrite('updating the quality-gate comment', () => + github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: botComment.id, + body: message + }) + ) + : await tryWrite('posting the quality-gate comment', () => + github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number, + body: message + }) + ); + + // Always surface the verdict in the run summary so it is visible + // even when the token cannot write to the PR. + await core.summary + .addHeading('PR quality gate', 2) + .addRaw(message.replace(MARKER, '').trim()) + .write(); + if (!posted) { + core.warning('Could not post quality-gate feedback to the PR; see the run summary above.'); } } else { try { @@ -157,15 +187,17 @@ jobs: issue_number: pr.number, name: 'needs-work' }); - } catch (e) { /* label wasn't present */ } + } catch (e) { /* label absent, or read-only fork token */ } if (botComment) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: botComment.id, - body: `${MARKER}\nAll quality checks passed. Thanks for the clean PR!` - }); + await tryWrite('updating the quality-gate comment', () => + github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: botComment.id, + body: `${MARKER}\nAll quality checks passed. Thanks for the clean PR!` + }) + ); } } From 0bc817752ca1f9cb562e6ef7fa8df33cfa7dcdda Mon Sep 17 00:00:00 2001 From: prakashUXtech Date: Thu, 2 Jul 2026 21:17:49 +0530 Subject: [PATCH 2/2] chore(lint): clear pre-existing ruff debt surfaced by enabling the gate Turning on lint for dev PRs exposed debt that had never been checked: - F811: the 'note' command's function was named observe_cmd, colliding with the 'observe' command's function of the same name. Both commands register fine (Click keys on the decorator, not the function name), so this is a rename to note_cmd with no behavior change. - I001 / ruff format: import ordering and formatting on six test files. Pure lint cleanup, no logic changes. --- src/soul_protocol/cli/main.py | 12 +++--------- tests/cross_runtime/test_crewai.py | 10 ++++------ tests/cross_runtime/test_langchain.py | 10 ++++------ tests/cross_runtime/test_pocketpaw.py | 12 ++++++------ tests/test_cli/test_note_cmd.py | 12 +++--------- tests/test_note/test_soul_note.py | 17 ++++------------- 6 files changed, 24 insertions(+), 49 deletions(-) diff --git a/src/soul_protocol/cli/main.py b/src/soul_protocol/cli/main.py index 41f74849..db1ccb85 100644 --- a/src/soul_protocol/cli/main.py +++ b/src/soul_protocol/cli/main.py @@ -1228,9 +1228,7 @@ async def _remember(): default=False, help="Skip contradiction detection on stored facts.", ) -def observe_cmd( - path, text, importance, emotion, memory_type, domain, no_dedup, no_contradictions -): +def note_cmd(path, text, importance, emotion, memory_type, domain, no_dedup, no_contradictions): """Note a fact in a Soul, with dedup against existing memories (#231). The brief for #231 originally specified ``soul observe`` for this @@ -1299,14 +1297,10 @@ async def _observe(): border = "cyan" sim_line = ( - f" Similarity [yellow]{similarity:.2f}[/yellow]\n" - if similarity is not None - else "" + f" Similarity [yellow]{similarity:.2f}[/yellow]\n" if similarity is not None else "" ) new_id_line = f" New ID [dim]{new_id}[/dim]\n" if new_id else "" - existing_line = ( - f" Existing ID [dim]{existing_id}[/dim]\n" if existing_id else "" - ) + existing_line = f" Existing ID [dim]{existing_id}[/dim]\n" if existing_id else "" console.print( Panel( diff --git a/tests/cross_runtime/test_crewai.py b/tests/cross_runtime/test_crewai.py index 49545734..2fefd844 100644 --- a/tests/cross_runtime/test_crewai.py +++ b/tests/cross_runtime/test_crewai.py @@ -36,9 +36,9 @@ async def _check(): assert len(results) > 0, f"No results for: {mem['content']}" contents = [r.content for r in results] - assert any( - mem["content"] in c for c in contents - ), f"Missing memory: {mem['content']}" + assert any(mem["content"] in c for c in contents), ( + f"Missing memory: {mem['content']}" + ) # CrewAI needs string content for r in results: @@ -47,9 +47,7 @@ async def _check(): asyncio.get_event_loop().run_until_complete(_check()) - def test_soul_identity_accessible_for_crewai_agent( - self, soul_path_semantic_only - ): + def test_soul_identity_accessible_for_crewai_agent(self, soul_path_semantic_only): """Soul identity fields are accessible for CrewAI agent backstory.""" async def _check(): diff --git a/tests/cross_runtime/test_langchain.py b/tests/cross_runtime/test_langchain.py index f66050b1..ebe51263 100644 --- a/tests/cross_runtime/test_langchain.py +++ b/tests/cross_runtime/test_langchain.py @@ -37,9 +37,9 @@ async def _check(): assert len(results) > 0, f"No results for: {mem['content']}" contents = [r.content for r in results] - assert any( - mem["content"] in c for c in contents - ), f"Missing memory: {mem['content']}" + assert any(mem["content"] in c for c in contents), ( + f"Missing memory: {mem['content']}" + ) # LangChain needs string content — verify it's available for r in results: @@ -48,9 +48,7 @@ async def _check(): asyncio.get_event_loop().run_until_complete(_check()) - def test_soul_identity_accessible_for_langchain_system_prompt( - self, soul_path_semantic_only - ): + def test_soul_identity_accessible_for_langchain_system_prompt(self, soul_path_semantic_only): """Soul identity fields are accessible for LangChain system prompt.""" async def _check(): diff --git a/tests/cross_runtime/test_pocketpaw.py b/tests/cross_runtime/test_pocketpaw.py index 9a732d05..b049addf 100644 --- a/tests/cross_runtime/test_pocketpaw.py +++ b/tests/cross_runtime/test_pocketpaw.py @@ -43,9 +43,9 @@ async def _check(): for mem in SEMANTIC_MEMORIES: results = await soul.recall(mem["content"]) contents = [r.content for r in results] - assert any( - mem["content"] in c for c in contents - ), f"Missing semantic memory: {mem['content']}" + assert any(mem["content"] in c for c in contents), ( + f"Missing semantic memory: {mem['content']}" + ) asyncio.get_event_loop().run_until_complete(_check()) @@ -63,9 +63,9 @@ async def _check(): # Every episodic memory should be present for mem in EPISODIC_MEMORIES: - assert any( - mem["content"] in c for c in contents - ), f"Missing episodic memory: {mem['content']}" + assert any(mem["content"] in c for c in contents), ( + f"Missing episodic memory: {mem['content']}" + ) asyncio.get_event_loop().run_until_complete(_check()) diff --git a/tests/test_cli/test_note_cmd.py b/tests/test_cli/test_note_cmd.py index 92882d2c..a9509e2b 100644 --- a/tests/test_cli/test_note_cmd.py +++ b/tests/test_cli/test_note_cmd.py @@ -83,9 +83,7 @@ def test_e2e_merge_overlapping_content(tmp_path): first = runner.invoke(cli, ["note", str(soul_path), "Aria likes Python"]) assert first.exit_code == 0, first.output - second = runner.invoke( - cli, ["note", str(soul_path), "Aria likes Python and async code"] - ) + second = runner.invoke(cli, ["note", str(soul_path), "Aria likes Python and async code"]) assert second.exit_code == 0, second.output out = second.output.lower() @@ -100,12 +98,8 @@ def test_e2e_no_dedup_writes_both(tmp_path): _birth_soul_at(str(soul_path)) runner = CliRunner() - first = runner.invoke( - cli, ["note", str(soul_path), "always store this raw", "--no-dedup"] - ) - second = runner.invoke( - cli, ["note", str(soul_path), "always store this raw", "--no-dedup"] - ) + first = runner.invoke(cli, ["note", str(soul_path), "always store this raw", "--no-dedup"]) + second = runner.invoke(cli, ["note", str(soul_path), "always store this raw", "--no-dedup"]) assert first.exit_code == 0, first.output assert second.exit_code == 0, second.output diff --git a/tests/test_note/test_soul_note.py b/tests/test_note/test_soul_note.py index a94d463e..2664f7f4 100644 --- a/tests/test_note/test_soul_note.py +++ b/tests/test_note/test_soul_note.py @@ -13,7 +13,6 @@ from soul_protocol.runtime.soul import Soul from soul_protocol.runtime.types import MemoryType - # --- Helpers ----------------------------------------------------------------- @@ -103,9 +102,7 @@ async def test_merge_path_supersedes_old_memory(): assert second["id"] != old_id assert second["existing_id"] == old_id sim = second["similarity"] - assert sim is not None and 0.6 <= sim <= 0.85, ( - f"Expected MERGE band similarity, got {sim}" - ) + assert sim is not None and 0.6 <= sim <= 0.85, f"Expected MERGE band similarity, got {sim}" # The old memory should be marked as superseded by the new one. all_facts = soul._memory._semantic.facts(include_superseded=True) @@ -193,12 +190,8 @@ async def test_procedural_tier_dedup_works(): """Procedural store goes through the same SKIP path as semantic.""" soul = await Soul.birth("Aria", archetype="t") - first = await soul.note( - "to deploy run make deploy and verify", type=MemoryType.PROCEDURAL - ) - second = await soul.note( - "to deploy run make deploy and verify", type=MemoryType.PROCEDURAL - ) + first = await soul.note("to deploy run make deploy and verify", type=MemoryType.PROCEDURAL) + second = await soul.note("to deploy run make deploy and verify", type=MemoryType.PROCEDURAL) assert first["action"] == "CREATE" assert second["action"] == "SKIP" @@ -217,9 +210,7 @@ async def test_return_shape_complete_across_all_paths(): create_result = await soul.note("Aria enjoys playing guitar") skip_result = await soul.note("Aria enjoys playing guitar") merge_result = await soul.note("Bob writes essays about typography") - merge_result_2 = await soul.note( - "Bob writes essays about typography and design" - ) + merge_result_2 = await soul.note("Bob writes essays about typography and design") for result in (create_result, skip_result, merge_result, merge_result_2): assert set(result.keys()) == {"action", "id", "existing_id", "similarity"}