fix(ui): approval centre and policy form called unwired callbacks
Neither noemad nor the test harness passed ui.Actions.Policy or Decide, so an operator's Approve, Deny and "add rule" forms dereferenced a nil function and returned 500. Both handlers now use the shared, audited server methods like the other study pages, and the unused callbacks are removed. The observer-escalation test only reached the role check, which is why this went unnoticed; the new browser test fails without the fix (policy form → 500) and passes with it.
This commit is contained in:
@@ -62,7 +62,7 @@ func (u *UI) policyCreate(w http.ResponseWriter, r *http.Request) {
|
|||||||
p.HoursFrom, p.HoursTo = &hf, &ht
|
p.HoursFrom, p.HoursTo = &hf, &ht
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if _, err := u.policyF(r, p); err != nil {
|
if _, err := u.studyF().CreatePolicy(r, p); err != nil {
|
||||||
u.flash(w, r, "error", err.Error(), "/policies")
|
u.flash(w, r, "error", err.Error(), "/policies")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -126,7 +126,7 @@ func (u *UI) approvalDecide(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
decision := r.PostFormValue("decision")
|
decision := r.PostFormValue("decision")
|
||||||
if _, err := u.decideF(r, id, decision, r.PostFormValue("note"), params); err != nil {
|
if _, err := u.studyF().Decide(r, id, decision, r.PostFormValue("note"), params); err != nil {
|
||||||
u.flash(w, r, "error", err.Error(), "/approvals")
|
u.flash(w, r, "error", err.Error(), "/approvals")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-7
@@ -21,7 +21,6 @@ import (
|
|||||||
"github.com/andydixon/noema/internal/attention"
|
"github.com/andydixon/noema/internal/attention"
|
||||||
"github.com/andydixon/noema/internal/auth"
|
"github.com/andydixon/noema/internal/auth"
|
||||||
"github.com/andydixon/noema/internal/beliefs"
|
"github.com/andydixon/noema/internal/beliefs"
|
||||||
"github.com/andydixon/noema/internal/capabilities"
|
|
||||||
"github.com/andydixon/noema/internal/consolidation"
|
"github.com/andydixon/noema/internal/consolidation"
|
||||||
"github.com/andydixon/noema/internal/conversation"
|
"github.com/andydixon/noema/internal/conversation"
|
||||||
"github.com/andydixon/noema/internal/goals"
|
"github.com/andydixon/noema/internal/goals"
|
||||||
@@ -60,8 +59,6 @@ type UI struct {
|
|||||||
entF func(r *http.Request, mind, id ids.ID) (api.EntityDetail, error)
|
entF func(r *http.Request, mind, id ids.ID) (api.EntityDetail, error)
|
||||||
mergeF func(r *http.Request, mind ids.ID, e graph.Entity, other string) error
|
mergeF func(r *http.Request, mind ids.ID, e graph.Entity, other string) error
|
||||||
distinctF func(r *http.Request, mind ids.ID, e graph.Entity, other string) error
|
distinctF func(r *http.Request, mind ids.ID, e graph.Entity, other string) error
|
||||||
policyF func(r *http.Request, p capabilities.Policy) (capabilities.Policy, error)
|
|
||||||
decideF func(r *http.Request, id ids.ID, decision, note string, params map[string]any) (any, error)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Actions groups the server-side operations the UI delegates to the API layer
|
// Actions groups the server-side operations the UI delegates to the API layer
|
||||||
@@ -80,13 +77,11 @@ type Actions struct {
|
|||||||
Resolve func(r *http.Request, mind, id ids.ID, outcome predictions.Status, reason string) (predictions.Result, error)
|
Resolve func(r *http.Request, mind, id ids.ID, outcome predictions.Status, reason string) (predictions.Result, error)
|
||||||
ResetState func(r *http.Request, mind ids.ID) (motivation.State, error)
|
ResetState func(r *http.Request, mind ids.ID) (motivation.State, error)
|
||||||
Consolidate func(r *http.Request, mind ids.ID) (consolidation.Job, error)
|
Consolidate func(r *http.Request, mind ids.ID) (consolidation.Job, error)
|
||||||
Study func() *api.Server // snapshots, forks, import, replay, experiments (audited server paths)
|
Study func() *api.Server // audited server paths shared with the API: study, capabilities, approvals, volition
|
||||||
Reflect func(r *http.Request, mind ids.ID) (reflection.Reflection, error)
|
Reflect func(r *http.Request, mind ids.ID) (reflection.Reflection, error)
|
||||||
Entity func(r *http.Request, mind, id ids.ID) (api.EntityDetail, error)
|
Entity func(r *http.Request, mind, id ids.ID) (api.EntityDetail, error)
|
||||||
Merge func(r *http.Request, mind ids.ID, e graph.Entity, other string) error
|
Merge func(r *http.Request, mind ids.ID, e graph.Entity, other string) error
|
||||||
Distinct func(r *http.Request, mind ids.ID, e graph.Entity, other string) error
|
Distinct func(r *http.Request, mind ids.ID, e graph.Entity, other string) error
|
||||||
Policy func(r *http.Request, p capabilities.Policy) (capabilities.Policy, error)
|
|
||||||
Decide func(r *http.Request, id ids.ID, decision, note string, params map[string]any) (any, error)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// View is the data passed to every template.
|
// View is the data passed to every template.
|
||||||
@@ -116,7 +111,7 @@ func New(deps app.Deps, a Actions) (*UI, error) {
|
|||||||
deps.Log.Info("ui: serving templates and static files from ./web (development)")
|
deps.Log.Info("ui: serving templates and static files from ./web (development)")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
u := &UI{deps: deps, static: staticFS, statusF: a.Status, ctxF: a.Context, wsF: a.Workspace, memF: a.Memory, belF: a.Belief, emitF: a.Emitter, postF: a.Post, goalF: a.Goal, goalUpdF: a.GoalUpdate, predF: a.Predict, resolveF: a.Resolve, resetF: a.ResetState, consolF: a.Consolidate, studyF: a.Study, reflF: a.Reflect, entF: a.Entity, mergeF: a.Merge, distinctF: a.Distinct, policyF: a.Policy, decideF: a.Decide}
|
u := &UI{deps: deps, static: staticFS, statusF: a.Status, ctxF: a.Context, wsF: a.Workspace, memF: a.Memory, belF: a.Belief, emitF: a.Emitter, postF: a.Post, goalF: a.Goal, goalUpdF: a.GoalUpdate, predF: a.Predict, resolveF: a.Resolve, resetF: a.ResetState, consolF: a.Consolidate, studyF: a.Study, reflF: a.Reflect, entF: a.Entity, mergeF: a.Merge, distinctF: a.Distinct}
|
||||||
if err := u.parse(tmplFS); err != nil {
|
if err := u.parse(tmplFS); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,46 @@
|
|||||||
|
package security
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"encoding/json"
|
||||||
|
"net/url"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/andydixon/noema/internal/capabilities"
|
||||||
|
"github.com/andydixon/noema/tests/harness"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Regression: the approval centre and policy form called UI callbacks that
|
||||||
|
// were never wired, so an operator could not approve, deny or add a rule
|
||||||
|
// from the browser. Both now go through the audited server paths.
|
||||||
|
func TestOperatorDecidesInTheBrowser(t *testing.T) {
|
||||||
|
h := harness.Start(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
m := h.Mind("desk")
|
||||||
|
c, csrf := h.Browser("admin", harness.AdminPass)
|
||||||
|
res, err := c.PostForm(h.Srv.URL+"/policies", url.Values{"_csrf": {csrf}, "mind_id": {m.ID.String()}, "capability": {"noema.note"}, "effect": {"ask"}, "reason": {"notes need a look"}})
|
||||||
|
if err != nil || res.StatusCode != 303 {
|
||||||
|
t.Fatalf("policy form: %v %d", err, res.StatusCode)
|
||||||
|
}
|
||||||
|
pols, _ := h.Deps.Capabilities.Policies(ctx, m.ID)
|
||||||
|
if len(pols) != 1 || pols[0].Effect != capabilities.Ask {
|
||||||
|
t.Fatalf("policy not created: %+v", pols)
|
||||||
|
}
|
||||||
|
for _, decision := range []string{"approve", "deny"} {
|
||||||
|
code, body := h.Do(h.Admin, "POST", "/api/v1/minds/desk/invoke", map[string]any{"capability": "noema.note", "params": map[string]any{"text": "hello"}, "reason": "test"})
|
||||||
|
var out capabilities.Outcome
|
||||||
|
json.Unmarshal(body, &out) //nolint:errcheck
|
||||||
|
if code != 200 || out.Status != "pending" {
|
||||||
|
t.Fatalf("invoke: %d %s", code, body)
|
||||||
|
}
|
||||||
|
res, err = c.PostForm(h.Srv.URL+"/approvals/"+out.ApprovalID.String()+"/decide", url.Values{"_csrf": {csrf}, "decision": {decision}, "note": {"from the browser"}})
|
||||||
|
if err != nil || res.StatusCode != 303 {
|
||||||
|
t.Fatalf("%s form: %v %d", decision, err, res.StatusCode)
|
||||||
|
}
|
||||||
|
a, _ := h.Deps.Capabilities.GetApproval(ctx, out.ApprovalID)
|
||||||
|
want := map[string]string{"approve": "executed", "deny": "denied"}[decision]
|
||||||
|
if a.Status != want || a.DecisionBy != "admin" {
|
||||||
|
t.Fatalf("%s: status %s by %q", decision, a.Status, a.DecisionBy)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user