Skip to content

chore(review): remove AutonomyLevel.suggest/.propose and fix a stale milestone-match comment - #4730

Merged
JSONbored merged 2 commits into
mainfrom
chore/4620-dead-config-decisions
Jul 10, 2026
Merged

chore(review): remove AutonomyLevel.suggest/.propose and fix a stale milestone-match comment#4730
JSONbored merged 2 commits into
mainfrom
chore/4620-dead-config-decisions

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

This is a decision-and-partial-implementation PR for #4620, which explicitly required a wire-or-remove call per item before any code changed. Verified every item against the actual current source (not just the issue's own characterization) — this caught two real mischaracterizations, documented below.

Decisions (all 9 items)

Item Decision Reasoning
AutonomyLevel.suggest/.propose REMOVE (this PR) Doc comment promised distinct "guidance/proposal" behavior; every read site (isActingAutonomyLevel/autonomyRequiresApproval) only ever distinguished acting from non-acting, so both were 100% identical to observe. No stored config uses either value — zero behavior change for any existing repo.
checkRunDetailLevel.deep REMOVE (follow-up) Confirmed both read sites (buildCheckRunAnnotations, formatCheckRunOutput in src/rules/advisory.ts) only branch on "minimal" vs everything else — "standard"/"deep" produce byte-identical output. The system's own design philosophy (formatCheckRunOutput's summary text: "Gittensory public check output is intentionally minimal. Detailed maintainer context is available only through private API/MCP surfaces") argues against ever wiring more public detail. Deferred to a follow-up PR — real footprint across 6 files (types.ts, openapi/schemas.ts, github/app.ts, api/routes.ts, rules/advisory.ts, db/repositories.ts), decision already made here so it's a fast, low-risk mechanical follow-up.
autoProjectMilestoneMatch: "auto" NOT dead — fixed the stale comment instead The issue (and the code's own comment) claimed this "behaves identically to suggest until #3185 wires real milestone attachment." Checked directly: #3185 shipped (closed 2026-07-10) and maybeSuggestMilestoneMatchForPr's "auto" branch genuinely calls attachToMilestone/attachToProject — this is a real, working, distinct behavior today. The only actual bug was a stale doc comment never updated after #3185 merged. Fixed in this PR.
firstTimeContributorGrace KEEP, no change Already deliberately decided via #2266 → PR #2411 ("mark gate.firstTimeContributorGrace as reserved/inert") 9 days ago — a dedicated prior issue already concluded blocker findings must remain closure outcomes regardless of contributor tenure, and chose to keep the field as documented-inert "for potential future use" rather than remove it. Not this issue's call to reverse.
copycatGateMode KEEP, no change Tied to #1969 (Plagiarism/copycat detection), which is still open. Genuinely reserved config surface for active, tracked future work — not abandoned.
qualityGateMode: "block" KEEP, no change Confirmed buildQualityGateWarning (src/rules/advisory.ts) always returns a severity: "warning" finding regardless of mode, and the finding code isn't in isConfiguredGateBlocker's allowlist either — this is a deliberate two-layer safety clamp (a fuzzy readiness score should never hard-block a PR), not forgotten dead code. Reversing it would be a real product decision about whether quality-gate should ever be allowed to block, not a cleanup.
reviewCheckMode: "visible" KEEP, no change The type's own doc comment is explicit: "the distinction is purely about how the operator should configure GitHub [branch protection]""visible" and "required" deliberately produce identical gittensory-side behavior by design; the value exists to signal operator intent, not to drive code. Not dead code.
ScreenshotTableGateAction: "advisory" KEEP, no change Also not actually dead — "advisory" skips only the auto-close path (screenshotTableMatch only forms when action === "close"), while the gate still evaluates and reports. A real, working, intentionally-scoped alternate mode (#4535), just mischaracterized by #4620's summary as a "no-op."
gateCheckMode Out of scope here Already tracked separately in #4618 (config-surface-reduction epic) — not duplicated in this issue's scope.

Net: 5 of 9 items were legitimate, deliberate design decisions already made elsewhere (not dead code) — only 2 are genuine removals, and 1 was a stale-comment bug rather than a config-surface issue at all. Recording this here so the next person doesn't re-litigate the same research.

What's actually in this PR

  • Removed "suggest"/"propose" from AutonomyLevel (src/types.ts), AUTONOMY_LEVELS (src/settings/autonomy.ts), the zod validator (src/api/routes.ts), the mirrored engine-package copies, and the maintainer-settings UI's local type/dropdown/description text.
  • Fixed ProjectMilestoneMatchMode's stale doc comment.
  • Updated 5 existing test files whose fixtures used "suggest"/"propose" as valid-but-arbitrary non-acting examples — replaced with "observe" (the value they were always behaviorally equivalent to) or another still-valid level, preserving each test's actual intent.

Follow-up (tracked, decision already made, not blocking this PR): remove checkRunDetailLevel.deep in a separate PR given its wider file footprint.

Closes #4620

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check (N/A — no API/OpenAPI schema change; enum shrink is TS-level only)
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:test
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — this is a pure type-surface removal with no new behavior; 319 tests across 5 affected files updated and passing, confirming zero behavior change for every remaining valid autonomy level.
  • npm run engine-parity:drift-check — clean; caught and fixed a real drift on the first pass (a comment-text mismatch between the two autonomy.ts copies).

If any required check was skipped, explain why:

  • No wrangler-binding, MCP, or build-pipeline surface touched. ui:test/ui:build weren't run locally (only ui:lint/ui:typecheck, both clean) given no runtime UI behavior changed (dropdown option count only); CI will run the full UI suite. test:coverage was run targeted on the 5 affected test files (319/319 passing) rather than the full unsharded suite.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed. (Ran the diff through the repo's own hasGenericSecretAssignment detector directly — clean.)
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics. (N/A)
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — TS-level enum only, no wire-format change; autonomy API field already accepted arbitrary strings server-side via allowlist-drop normalization)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — dropdown option list only)
  • Visible UI changes include a UI Evidence section below. See below — minor, text-only settings-panel change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A)

