Skip to content

feat(harness): add a component manifest and edit contract - #782

Open
HugoXK wants to merge 2 commits into
ThinkFlowLab:mainfrom
HugoXK:feat/harness-manifest
Open

HugoXK wants to merge 2 commits into
ThinkFlowLab:mainfrom
HugoXK:feat/harness-manifest

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #781.

What changed

Add agentdescent harness manifest|check and HarnessManifest: named file-level
components with an RRSI/SHE responsibility, editable/frozen state, per-component
file/line budgets, global budgets and allowed/forbidden paths. check fails
closed on frozen, forbidden, unowned or over-budget edits (0 valid, 1 violation,
2 input error), and tree_kwargs() projects the contract onto FileTree so an
unlisted file is never silently editable. This is the contract/checker half; the
existing runner and promotion path still own execution and acceptance. No model
or network call.

Tip SHA: e154f35
Ran:

  • pytest -q -n 12 (whole suite) -> 20 failed, 10 errors, all identical on
    unchanged main 27c43ee4dc03 (no docker daemon: python:3.11-slim
    cannot be pulled; host sandbox and uninstall cases).
  • pytest tests/test_harness_manifest.py -> 8 passed
  • python -m tools.gen_api_docs --check -> pass
  • python -m tools.changelog_fragments check -> pass (63 fragments)
  • mkdocs build --strict -> pass
    Not run:
  • a real harness repo rollout -> this PR is the manifest/checker; tests use a synthetic tree

Review fixes at this tip

  • tree_kwargs() treats editable: false exactly like frozen: true and never
    falls back to editable=("**",), so an all-read-only manifest stays read-only
    and the projection agrees with check(). Regression tests cover the
    read-only-only and mixed cases.
  • The violation reason is "not editable" when frozen is false.
  • Added the RRSI responsibility names to the accepted vocabulary and documented
    first-match ordering.

HugoXK added a commit to HugoXK/agentdescent that referenced this pull request Nov 5, 2026
@HugoXK HugoXK added area:cli The agentdescent command line (cli.py, doctor, plan/evolve/status/apply) enhancement New feature or request labels Nov 5, 2026
Birfy
Birfy previously requested changes Nov 5, 2026

@Birfy Birfy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: CHANGES REQUESTED
Tip SHA: 9f5b27c
Ran:

  • pytest tests/test_harness_manifest.py tests/test_docs_links.py tests/test_changelog_fragments.py -q → pass (fresh clone)
  • python -m tools.changelog_fragments check → pass
  • python -m tools.gen_api_docs --check → pass
  • manual FileTree(... , **manifest.tree_kwargs()).writable(...) probes for mixed / all-frozen / editable:false manifests
    Not run:
  • full pytest -q across 3.9–3.13 (CI still queued)
  • mkdocs build --strict (same libcairo gap as the PR)
  • a real harness-repo rollout (out of scope; this PR is contract/checker only)

Summary

Closes #781 with a clear fail-closed contract (HarnessManifest + harness manifest|check), docs, changelog fragment, and focused tests. Mixed editable+frozen manifests project correctly onto FileTree, and check covers frozen / forbidden / unowned / budget cases. Blocking issue is that tree_kwargs() does not preserve the read-only contract in two documented shapes, which silently re-opens paths the PR claims stay closed.

Blocking: tree_kwargs() invert for read-only-only manifests

Docs say frozen: true or editable: false make paths read-only, and that “a path outside every component is not editable by the loop”. Implementation:

target = frozen if component.frozen else editable
if not component.frozen and not component.editable:
    continue  # dropped from both lists
…
"editable": tuple(dict.fromkeys(editable)) or ("**",),

Probed on this tip against FileTree.writable:

