docs(goal): correct four stale status statements for merged capabilities - #181
Conversation
|
HOLD — this is a docs-only correction and no introduced code defect was found in the bounded pass, but there is no formal review or CI result on current head |
PeterGuy326
changed the title
|
Current-head CI approval readback (2026-09-10) Head:
Failures at this snapshot: The title scope GOAL violated the lowercase Conventional Commit pattern. Changed it to docs(goal) without touching code; the newly triggered PR Policy run 34435739516 was also approved and is green. The old attempt failure above is historical metadata evidence. |
- §5 跨设备恢复: separate merge_conservative (done) from 增量同步 and 断点上传 (not done), so one clause cannot make the other two look shipped. - §5 人类可视化: correction/supersede relations are merged (0022_memory_relations.sql, relation.go); only audit history remains. - §5 数据可移植性: merge_conservative import is merged (merge.go, 0023_workspace_import_merge.sql); update "fresh restore only" claim. - §6 P1: split merge_conservative (checked) from 增量包/断点上传 (still open). - §6 P2: check off correction/supersede, 导入历史 and 权限管理界面 (all shipped via PRs bytefolk#90/bytefolk#95, bytefolk#102, bytefolk#100). Fixes bytefolk#179.
e26373f to
4a25bd2
Compare
|
Not a review, not an approval, carries no vote, does not ask to merge or close. No write action taken here: no review submitted, no First evidence post on this head. 1. 负责人
2. Provenance of what I read
I unpacked that exact commit and pinned it: The diff itself is one file, 3. 逐条核 — two claims fully hold, one holds only at the service layer, one is compoundClaim:
So after this PR merges, Claim: correction/supersede 关系已实现 + 不可变 (row 人类可视化, and part of the P2 Claim: 导入历史已实现. Holds end to end: Claim: 更完整的权限管理界面 (the third sub-clause of the same P2 item). Holds: The shape to note is that this P2 line is one checkbox carrying three sub-claims, and marking the compound Where the PR is correctly conservative, so this doesn't read as one-sided: it keeps 4. 修复 commitNone is owed to this diff: it contains no code and no false statement in the strict sense — 5. 回归 / CI 证据 on the exact current head
|
This PR cannot merge, and it is not a review problemRecording this so nobody waits on an approval that would not help.
Mechanism
codeql:
name: CodeQL (${{ matrix.language }})
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}This PR's head is When a matrix job is skipped by a job-level That context name matches none of the three required ones. The required contexts are therefore never satisfied — not "failing", but absent, which branch protection treats as pending forever. Confirmation by contrastSame-repo PRs in this repository report all four contexts normally. mem #165 and #188, both merged 2026-09-10: This PR's rollup contains one CodeQL entry, the literal What will not fix it
What will fix itThe guard is a defensible security choice: the Available resolutions, in rough order of how much they preserve the current security posture:
Option 1 is a mechanical fix the author or any maintainer can do. Options 2–4 are owner decisions that affect all twelve repositories, because Scope of the same trap, org-wideEvery open fork PR in a repository that requires a CodeQL context is permanently unmergeable:
Worth naming the consequence for external contributors specifically: I have not changed any branch protection, ruleset or workflow file, and I am not opening a PR against the template unilaterally — the same reasoning that made me leave |
waterbro-8
left a comment
waterbro-8
left a comment
There was a problem hiding this comment.
APPROVE — all four corrected statements verify against current main
Docs-only (GOAL.md, 4 statements + 2 checkboxes), no code path touched. Since the entire
value of this file is that it does not overstate, I re-derived each new claim against 3c13f04
rather than taking the diff's word for it.
GOAL.md now claims |
Evidence on main | Landed |
|---|---|---|
merge_conservative 已实现 |
server/internal/workspacetransfer/merge.go, migration 0023_workspace_import_merge.sql, transfer_merge_integration_test.go, surfaced in web/src/lib/workspace-transfer.ts + web/src/lib/types.ts |
36527fe (#98), 2026-08-19 |
| correction/supersede 关系已实现 | migration 0022_memory_relations.sql, handlers_memory.go, web/src/components/memory/MemoryRelationsPanel.tsx + CreateRelationDialog.tsx |
48efd39 (#90), 2026-08-07 |
| P2: 导入历史 | server/internal/workspacetransfer/history.go, web/src/pages/TransferPage.tsx + TransferPage.history.test.tsx |
— |
| P2: 更完整的权限管理界面 | web/src/pages/PermissionsPage.tsx (+ test), web/src/lib/permissions.ts |
2caa8e3 (#100), 2026-08-19 |
So GOAL.md was roughly a month stale when this PR was opened, and every replacement
statement is true at the head you are asking to merge.
The negative half also holds. You left 增量包 / 断点上传 / 本地同步盘 unchecked. I grepped for
resumable / multipart / chunked upload and for incremental or delta bundles across server/
and web/src: every resumable hit belongs to durablecontext (context continuation, not
upload), and there is no incremental-bundle implementation. Correctly left unchecked.
CI
At head 4a25bd25 all 4 workflow runs on bytefolk/mem are completed/success (CI,
Validate Agent memory, ByteFolk Security Baseline, PR Policy) and all 17 check-runs are
green, including Conventional title and linked issue. The only blocker is
mergeable_state=behind — please sync with main before merge so the green is re-proven on the
merge result.
Provenance
Diff read plus a per-claim grep of main (3c13f04) and git log dating of each cited file.
Nothing executed; no Go/Node tests run locally.
## What this changes
One line deleted from `.github/workflows/bytefolk-security.yml` — the
`if:` guard on the `codeql` job. Nothing else in the file or the
repository changes.
```diff
codeql:
name: CodeQL (${{ matrix.language }})
- if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}
runs-on: ubuntu-24.04
```
Blob size goes 1988 -> 1867 bytes. The deleted line is exactly 121 bytes
including its newline, so the arithmetic accounts for the whole delta:
no whitespace, reordering or annotation change rode along.
## Why
The guard's second clause compares the head repository to the base
repository. It is false for every fork pull request, so the `codeql` job
is skipped.
GitHub does not evaluate a job's `name:` expression when the job is
skipped by its `if:` guard. The skipped job publishes the raw template
string as its check name, so fork PRs in this repository report a check
literally named `CodeQL (${{ matrix.language }})`, which matches none of
the three required contexts `CodeQL (go)`, `CodeQL
(javascript-typescript)` and `CodeQL (python)`.
This is not hypothetical. Two fork PRs by `@sun-970` were measured
through `GET /repos/bytefolk/mem/commits/{sha}/check-runs` after their
first-time workflow runs were approved:
| PR | head | total | success | failed | skipped |
| --- | --- | --- | --- | --- | --- |
| #172 | `a29e9163` | 17 | 16 | 0 | 1 |
| #181 | `4a25bd25` | 17 | 16 | 0 | 1 |
The single skip in each is `CodeQL (${{ matrix.language }})`. Both are
otherwise fully green and both report `mergeable: true`, yet neither can
ever merge: the three CodeQL contexts are required and nothing the
contributor does produces them. The un-interpolated name is a symptom of
the skip, not a separate defect — on `bytefolk/digital-employee#250` the
same expression interpolated to `CodeQL (javascript-typescript)` as soon
as the job actually ran.
## Why deleting the guard rather than adding a `codeql-fork` job
`bytefolk/digital-employee#250` solved this by adding a second job with
the inverted guard. That job is identical to the baseline `codeql` job
apart from the guard and the language matrix, so a second lane
duplicates roughly 37 lines to express what deleting one line already
says.
The guard is also provably redundant for every trigger other than a fork
PR:
- `push`, `schedule`, `workflow_dispatch` — the first clause
`github.event_name != 'pull_request'` is already true.
- Same-repository `pull_request` — the second clause is already true.
- Fork `pull_request` — the job was skipped; it now runs. **This is the
only behavioural change.**
## Evidence that CodeQL actually works on a fork `pull_request` run
`bytefolk/digital-employee#250` is a fork PR (head
`PeterGuy326/digital-employee:feat/issue-245-memory-config`, sha
`abb2d58b29710927742c828a510c419c7c53efb8`). Its `codeql-fork` job
concluded `success` — check runs `102757987331` (84s) and `102757703884`
(86s).
That job declares `permissions: security-events: write` and contains
**no** `continue-on-error` on the job or on any step. A `success` job
conclusion therefore means every step succeeded, including `Analyze` —
and `github/codeql-action/analyze` fails with HTTP 403 when
`security-events: write` is absent. So SARIF upload from a fork
`pull_request` run is confirmed, not assumed.
Fork `pull_request` runs still receive no secrets and read-only
`contents`. All ByteFolk repositories are public.
## Tradeoff, stated plainly
After this change, fork PRs build untrusted code in a job holding
`security-events: write`. The run still has no secrets and read-only
`contents`, and every other required check in this repository (`Go`,
`Worker`, `Web`, `PostgreSQL integration`, `Web memory and transfer
acceptance`, `HTTP, CLI and MCP lifecycle`, `Workflow, scripts and
Compose`) already builds that same untrusted code, so this adds no new
class of exposure. It is the posture GitHub's own default CodeQL setup
takes for public repositories.
The alternatives were considered and rejected:
- `pull_request_target` would hand a write-scoped token to a workflow
run over attacker-influenced code.
- Self-reporting the three contexts through the Statuses API would
fabricate a required check that never ran.
- Granting contributors organization membership does not help at all:
the guard compares repositories, not author identity.
## What this does not touch
- No `permissions:` block changes. The job keeps exactly `contents:
read`, `actions: read`, `packages: read`, `security-events: write`.
- No required status context, branch protection rule or ruleset is
weakened or removed.
- No action SHA, trigger, matrix entry, step or job other than the
deleted line.
- The pinned `# v4.37.4` annotations are deliberately left alone. They
are the subject of bytefolk/.github#32 and #203, and this PR must stay
disjoint from them.
## Interaction with #203
#203 edits this same file, but only the three `github/codeql-action/*`
annotation lines at 58, 65 and 68, plus `bytefolk-scorecard.yml`. This
PR deletes line 36. The hunks do not overlap, so the two 3-way-merge
cleanly in either order. Neither needs rebasing because of the other.
## Checks
`Workflow, scripts and Compose` runs `actionlint` over every workflow.
Deleting a job-level conditional cannot introduce an actionlint finding;
the resulting YAML keeps `name`, `runs-on`, `timeout-minutes`,
`permissions`, `strategy` and `steps` on the `codeql` job. The release
pin validators in that job (`validate_release_action_pins.sh`,
`test_release_guards.sh`, `test_release_helpers_compat.sh`) all hardcode
`release.yml` and never read this file.
This branch is same-repository rather than a fork, so the PR's own three
CodeQL contexts can go green here — otherwise the fix could not
demonstrate itself.
## Follow-up
The identical guard sits in
`bytefolk/.github/workflow-templates/bytefolk-security.yml`, which is
where every consumer copied it from. A separate template PR is filed so
the fix propagates instead of regressing on the next template sync. Any
consumer that added its own `codeql-fork` lane (`digital-employee`, via
#250) must delete it once the template changes, otherwise two jobs
publish the same check name.
After this merges, #172 and #181 need a branch update rather than a
plain re-run: for `pull_request` events GitHub takes the workflow YAML
from the merge ref, while a re-run replays the workflow version captured
when the run was created. Both are already `state: behind` under
`strict: true`, so they need the update regardless.
Refs bytefolk/.github#35
Co-authored-by: 勒布朗-詹姆斯 <2986253039@qq.com>
3 participants
Summary
Fixes #179.
GOAL.md§5 and §6 contain four status statements that list capabilities as missing when they are already merged and reachable onmain. This PR corrects them to match what the code actually supports.Changes
§5 — 现状与愿景的偏差 table
merge_conservative(done,merge.go+0023_workspace_import_merge.sql) from 增量同步 and 断点上传 (not done). The old wording made all three look missing when only two are.0022_memory_relations.sql,relation.go); only audit history remains open.merge_conservativeimport is merged; update "当前服务只支持 fresh restore" to reflect that merge is now supported.§6 — 近期优先级
merge_conservative(checked) from 增量包/断点上传 (still open). Previously one unchecked item bundled done and undone work together.Evidence
Each correction is backed by source-level evidence (file presence + migration content) as documented in #179. No runtime verification was performed; see issue's "Evidence level: E2 — source-level" note.
Test plan