UI Evidence

Minor text-only change to the maintainer settings panel's autonomy dropdown (fewer options: observe → auto_with_approval → auto instead of observe → suggest → propose → auto_with_approval → auto) and its description text. No screenshot captured — purely a dropdown-option-count reduction with the exact same visual layout, not a rendering/state change.

Notes

  • Two of Resolve dead config branches (wire or remove) #4620's 9 items turned out to be mischaracterized by the original audit summary (autoProjectMilestoneMatch was actually already fixed by a since-shipped issue; ScreenshotTableGateAction.advisory was never actually dead). Verifying against live source rather than trusting the issue text caught both before any wrong code change happened.

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 10, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 33474d9 Commit Preview URL

Branch Preview URL
Jul 10 2026, 09:59 PM

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.22%. Comparing base (eafcd35) to head (3953217).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4730   +/-   ##
=======================================
  Coverage   94.22%   94.22%           
=======================================
  Files         440      440           
  Lines       38714    38714           
  Branches    14103    14103           
=======================================
  Hits        36478    36478           
  Misses       1576     1576           
  Partials      660      660           
Files with missing lines Coverage Δ
...ackages/gittensory-engine/src/settings/autonomy.ts 100.00% <100.00%> (ø)
src/api/routes.ts 94.26% <ø> (ø)
src/settings/autonomy.ts 100.00% <100.00%> (ø)
src/types.ts 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 10, 2026
@loopover-orb

loopover-orb Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-10 22:24:59 UTC

11 files · 1 AI reviewer · no blockers · readiness 100/100 · CI failing · blocked

🛑 Suggested Action - Manual Review

  • Touches a guarded path — held for manual review: This PR changes guardrail-protected path(s): src/settings/autonomy.ts (matched src/settings/autonomy.ts).

Review summary
This PR mechanically removes the dead AutonomyLevel.suggest/.propose values (verified identical-to-observe behavior at every read site) and fixes a stale doc comment claiming auto milestone-match was still unwired after #3185 shipped. The removal is applied consistently across both duplicated settings modules, the shared types, the API zod schema, the UI's local duplicate type, and every affected test file — no leftover reference to the removed values is visible in the diff. Scope is appropriately narrow: it explicitly defers the checkRunDetailLevel.deep item to a follow-up rather than bundling it, and ties itself to issue #4620.

Nits — 7 non-blocking
  • Confirm there's no DB-level CHECK constraint/migration still enumerating "suggest"/"propose" for the stored autonomy column — the PR asserts no stored config uses them but doesn't show the storage layer to prove it.
  • apps/gittensory-ui/src/components/site/app-panels/maintainer-settings.tsx hand-duplicates AutonomyLevel/AUTONOMY_LEVELS instead of importing the shared type from src/types.ts — pre-existing debt, but since this PR touches the exact lines, worth importing the shared type instead.
  • The updated doc comment on ProjectMilestoneMatchMode (src/types.ts) asserts maybeSuggestMilestoneMatchForPr's "auto" branch calls attachToMilestone/attachToProject — that function isn't in the diff, so this claim is unverified from what's visible (low risk, comment-only).
  • Add a one-line grep/CI check (or note in the PR body) confirming no remaining "suggest" / "propose" string literal exists outside the changed files, since the safety argument rests entirely on that claim.
  • Follow up per the PR's own plan to remove checkRunDetailLevel.deep given the decision is already made — low risk, mechanical, same shape as this PR.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.

CI checks failing

  • validate
  • validate-code
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #4620
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 48 registered-repo PR(s), 40 merged, 285 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 285 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 48 PR(s), 285 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Visual preview
Route Viewport Before (production) After (this PR's preview) Diff
/ desktop before / after /
/ mobile before / (mobile) after / (mobile)

Click any thumbnail to open the full-size screenshot. Before = production · After = this PR's preview deploy.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

…milestone-match comment (#4620)

Both values were 100% behaviorally identical to "observe" since day one --
isActingAutonomyLevel/autonomyRequiresApproval only ever distinguished
acting (auto/auto_with_approval) from everything else, so the doc
comment's promise ("surface guidance/concrete proposals without
executing") was never implemented. No stored config used either value
(both already degraded to observe's no-op behavior), so this is a pure
type-surface removal with no runtime behavior change for any existing
repo. Updates the mirrored engine-package copy + the UI's own local
type/dropdown/description text in the same commit.

Also fixes ProjectMilestoneMatchMode's doc comment, which still claimed
"auto" behaves identically to "suggest" until #3185 wires real milestone
attachment -- #3185 shipped (verified: maybeSuggestMilestoneMatchForPr's
"auto" branch genuinely calls attachToMilestone/attachToProject), the
comment was just never updated to match.
…uggest removal (#4620)

test/unit/config-templates.test.ts still exercised the removed "suggest"
level, which normalizeAutonomyPolicy now silently drops as invalid --
switch both fixtures to auto_with_approval, a still-valid level distinct
from the sibling "auto" action in the same assertion.
@JSONbored
JSONbored merged commit f09c0ce into main Jul 10, 2026
8 checks passed
@JSONbored
JSONbored deleted the chore/4620-dead-config-decisions branch July 10, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

Resolve dead config branches (wire or remove)

1 participant