manifest shape tree_kwargs other.txt writable?
mixed (prompts/** editable + safety/** frozen) editable=prompts/**, frozen=safety/** False (good)
all components frozen: true editable=**, frozen=safety/** True
only editable: false (no frozen) editable=**, frozen=() True (and the “read-only” component itself is writable)

So an all-frozen or editable:false manifest projects to “everything editable (minus explicit frozen globs)”, which contradicts the PR/body claim that unlisted files are never silently editable. check() still refuses those edits, so the checker and the FileTree projection disagree.

Fix direction: put non-editable paths into frozen (treat editable: false like frozen: true for projection); do not fall back to editable=("**",) when the editable list is empty — prefer editable=() (or only the explicitly editable globs) so fail-closed wins. Add regression tests for all-frozen and editable:false that assert FileTree(...).writable("unlisted") is False and that the read-only component paths are not writable.

Non-blocking

  1. Docs vs RESPONSIBILITIES: prose cites RRSI names (context_mgmt, client_tool, memory) that are not in the enum; the allowed set is SHE-ish (system_prompt, tool_policy, …). Either add aliases or drop the RRSI name list so users don’t paste rejected values.
  2. forbidden_paths load path: uses _text only, so "../x" loads; allowed_paths / component paths go through _paths. Align validation.
  3. List-form --changes: path lists set content to None, so max_lines never fires. Fine if intentional; say so in docs.
  4. PR body Tip SHA still cites 46bcbf5a…; tip is 9f5b27c8… after the changelog rename commit.
  5. Unused imports in harness_manifest.py: field, Iterable.

Risk / merge

risk_gate.py → T3 (597 lines / 6 files over the T2 size cap). Even after a green tip this waits on Danyang’s click; not merging from this review.

Birfy
Birfy previously requested changes Nov 5, 2026

@Birfy Birfy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: CHANGES REQUESTED
Tip SHA: 9f5b27c
Ran:

  • pytest tests/test_harness_manifest.py -q → 6 passed
  • Ad-hoc tree_kwargs() probes for non-editable / frozen-only manifests
    Not run:
  • wiring into a real FileTree evolve loop (PR is contract+CLI only)
  • mkdocs build --strict

Summary

Useful first cut: named components, frozen/forbidden/unowned/budget checks, CLI harness manifest|check, and changelog/docs. Offline tests cover the happy path and the main violation classes. One consistency bug below blocks PASS.

Blockers

  1. tree_kwargs() falls back to editable=(\"**\",) when no editable component remains — undoes non-editable intent and disagrees with check().
    "editable": tuple(dict.fromkeys(editable)) or ("**",),
    Probes on this tip:
    • one component editable=False, frozen=False, paths a/** → tree_kwargs() == {editable:(\"**\",), frozen:()} so FileTree would treat the whole tree as editable, while check({\"a/x\": \"…\"}) rejects the same path as frozen.
    • frozen-only manifest (editable=False, frozen=True on a/**) → {editable:(\"**\",), frozen:(\"a/**\",)}: anything outside a/** becomes editable via FileTree even though check says those paths are unowned.
      Fix: do not invent ** when the editable list is empty (use () / require at least one editable component / or project unowned paths into frozen). Add a regression test that tree_kwargs()[\"editable\"] is empty (or equal to the frozen set policy you choose) when every component is non-editable, and that it never re-opens paths check would reject.

Nits (non-blocking)

  • Violation text always says "is frozen" for editable=False even when frozen is false — prefer "not editable".
  • Module docstring cites RRSI names (context_mgmt, client_tool, …) that are not in RESPONSIBILITIES.
  • Overlapping component globs: first match wins with no warning; fine for v1, mention in docs.
  • Expect risk:T3 once the controller labels (new package surface + size).

Happy to re-review once the tree_kwargs empty-editable fallback is fixed and tested.

@Birfy
Birfy dismissed their stale review November 5, 2026 08:17

Duplicate of prior CHANGES REQUESTED on the same tip.

@HugoXK
HugoXK force-pushed the feat/harness-manifest branch from 9f5b27c to 646eb63 Compare November 5, 2026 08:27
@HugoXK
HugoXK requested a review from Birfy November 5, 2026 08:27

@Birfy Birfy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip SHA: 646eb63
Ran:

  • pytest tests/test_harness_manifest.py -q → 8 passed
  • ad-hoc: non-editable / frozen-only tree_kwargs() now return editable=() and project paths into frozen (no ** fallback)
    Not run:
  • wiring into a live FileTree evolve loop
  • mkdocs build --strict
    Verdict: PASS

Prior CR on 9f5b27c addressed: empty-editable no longer invents **, and violation text distinguishes frozen vs not-editable. Expect risk:T3; awaiting Danyang click.

@Birfy
Birfy dismissed their stale review November 5, 2026 08:29

Superseded by PASS on tip 646eb63.

@HugoXK HugoXK added the awaiting-approval Green T2/T3 PR: waits for a PASS at its tip and Danyang's merge click label Nov 5, 2026
@HugoXK

HugoXK commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

@Birfy All required checks are green and the latest review is PASS at 646eb63f. Ready for the owner merge click; awaiting-approval is set.

@HugoXK
HugoXK force-pushed the feat/harness-manifest branch from 646eb63 to 44ac261 Compare November 5, 2026 09:39
@HugoXK

HugoXK commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main 4c0515b2; combined the traces and harness CLI/parser blocks. Targeted manifest + traces tests pass. Please re-review.

Birfy
Birfy previously requested changes Nov 5, 2026

@Birfy Birfy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: CHANGES REQUESTED

Tip SHA: 44ac261

The rebase onto 4c0515b2 dropped main's tenant report subcommand (added in #740) from agentdescent/cli.py. git diff origin/main...HEAD -- agentdescent/cli.py removes the elif action == "report" branch in cmd_tenant, its printer block, and the tenant_sub.add_parser("report", ...) parser, and puts back the old error string "tenant needs init, plan or run". On this tip agentdescent tenant report --help fails with invalid choice: 'report' (choose from init, plan, run). The same 31 deleted lines appear identically in #772, #776, #778, #780 and #782, so it looks like one shared conflict resolution kept the pre-#740 side.

The feature diff itself matches the previously PASSed tip; only the conflict resolution regressed. Please restore the tenant report code from main (merge origin/main and keep both sides), and it would be worth adding a CLI-level test for tenant report so this can't silently regress again (tests/test_persistent_tenancy.py / tests/test_tenant_platform.py still pass without it).

Ran: interdiff vs previous PASS tip; git diff origin/main...HEAD --stat; agentdescent tenant report --help on #782's tip; pytest tests/test_persistent_tenancy.py tests/test_tenant_platform.py (pass, they don't cover the CLI).
Not run: full suite. Note: GitHub Actions hasn't fired on any PR since the repo moved to ThinkFlowLab, so there is no CI on this tip.

@HugoXK
HugoXK force-pushed the feat/harness-manifest branch from 44ac261 to d5e1822 Compare November 6, 2026 01:20
@HugoXK
HugoXK requested a review from Birfy November 6, 2026 01:20
@HugoXK

HugoXK commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Restored the tenant report subcommand from current main 4c0515b2 and kept the feature block. Targeted tenant + feature + traces tests pass. Please re-review d5e1822e04.

Birfy
Birfy previously approved these changes Nov 6, 2026

@Birfy Birfy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: PASS
Tip SHA: d5e1822

Prior CR (tenant report dropped on rebase) is fixed. tenant report parser/handler match main; agentdescent tenant report --help works. Feature suite still green.

Ran:

  • git diff origin/main...HEAD -- agentdescent/cli.py (tenant report intact)
  • agentdescent tenant report --help → OK
  • pytest tests/test_harness_manifest.py -q → 8 passed
    Not run: live FileTree evolve wiring; mkdocs build --strict.

risk_gate → T3 (size 626 > T2 cap). Do not auto-merge; awaiting Danyang click / 「都合了」. Actions still dead — no fresh CI on this tip.

@Birfy

Birfy commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

Retrigger CI after Actions re-enabled for ThinkFlowLab/agentdescent.

@Birfy Birfy closed this Nov 6, 2026
@Birfy Birfy reopened this Nov 6, 2026
@Birfy
Birfy dismissed their stale review November 6, 2026 06:59

Superseded by APPROVE on tip d5e1822 (tenant report restored); dismissing stale CR on 44ac261.

@Birfy

Birfy commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK This PR is CONFLICTING against current main (7651fb9c after recent merges). Please update the branch from main (merge preferred) and push so CI can re-run. Thanks!

@HugoXK
HugoXK force-pushed the feat/harness-manifest branch from d5e1822 to 89c54e2 Compare November 6, 2026 08:53
@github-actions github-actions Bot added the risk:T3 Risk tier T3 (core loop, release, process; needs Danyang's click), set by the controller label Nov 6, 2026
@HugoXK

HugoXK commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

Rebased onto main 426a1f54; required checks (build + test 3.9–3.13) are green at 89c54e2082. Risk gate: T3 — ready for @Birfy's explicit merge click.

Birfy
Birfy previously approved these changes Nov 6, 2026

@Birfy Birfy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: PASS

Tip SHA: 89c54e2

Rebase onto main 426a1f54. Feature modules unchanged vs prior PASS tip d5e1822e045a; only cli.py / mkdocs.yml picked up main. tenant report intact. Required CI green (build + test 3.9–3.13); mergeState UNSTABLE from stale cancelled reviewed tip fail only.

Ran:

  • python3 .github/scripts/risk_gate.py --repo ThinkFlowLab/agentdescent --pr 782 → T3 (624 lines / 6 files > T2)
  • pytest tests/test_harness_manifest.py -q → 8 passed
  • agentdescent harness --help / tenant report --help → OK
    Not run:
  • real harness repo rollout (tests use synthetic tree)
  • mkdocs build --strict

risk:T3 + awaiting-approval → no squash-merge; waiting Danyang click / 「都合了」.

@Birfy

Birfy commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK This PR is CONFLICTING again against current main (5e41c68c after #780 expand-vs-evaluate landed). Please update from main (merge preferred) and push so CI can re-run. Thanks!

@Birfy

Birfy commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK This PR is CONFLICTING again against current main (3ec07362 after #795/#796/#797). Please update from main (merge preferred) and push so CI can re-run. Thanks!

@Birfy

Birfy commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK This PR is CONFLICTING again against current main (20689124 after #798 ReflectiveFusion landed). Please update from main (merge preferred) and push so CI can re-run. Thanks!

@Birfy

Birfy commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK This PR is CONFLICTING again against current main (7fdbfa57 after #800 and later lands). Please update from main (merge preferred) and push so CI can re-run. Thanks!

@Birfy

Birfy commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK This PR is CONFLICTING again against current main (b7da5e36 after #811/#815 landed). Please update from main (merge preferred) and push so CI can re-run. Thanks!

@Birfy

Birfy commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK This PR is CONFLICTING again against current main (f1192800 after design-debt merge wave #810–#815). Please update from main (merge preferred) and push so CI can re-run. Thanks!

@Birfy

Birfy commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK This PR is CONFLICTING again against current main (5a56eb5b after #816+#826). Please update from main (merge preferred) and push so CI can re-run. Thanks!

@Birfy

Birfy commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator

@HugoXK This PR is CONFLICTING again against current main (59ba1460 after design-debt + #830). Please update from main (merge preferred) and push so CI can re-run. Thanks!

@HugoXK

HugoXK commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto main (91c936a6); required checks (build + test 3.9–3.13) green at 9b1f5dc7. Local tests/test_harness_manifest.py: 8 passed. The previous PASS is void at the new tip; reviewed tip is report-only. Ready for a fresh PASS and Danyang's merge click.

@Birfy

Birfy commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS

Tip SHA: 9b1f5dc

Rebase onto main 91c936a6. git diff main...HEAD is +624/-0 over 6 files. harness_manifest.py, docs/harness-manifest.md, the changelog fragment and tests/test_harness_manifest.py are byte-identical to the prior PASS tip 89c54e20; cli.py / mkdocs.yml only picked up main. tenant report intact. Local tests/test_harness_manifest.py: 8 passed. Required CI green (build + test 3.9–3.13).

risk_gate: T3 (size 624 lines) — not merging; waiting for Danyang's click.

@HugoXK
HugoXK force-pushed the feat/harness-manifest branch from 9b1f5dc to e154f35 Compare November 9, 2026 09:09
@HugoXK HugoXK added awaiting-approval Green T2/T3 PR: waits for a PASS at its tip and Danyang's merge click and removed awaiting-approval Green T2/T3 PR: waits for a PASS at its tip and Danyang's merge click labels Nov 9, 2026
@HugoXK

HugoXK commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

Maintenance after #772 merged: rebased onto main (27c43ee4); required checks green at e154f355. Local tests/test_harness_manifest.py: 8 passed. Ready for a fresh PASS and the merge click.

@HugoXK HugoXK removed the awaiting-approval Green T2/T3 PR: waits for a PASS at its tip and Danyang's merge click label Nov 9, 2026
@Birfy

Birfy commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS (tip e154f35)

Rebased onto main 27c43ee (after #772 merged). Feature diff is byte-identical to the previously PASSed tip 9b1f5dc (+624/-0, 6 files). Required CI green on this tip, merge state CLEAN. risk_gate: T3 (size) — waiting for owner click; not merged by automation.

Resolve agentdescent/cli.py by keeping both sides' commands: the branch's
`harness` manifest/check and main's `skills` report/audit/search, each with its
own error path. Everything else merged automatically.
@HugoXK

HugoXK commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Maintenance sync after #776 merged: old tip e154f3555c89 → new tip b0fcd1589d46. Main (1aaf1bd0) was merged in, not rebased; pushed fast-forward.

  • Only conflict: agentdescent/cli.py (both sides added a command) — kept the harness manifest/check block and the skills report/audit/search block, each with its own error path. mkdocs.yml merged cleanly; nothing beyond the conflict resolution.
  • Checks green at b0fcd158: build + test 3.9–3.13, deploy skipped.
  • Local: full suite 6832 collected, the same 20 failed / 10 error node ids as main 1aaf1bd0 (host-only docker and example paths); mkdocs build --strict ok; tests/test_harness_manifest.py + tests/test_skill_bank.py: 16 passed.

Please re-review.

@Birfy

Birfy commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS

Tip SHA: b0fcd15

New commit since the PASS at e154f35 is only a merge of main (1aaf1bd, #776). The PR's own changes against main are line-for-line identical to the previously PASSed tip (+624/-0, 6 files). Required CI is green and the PR is CLEAN.

risk_gate: T3 (size 624 lines > 250), so this waits for the owner's click; not merging.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:cli The agentdescent command line (cli.py, doctor, plan/evolve/status/apply) awaiting-approval Green T2/T3 PR: waits for a PASS at its tip and Danyang's merge click enhancement New feature or request risk:T3 Risk tier T3 (core loop, release, process; needs Danyang's click), set by the controller

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harness component manifest and edit contract

2 participants